-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
WP_Theme_JSON_Gutenberg: Add nested indexed array schema sanitization #56447
Conversation
This reverts commit 27c0f6f.
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/class-wp-theme-json-gutenberg.php ❔ phpunit/class-wp-theme-json-test.php |
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.
I think this is a better solution than #53273 because it adds less complexity, and is more extensible in case we want to add more sanitization to keys containing indexed arrays.
I tested this according to the instructions, as well as modifying some tests in the Tests_Fonts_WPRESTFontLibraryController_InstallFonts
and ensuring that the sanitization is working as expected.
We should keep an eye on any related unit tests after this is merged.
…#56447) * Add GB specific resolver * changing unset function * adding nested validation * rename function * Revert "Add GB specific resolver" This reverts commit 27c0f6f. * removing not needed check * removing not needed check * Remove keys from tree that are not arrays when they are defined as arrays in schema * remove key if it is empty * adding tests * adding function docs * php format * add comment to function --------- Co-authored-by: hellofromtonya <[email protected]>
@matiasbenedetto Can you confirm you will backport this as part of the Fonts backport process, or shall we commit this separately? |
@getdave 👋 nope, I think it's convenient to sync-up this in the core repo in a different PR. This functionality is useful not just for font families related settings, but for any indexed array we want to sanitize inside WP_Theme_JSON. |
What?
WP_Theme_JSON_Gutenberg
class.WP_Theme_JSON_Gutenberg
This is an alternative approach to #53273, and hopefully fixes: #52798
Co-authrored by: @jffng
Why?
To sanitize data stored in indexed arrays, for example, font family definitions. Without this PR, that's not possible.
Currently,
WP_Theme_JSON
sanitization is not able to sanitize data contained on indexed arrays. So certain data from theme.json, for example,settings.typography.fontFamilies
which is a JSON array, cannot be sanitized because when parsing the JSON dataWP_Theme_JSON
translates JSON arrays into PHP indexed arrays and the class is not capable of sanitizing that kind of data.How?
Treating the associative arrays and indexed arrays differently.
Testing Instructions
npm run test:unit:php
functions.php
file.