-
Notifications
You must be signed in to change notification settings - Fork 3.2k
WP_Theme_JSON: preserve valid non-preset settings when KSES filters are active #10534
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?
WP_Theme_JSON: preserve valid non-preset settings when KSES filters are active #10534
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. |
| * @param array $input Input settings to process. | ||
| * @param array $output Output settings array (passed by reference). | ||
| * @param array $schema Schema to validate against (typically VALID_SETTINGS). | ||
| * @param array $path Current path in the schema (for recursive calls). |
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.
This seems like an opportunity to supply a more specific type for the $path parameter, like array<string|int>:
| * @param array $input Input settings to process. | |
| * @param array $output Output settings array (passed by reference). | |
| * @param array $schema Schema to validate against (typically VALID_SETTINGS). | |
| * @param array $path Current path in the schema (for recursive calls). | |
| * @param array $input Input settings to process. | |
| * @param array $output Output settings array (passed by reference). | |
| * @param array $schema Schema to validate against (typically VALID_SETTINGS). | |
| * @param array<string|int> $path Current path in the schema (for recursive calls). |
At first I was thinking string[], but in looking at _wp_array_set(), it has this logic:
wordpress-develop/src/wp-includes/functions.php
Lines 5171 to 5178 in a107147
| foreach ( $path as $path_element ) { | |
| if ( | |
| ! is_string( $path_element ) && ! is_integer( $path_element ) && | |
| ! is_null( $path_element ) | |
| ) { | |
| return; | |
| } | |
| } |
Curiously, there is no _doing_it_wrong() here. Also curious is that that null is apparently allowed among the $path array? This seems wrong.
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.
Good spotting. yeah it seems weird that $path = array('a', null, 'b') is allowed.
Looks like the tests don't cover it either
…set, non-CSS settings during property removal. Added unit tests to validate the functionality of safe settings and their existence in VALID_SETTINGS.
…e_properties`. This update includes the ticket number 64280 in the documentation for the tests that validate safe settings and their paths in valid settings.
… used only internally. Update unit tests to validate the removal of insecure properties and ensure safe settings are not allowed to be unsafe.
…n settings. Introduce a new method to preserve valid typed settings based on schema markers. Update unit tests to ensure correct behavior for boolean values and their validation against the schema.
…o assertions. This improves clarity on expected outcomes for lightbox settings and appearance tools, ensuring better maintainability and understanding of test intentions.
…alls in `preserve_valid_typed_settings`. This change enhances consistency in method referencing within the class.
…ions for parameters in the preserve_valid_typed_settings method, enhancing clarity and type safety.
26f0a1a to
82fd70a
Compare
Co-authored-by: Weston Ruter <[email protected]>
A PR to sync WordPress/gutenberg#73452
Problem
When KSES filters are active (via
add_action( 'init', 'kses_init_filters' )), valid non-preset settings in Global Styles are being incorrectly filtered out. Specifically:lightbox.enabledandlightbox.allowEditingfor Image blocksThe issue occurs because
remove_insecure_settings()only preserved:PRESETS_METADATA)INDIRECT_PROPERTIES_METADATA)All other valid settings were being stripped, even though they're defined in
VALID_SETTINGSand are safe scalar values or arrays.Related Issue: WordPress/gutenberg#73157
Solution
Added a new
preserve_valid_settings()method andSAFE_SETTINGSconst that work to preserves valid settings.Testing Instructions
Manual Testing
Enable KSES filters:
Add this to your theme's
functions.phpor a plugin.Test Image Block Lightbox Settings:
Test Other Valid Settings:
Trac ticket: https://core.trac.wordpress.org/ticket/64280
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.