Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.1/8.2 warnings and errors #800

Open
surrim opened this issue Jan 31, 2023 · 2 comments
Open

PHP 8.1/8.2 warnings and errors #800

surrim opened this issue Jan 31, 2023 · 2 comments
Labels
Milestone

Comments

@surrim
Copy link
Contributor

surrim commented Jan 31, 2023

I'm currently working on PHP 8.2 support. The biggest problems and errors are already fixed 🚀
There are many smaller warnings and errors waiting for fixes.

Currently I'm stuck on updating media properties.
To reproduce: Edit any media and click safe. The following SQL statements will be executed:

DELETE FROM serendipity_mediaproperties
                                WHERE mediaid = 123
                                  AND property_group = 'base_property'
                                  AND property_subgroup = '';
INSERT INTO serendipity_mediaproperties (mediaid, property_group, property_subgroup, property, value)
                                   VALUES (123, 'base_property', '', 'DATE', '1688169600');
INSERT INTO serendipity_mediaproperties (mediaid, property_group, property_subgroup, property, value)
                                   VALUES (123, 'base_property', '', 'COPYRIGHT', 'root');
INSERT INTO serendipity_mediaproperties (mediaid, property_group, property_subgroup, property, value)
                                   VALUES (123, 'base_property', '', 'TITLE', 'filename.png');
INSERT INTO serendipity_mediaproperties (mediaid, property_group, property_subgroup, property, value)
                                   VALUES (123, 'base_property', '', 'COMMENT1', '');
INSERT INTO serendipity_mediaproperties (mediaid, property_group, property_subgroup, property, value)
                                   VALUES (123, 'base_property', '', 'COMMENT2', '');
INSERT INTO serendipity_mediaproperties (mediaid, property_group, property_subgroup, property, value)
                                   VALUES (123, 'base_property', '', 'ALT', '');
INSERT INTO serendipity_mediaproperties (mediaid, property_group, property_subgroup, property, value)
                                   VALUES (123, 'base_property', 'internal', 'id', '123'); # error

First the (sub)groups 'base_property' / '' values are deleted, then 'base_property' / '' values are written - but also 'base_property' / 'internal' values. That can't work (same for 2.4.0) because of the unique index media_idx (mediaid, property, property_group, property_subgroup).
The relevant code is located here:

  • include/admin/images.inc.php: serendipity_parsePropertyForm()
  • include/functions_images.inc.php: serendipity_insertMediaProperty()
  • include/functions_images.inc.php: serendipity_db_query()

Thanks for any contributions - also for information how 2.4.0 can execute the same code without crashing.

@surrim surrim added this to the 2.4.1 milestone Jan 31, 2023
@onli
Copy link
Member

onli commented Jan 31, 2023

(The happy smiley is about the mentioned other fixes ;) )

Let's make sure I understand this correctly: The DELETE deletes everything with 'base_property' with an empty subgroup. So all following INSERTs where the subgroup is empty can work (as the property differs), but the failing one can not because the subgroups is 'internal' -> it was not deleted before and still exists, the index complains.

If that's the case I would also doubt that this works in 2.4.0 (you already confirmed it does?)

@surrim
Copy link
Contributor Author

surrim commented Jan 31, 2023

The DELETE deletes everything with 'base_property' with an empty subgroup. So all following INSERTs where the subgroup is empty can work (as the property differs), but the failing one can not because the subgroups is 'internal' -> it was not deleted before and still exists, the index complains.

Yes, exactly.
There are much more INSERTs with 'base_property / internal following. I just printed the statements before they are executed. I compared the relevant source code with 2.4.0 and it's the same. I didn't dive deeper - it was quite late 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants