Make WordPress Core

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:

  1. The VALID_STYLES schema marks CSS as null, which means remove_keys_not_in_schema() accepts any type for this key - arrays and integers pass through schema sanitization unchanged.
  2. 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 reach process_blocks_custom_css().
  3. The method was made public static in 7.0 (changeset 61678) for use by the custom-css block support, 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

Return early when $css is not a string, alongside the existing empty() check, preventing errors when a non-string value (e.g. null, array) is passed to WP_Theme_JSON::process_blocks_custom_css().

## Proposed changes

  • Adds an ! is_string() guard to WP_Theme_JSON::process_blocks_custom_css(), so the method returns an empty string when passed a non-string $css value instead of attempting to process it.
  • Prevents PHP warnings/fatal 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 $css would still fall through to explode( '&', $css ) and trigger a TypeError on 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 the custom-css block support, 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.

#2 @wildworks
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.

https://developer.wordpress.org/reference/classes/wp_theme_json/process_blocks_custom_css/#parameters

#3 @hannahtinkler
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.

Note: See TracTickets for help on using tickets.