-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Notify about double serialization. - 62483 #7847
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
base: trunk
Are you sure you want to change the base?
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
@apermo PHP Unit Tests are failing, this changeset will require some test case update. |
|
@audrasjb I'll try to figure out how to fix them. In case I need help I'll reach out. |
src/wp-includes/functions.php
Outdated
| _doing_it_wrong( | ||
| 'maybe_serialize', | ||
| 'Double serialization detected, serialized data is likely sent to update_option(), add_option(), update_*_meta(), add_*_meta() and similar functions. These functions will automatically serialize if needed.', | ||
| '3.6.1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual error message generated is:
Double serialization detected, serialized data is likely sent to update_option(), add_option(), update_*_meta(), add_*_meta() and similar functions. These functions will automatically serialize if needed. (This message was added in version 3.6.1.)
I assumed that the third parameter of _doing_it_wrong() expected the WP version, where the behavior was added, but reading the message, this looks like it should be the version where the message is added, so possibly 6.8?
Your thought?
|
@audrasjb That recent push should fix all but one test error, and I'm unsure how to handle that one. That is thrown in The Data provider will return value that will trigger the double serialization. wordpress-develop/tests/phpunit/tests/privacy/wpPrivacyGeneratePersonalDataExportFile.php Line 237 in 12eaef1
My best guess would be to remove that single line, that did fix the remaining error locally. It was added by @SergeyBiryukov in https://core.trac.wordpress.org/changeset/50613 Happy to discuss this, maybe Sergey can provide an idea. |
|
Added a condition to the test, instead of altering the dataset, so the test is not weakened. |
…invalid_type() This catches the `maybe_serialize` `_doing_it_wrong()` message.
Added _doing_it_wrong() to maybe_serialize to notify about double serialization.
Trac ticket: https://core.trac.wordpress.org/ticket/62483#ticket
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.