Opened 5 weeks ago
Last modified 4 weeks ago
#65608 new enhancement
Themes: Harden process_blocks_custom_css() against non-string input
| Reported by: | hannahtinkler | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Awaiting Review |
| Component: | Themes | Version: | trunk |
| Severity: | normal | Keywords: | has-patch has-unit-tests close |
| Cc: | Focuses: |
Description
WP_Theme_JSON::process_blocks_custom_css() accepts a $css parameter typed as string in its docblock, but performs no runtime type check. If a non-string value reaches the function, the explode() call will produce a PHP fatal error (8.X) or warning (7.X).
This can happen because:
- The
VALID_STYLESschema marks CSS as null, which meansremove_keys_not_in_schema()accepts any type for this key - arrays and integers pass through schema sanitization unchanged. - The
wp_theme_json_data_*filters allow plugins and themes to modify the theme JSON data at runtime. A callback that returns a non-string value for CSS (e.g. an array) will not be caught by the sanitization and will reachprocess_blocks_custom_css(). - The method was made
public staticin 7.0 (changeset 61678) for use by thecustom-css blocksupport, making it callable by external code.
The function already has an empty() guard at the top. Extending this to also return early on a non-string value keeps the fix local to the function and consistent with how it already handles unusable input.
Change History (3)
This ticket was mentioned in PR #12478 on WordPress/wordpress-develop by @hannahtinkler.
5 weeks ago
#1
- Keywords has-patch has-unit-tests added
#2
@
4 weeks ago
- Keywords close added
Thanks for the report.
Extending this to also return early on a non-string value keeps the fix local to the function and consistent with how it already handles unusable input.
Doing so would hide fatal errors, preventing developers from realizing they are passing incorrect parameters. I believe it is the developer's responsibility to adhere to the types defined in the Docblock.
#3
@
4 weeks ago
Thanks for checking this out @wildworks 🙂
Doing so would hide fatal errors, preventing developers from realizing they are passing incorrect parameters.
That's a fair concern, but I think the context here is different from a developer directly calling process_blocks_custom_css() with the wrong type. The main internal call site is get_styles_for_block(), which passes $node['css'] (data from $this->theme_json that has passed through wp_theme_json_data_* filters). Plugins/themes/custom code whose filter callbacks accidentally return a non-string for CSS wouldn't always see a useful error pointing at their code - the page may not render at all.
Other methods in this class (to_ruleset(), remove_keys_not_in_schema(), is_valid_viewport_breakpoint_size() etc) already protect against non-string input by silently skipping non-string values, so doing so in process_blocks_custom_css() would be internally consistent.
That said, if the concern is specifically about hiding the mistake, I have updated the PR to add a _doing_it_wrong() notice alongside the early return so that the incorrect usage is surfaced in debug logs without crashing the site.
it is the developer's responsibility to adhere to the types defined in the Docblock
In principal I agree, but per the handbook:
When using a filter, the type of the variable that's returned from the filter is not guaranteed, even if it's documented [...] The best way to make sure the value returned from any filter callbacks stays the type you expect is to validate the data type in your code.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Return early when
$cssis not a string, alongside the existingempty()check, preventing errors when a non-string value (e.g.null, array) is passed toWP_Theme_JSON::process_blocks_custom_css().## Proposed changes
! is_string()guard toWP_Theme_JSON::process_blocks_custom_css(), so the method returns an empty string when passed a non-string$cssvalue instead of attempting to process it.TypeErrors that would otherwise occur when a non-string value (e.g. an integer, an array) reaches the string functions (explode(),str_contains(), etc.) further down the method.## Why are these changes being made?
The existing
empty( $css )check only short-circuits falsey values. A non-empty, non-string value (e.g. an array) passed as$csswould still fall through toexplode( '&', $css )and trigger aTypeErroron PHP 8+. Guarding for! is_string()hardens the method against unexpected input from any caller and keeps it returning a predictable empty string. This data can come from a filter, and the method was also made public in 7.0.0 for thecustom-css blocksupport, so it's worth ensuring it degrades gracefully rather than fatalling on malformed input.Trac ticket: https://core.trac.wordpress.org/ticket/65608
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 4.8
Used for: Test case suggestions; implementation written by me, tests cases reviewed by me.