Fix a fatal error when binding non-scalar values, and static analysis issues, in WP_Interactivity_API

[westonruter]

Static analysis work on WP_Interactivity_API (Core-64898) surfaced a fatal error when data-wp-bind binds a non-scalar value to an HTML attribute (Core-65740). This fixes both, and closes the neighbouring gaps that fix brought to light.

The fatal error

Any non-scalar value reaching WP_HTML_Tag_Processor::set_attribute() hit a TypeError, because that method is typed string|bool and passes the value straight to the escaping functions. Both escaping paths failed, in different places:

Bound attribute	Failure
Ordinary, e.g. id	TypeError: strtr(): Argument #1 ($string) must be of type string, array given in WP_HTML_Tag_Processor::set_attribute()
URI, per wp_kses_uri_attributes(), e.g. href	TypeError: ltrim(): Argument #1 ($string) must be of type string, array given, from inside esc_url()

This is reachable from post content alone, with no plugin code, because data-wp-context can supply the array inline. Paste either of these into the post editor's code editor (⌥⌘M) to reproduce on trunk:

<!-- wp:accordion -->
<div class="wp-block-accordion"><span data-wp-interactive="myPlugin" data-wp-context='{"list":[1,2,3]}' data-wp-bind--id="context.list">Boom</span></div>
<!-- /wp:accordion -->

<!-- wp:accordion -->
<div class="wp-block-accordion"><span data-wp-interactive="myPlugin" data-wp-context='{"list":[1,2,3]}'><a data-wp-bind--href="context.list">Boom</a></span></div>
<!-- /wp:accordion -->

The core/accordion wrapper is needed only because directives are processed for blocks whose supports.interactivity is true; any of those works. The directives have to sit on a nested element, since the accordion's own render filter overwrites data-wp-interactive and data-wp-context on its wrapper.

data_wp_bind_processor() now rejects such a value with _doing_it_wrong() and leaves the attribute unset (removing it if it was already present, consistent with how a null value is handled), rather than taking down the whole page render.

Objects

The interesting case is objects, since the value bound on the server is also serialized into the client store and re-evaluated during hydration. Those two have to agree, and only the value the client receives can decide it — so an object is resolved by round-tripping it through wp_json_encode()/json_decode(), which is exactly how that store is built. Agreement is then structural rather than checked.

Object	Before	Client store	After
__toString() only	id="the-string-form"	{"prop":"prop value"}	notice, attribute unset
__toString() + jsonSerialize() agreeing	id="the-string-form"	"the-string-form"	id="the-string-form"
__toString() + jsonSerialize() differing	id="the-string-form"	{"not":"the string"}	notice, attribute unset
JsonSerializable, no __toString()	fatal	"the-string-form"	id="the-string-form"
serializing to another serializable object	notice, attribute unset	"the-inner-string"	id="the-inner-string"

Rows 1 and 3 rendered a value the client then overwrote with [object Object], since PHP coerces __toString() for the escaping functions' string parameters but JSON has no equivalent. Row 4 fataled even though the client store was already correct, purely because PHP cannot coerce an object without __toString(). Row 5 is why the resolution round-trips rather than calling jsonSerialize() once: a single call stops after one level, whereas the encoder keeps going, so the server rejected a value the browser was about to accept.

Resolving the object before the null check also means one serializing to null removes the attribute quietly, and the rest composes: a serialized number is formatted as below, a boolean keeps the boolean-attribute semantics.

Note that __toString() is never consulted. Row 3 is rejected because the object's own JSON opinion is what the browser will see.

Numbers

Numbers are formatted by the same encoder rather than cast to string, which closes two more divergences from the client. Measured on PHP 7.4:

value	(string)	wp_json_encode()	JS String(v)
1.5 under LC_NUMERIC=de_DE	1,5	1.5	1.5
1/3	0.33333333333333	0.3333333333333333	0.3333333333333333

Casting a float is locale-dependent before PHP 8.0 (RFC), and 7.4 is the minimum supported version. The cast also rounds to precision where the store uses serialize_precision. Routing both through one function makes them agree whatever those settings are. Integers were never affected, as only floats consult the locale.

Neither is a regression from the fix above — the cast is the same conversion set_attribute() was already applying implicitly, one frame later — but both are worth closing while the line is being touched.

INF and NAN

These are scalars, so they passed the check above, but JSON can represent neither. wp_json_encode() on the store returns false, and WP_Script_Modules::print_script_module_data() casts that failure to a string, so the client is served an empty <script type="application/json"> in place of all of its state: every binding on the page loses its value, not only this one. The attribute meanwhile received INF or NAN, which means nothing as an attribute value and does not match the client either, where the same numbers render as Infinity and NaN.

They are now rejected too, with their own message, since describing them as non-scalar would be wrong. Rejecting the binding only draws attention to the problem; removing the value from the state is what resolves it.

Static analysis

The rest is PHPStan level 10 work on the same code path, which is how the missing type checks came to light:

• WP_Interactivity_API::$directive_processors gains an array shape whose values are the literal method names. Without it, the dynamic dispatch through call_user_func_array() is unresolvable, so all eight data_wp_*_processor() methods were reported unused and the callable array was reported as not callable. Ten errors, no ignores.
• get_directive_entries() declares its return type and array shape, and now skips a directive name it cannot parse instead of destructuring null. The get_attribute() null check is defensive: the attribute names come from get_attribute_names_with_prefix(), so it should not be reachable.
• parse_directive_name() documents its shape and guarantees a non-empty prefix. The character check became an anchored whitelist, and a name beginning with -- no longer yields an empty prefix. Neither changes the outcome for a directive that was previously matched — an empty prefix matched no known directive either way — but the return type is now honest.
• extract_directive_value() documents its tuple shape.
• WP_HTML_Tag_Processor::$attributes and get_attribute_names_with_prefix() gain @phpstan- annotations only. No functional change.
• data_wp_bind_processor() gains a void return type.

Testing instructions

npm run test:php -- --group interactivity-api

New coverage in tests/phpunit/tests/interactivity-api/wpInteractivityAPI-wp-bind.php. Non-scalar values are exercised as a cross product with the attribute bound, so the strtr() and esc_url() escaping paths each fail on their own data set rather than one masking the other, and each asserts the exact _doing_it_wrong() message rather than only that it was triggered. There is also a test asserting that the rendered attribute value appears verbatim in the JSON sent to the client, which is the invariant the object handling rests on.

The locale test skips itself where no comma-decimal locale is installed, so it will not fail a runner without one.

To confirm the tests would catch a regression, remove the is_scalar() block from data_wp_bind_processor() and re-run: 32 errors and 8 failures across the 40 data sets.

Verified against PHP 7.4 as well as 8.3, since the float formatting behaves differently between them.

Follow-ups, not addressed here

• state() is the better place to catch a non-finite float. Binding is only where you happen to notice a store which is already broken, and one such value anywhere in the state discards the whole of it.
• _process_directives() restores $context_stack and $namespace_stack only on the unbalanced-tags path, not when an exception is thrown mid-traversal. A caught fatal therefore corrupts every later process_directives() call on the same instance. Moot for this bug now that the throw is prevented, but it is a real fragility.
• The is_array() branch in the directive dispatch in _process_directives() is unreachable: nothing writes to $directive_processors, so no value is ever an array callable.

Trac ticket: https://core.trac.wordpress.org/ticket/65740
Trac ticket: https://core.trac.wordpress.org/ticket/64898

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for:

• Troubleshooting PHPStan complaining that the data_wp_bind_processor method was unused. See 1638ca3.
• Writing tests.
• Resolving a bound object to whatever it serializes to, and the wording of the _doing_it_wrong() messages.
• Implementing and verifying the number formatting. Using the JSON encoder for it was my own approach, prompted by @dmsnell raising the locale question in review.
• Rejecting INF and NAN, after I asked whether they belonged with the other values which cannot reach the client.
• Drafting this description. The behavior tables and the reproduction snippets above were verified against a local install rather than asserted.

---

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.

[westonruter]

@dmsnell How does this look to you?

[dmsnell]

my only immediate response is questioning if passing numbers here is common, and whether we should call _doing_it_wrong() in these cases, if we allow them at all.

I’m reminded of some stringifying issues with CSS numbers, whereas the conversion is locale-dependent.

that is, would we want to use false|null, true, and string values only and check is_string()?

[westonruter]

My intention was to not cause anything to fail which previously passed through. There was already the case for \Tests_WP_Interactivity_API_WP_Bind::test_wp_bind_sets_number_value() for an int value, ensuring it is implicitly coerced to string. There just wasn't the same for a float.

And now in e6d704c it also handles JsonSerializable.

[westonruter]

In regards to floats, it's news to me that PHP would use the locale to when coercing floats to strings. However, this behavior was eliminated in PHP 8 so the JSON-compatible representation is always used: https://php.watch/versions/8.0/float-to-string-locale-independent

But I've addressed it in b292380 by using wp_json_encode() to convert an int and a float to a string. This bypasses locale-specific formatting.

In the course of doing that, I discovered that INF and NAN are additional scalar values in PHP which cannot be expressed in JSON. Fixed in 1791d03 and d15a50e.

My goal here is to make sure that there is no back-compat breakage, where int and float previously worked.

[Copilot]

Pull request overview

This PR tightens up types and edge-case handling in the Interactivity API’s directive parsing/collection path and hardens data_wp_bind_processor() against non-string attribute values, addressing static analysis concerns around nullable returns and value types.

Changes:

• Adds PHPStan/typed-property annotations for directive processor maps and directive parsing/entry extraction return shapes.
• Makes directive parsing/entry extraction more defensive (e.g., handling empty substrings, null attribute lists/values, invalid directive names).
• Ensures data-wp-bind only writes scalar attribute values (casting numbers to strings and rejecting non-scalars with _doing_it_wrong()).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/wp-includes/interactivity-api/class-wp-interactivity-api.php	Adds stricter typing and defensive checks for directive parsing/entry extraction; hardens data-wp-bind attribute writes against non-scalar values.
src/wp-includes/html-api/class-wp-html-tag-processor.php	Adds PHPStan annotations clarifying attribute map and get_attribute_names_with_prefix() return types.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Test using WordPress Playground

The 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

• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[dmsnell]

noting that pre-PHP-8.0 this would have returned false if the directive name was shorter than 8 bytes.

👍

[github-actions]

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 props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props westonruter, luisherranz, dmsnell, darerodz.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[westonruter]

The inclusion of 0 === $suffix_index here is to guarantee that when this method never returns an empty string for the prefix. This seems like it should never happen.

[westonruter]

The '' === $name check here is to ensure that this never returns an empty string. Perhaps it would be better like this, though, as an allowlist for the entire string:

Suggested change

	if ( '' === $name || preg_match( '/[^a-z0-9\-_]/i', $name ) ) {
	if ( 1 !== preg_match( '/^[a-z0-9\-_]+$/i', $name ) ) {

[westonruter]

Applied in a7a46b6 and b1cb905.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/wp-includes/interactivity-api/class-wp-interactivity-api.php:1176

• The non-finite-float _doing_it_wrong() message says “INF or NAN”, but this branch is also triggered for -INF (since wp_json_encode() fails for both infinities). This makes the message slightly inaccurate/misleading for negative infinity cases. Consider updating the wording to include -INF (or to describe “non-finite values” more generally) and adjust the corresponding test expectations.

								sprintf(
									/* translators: %s: The attribute name. */
									__( 'Attempted to bind INF or NAN to the "%s" attribute. JSON can represent neither, so the client is sent no state at all. Cast the value to a string before storing it in state or context.' ),
									esc_html( $entry['suffix'] )

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

[DAreRodz]

This fix makes sense. Thanks, @westonruter. 🙏

I have read through the code, and the changes look good to me. I haven't had time to test it, though.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

[luisherranz]

Hey @westonruter, thanks for working on this 🙂

In general it looks good, although this assumes the resolved value of a directive can only be something contained in state or context, but in reality it can also be derived state returned by a PHP closure.

The value the closure returns never reaches the client in any form. The store receives {} for that prop (the closure degrades to an empty object inside wp_json_encode()) plus its path in derivedStateClosures. The client value comes exclusively from the JS getter.

I'm not suggesting the behavior should be different for derived state. I think it should be the same either way, and rejecting non-scalar values uniformly seems right to me, since an array from a closure also fatals on trunk. With that in mind, I see two separate things in this PR.

The messages assume the stored case (state or context), and for closures they say things that aren't accurate.

1. "Cast the value to a string before storing it in state or context" cannot be followed when the value comes from a closure, because nothing is stored. The fix there is to return a string from the closure.
2. "JSON can represent neither, so the client is sent no state at all" is only true when the INF or NAN sits in stored state. Coming from a closure, it never enters the store, and the store encodes fine.

The change from __toString() to serialization for objects: The serialization reasoning covers stored values (state or context), where the value the client ends up with really is the serialized form, so resolving through the encoder computes exactly what the client will have. But for derived state there's no serialization involved at all, and the server can never know what the JS getter returns. Agreement there is the developer's responsibility for every value. This PR now rejects closures returning stringable objects, which is not backward compatible. The surface is very small, so I don't have a strong opinion on whether it should change or not. I just wanted to point it out to make the decision with that case in mind.

Another option to consider would be to replicate in PHP what JavaScript's String() does. An object becomes [object Object], an array becomes a comma join, and INF and NAN become "Infinity" and "NaN", although I'm not sure if, especially the [object Object] part, would make much sense.

[westonruter]

@luisherranz Would you please push up commits to this branch that apply your suggestions?

Another option to consider would be to replicate in PHP what JavaScript's String() does. An object becomes [object Object], an array becomes a comma join, and INF and NAN become "Infinity" and "NaN", although I'm not sure if, especially the [object Object] part, would make much sense.

IMO, it would be better to have a warning so that the author would be more readily discover their usage error.

[luisherranz]

I've pushed the updated messages and comments to take into account that those values can also come from derived state closures, and I leave it up to your decision what to do with the reject of closures returning stringable objects.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (7)

tests/phpunit/tests/interactivity-api/wpInteractivityAPI-wp-bind.php:659

• This expected _doing_it_wrong() message says the bound value must resolve to a string, but the implementation allows any scalar. Update the expected message if the core notice is revised to mention “scalar value”.

				'WP_Interactivity_API::data_wp_bind_processor' => 'Attempted to bind a non-scalar value to the "id" attribute. Ensure the state/context property or the derived state closure resolves to a string. (This message was added in version 7.1.0.)',

tests/phpunit/tests/interactivity-api/wpInteractivityAPI-wp-bind.php:697

• This expected _doing_it_wrong() message says the bound value must resolve to a string, but the implementation allows any scalar. Update the expected message if the core notice is revised to mention “scalar value”.

				'WP_Interactivity_API::data_wp_bind_processor' => 'Attempted to bind a non-scalar value to the "id" attribute. Ensure the state/context property or the derived state closure resolves to a string. (This message was added in version 7.1.0.)',

tests/phpunit/tests/interactivity-api/wpInteractivityAPI-wp-bind.php:948

• If the core _doing_it_wrong() string is updated to accurately describe accepted values (scalar, not just string), the expected string in this sprintf() should be updated to match to avoid brittle test failures.

					'Attempted to bind a non-scalar value to the "%s" attribute. Ensure the state/context property or the derived state closure resolves to a string. (This message was added in version 7.1.0.)',

tests/phpunit/tests/interactivity-api/wpInteractivityAPI-wp-bind.php:984

• If the core _doing_it_wrong() string is updated to accurately describe accepted values (scalar, not just string), the expected string in this sprintf() should be updated to match to avoid brittle test failures.

					'Attempted to bind a non-scalar value to the "%s" attribute. Ensure the state/context property or the derived state closure resolves to a string. (This message was added in version 7.1.0.)',

src/wp-includes/interactivity-api/class-wp-interactivity-api.php:1161

• The _doing_it_wrong() message says the bound value must resolve to a string, but this code path explicitly allows all scalar values (string|bool|int|float, with non-finite floats rejected separately). This wording is misleading for developers debugging the notice; consider referring to a “scalar value” instead.

								__( 'Attempted to bind a non-scalar value to the "%s" attribute. Ensure the state/context property or the derived state closure resolves to a string.' ),

src/wp-includes/interactivity-api/class-wp-interactivity-api.php:1193

• The non-finite number notice currently says the value must resolve to a string, but finite numbers are valid and handled (formatted via wp_json_encode()). Adjusting the message to indicate “finite number or string” will make the guidance accurate.

									__( 'Attempted to bind INF or NAN to the "%s" attribute. Ensure the state/context property or the derived state closure resolves to a string.' ),

tests/phpunit/tests/interactivity-api/wpInteractivityAPI-wp-bind.php:530

• This test hard-codes the _doing_it_wrong() message text. If the core notice is updated to accurately allow finite numbers (not only strings), the expected string here should be updated to match.

This issue also appears in the following locations of the same file:

• line 659
• line 697
• line 948
• line 984

				'WP_Interactivity_API::data_wp_bind_processor' => 'Attempted to bind INF or NAN to the "data-ratio" attribute. Ensure the state/context property or the derived state closure resolves to a string. (This message was added in version 7.1.0.)',