-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Fix a fatal error when binding non-scalar values, and static analysis issues, in WP_Interactivity_API
#12725
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?
Changes from all commits
9085345
95b14c6
1638ca3
39cb3d1
dc7cdae
359638a
7caf72d
2e90b5d
ba5e81e
44cfc28
56026f1
abe8baf
e6d704c
1cc4275
921d90d
ae99fce
77ec31f
b292380
1791d03
d15a50e
41486fb
33b39fc
0b83feb
906481e
91a712a
6324a02
3e82857
a7a46b6
b1cb905
dc521cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,9 +17,19 @@ final class WP_Interactivity_API { | |
| * Holds the mapping of directive attribute names to their processor methods. | ||
| * | ||
| * @since 6.5.0 | ||
| * @var array | ||
| * @var array<string, string> | ||
| * @phpstan-var array{ | ||
| * 'data-wp-interactive': 'data_wp_interactive_processor', | ||
| * 'data-wp-router-region': 'data_wp_router_region_processor', | ||
| * 'data-wp-context': 'data_wp_context_processor', | ||
| * 'data-wp-bind': 'data_wp_bind_processor', | ||
| * 'data-wp-class': 'data_wp_class_processor', | ||
| * 'data-wp-style': 'data_wp_style_processor', | ||
| * 'data-wp-text': 'data_wp_text_processor', | ||
| * 'data-wp-each': 'data_wp_each_processor', | ||
| * } | ||
| */ | ||
| private static $directive_processors = array( | ||
| private static array $directive_processors = array( | ||
| 'data-wp-interactive' => 'data_wp_interactive_processor', | ||
| 'data-wp-router-region' => 'data_wp_router_region_processor', | ||
| 'data-wp-context' => 'data_wp_context_processor', | ||
|
|
@@ -99,8 +109,17 @@ final class WP_Interactivity_API { | |
| * | ||
| * This is only available during directive processing, otherwise it is `null`. | ||
| * | ||
| * An entry is the namespace the directive defined. It is `false` instead when | ||
| * the directive did not define a usable one — the attribute was empty, or its | ||
| * JSON held no `namespace`, or the namespace did not match the accepted | ||
| * characters — and no enclosing `data-wp-interactive` was in effect to inherit | ||
| * from. An entry is pushed either way, because one is popped for every closing | ||
| * tag regardless of what the directive contained, so `false` is what stands in | ||
| * for "no namespace here" and keeps the stack balanced. | ||
| * | ||
| * @since 6.6.0 | ||
| * @var array<string>|null | ||
| * @var array<string|false>|null | ||
| * @phpstan-var list<string|false>|null | ||
| */ | ||
| private $namespace_stack = null; | ||
|
|
||
|
|
@@ -764,21 +783,26 @@ private function evaluate( $entry ) { | |
| * @since 6.9.0 | ||
| * | ||
| * @param string $directive_name The directive attribute name. | ||
| * @return array An array containing the directive prefix, optional suffix, and optional unique ID. | ||
| * @return array|null An array containing the directive prefix, optional suffix, and optional unique ID, or null if the directive name cannot be parsed. | ||
| * @phpstan-return array{ | ||
| * prefix: non-empty-string, | ||
| * suffix: string|null, | ||
| * unique_id: string|null, | ||
| * }|null | ||
| */ | ||
| private function parse_directive_name( string $directive_name ): ?array { | ||
| // Remove the first 8 characters (assumes "data-wp-" prefix) | ||
| $name = substr( $directive_name, 8 ); | ||
| $name = (string) substr( $directive_name, 8 ); | ||
|
|
||
| // Check for invalid characters (anything not a-z, 0-9, -, or _) | ||
| if ( preg_match( '/[^a-z0-9\-_]/i', $name ) ) { | ||
| // Ensure the name only contains valid characters (anything a-z, A-Z, 0-9, -, or _). | ||
| if ( 1 !== preg_match( '/^[a-zA-Z0-9\-_]+$/', $name ) ) { | ||
| return null; | ||
| } | ||
|
|
||
| // Find the first occurrence of '--' to separate the prefix | ||
| // Find the first occurrence of '--' to separate the prefix, as long as it is not at the beginning of the string. | ||
| $suffix_index = strpos( $name, '--' ); | ||
|
|
||
| if ( false === $suffix_index ) { | ||
| if ( false === $suffix_index || 0 === $suffix_index ) { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The inclusion of |
||
| return array( | ||
| 'prefix' => $name, | ||
| 'suffix' => null, | ||
|
|
@@ -794,7 +818,7 @@ private function parse_directive_name( string $directive_name ): ?array { | |
| return array( | ||
| 'prefix' => $prefix, | ||
| 'suffix' => null, | ||
| 'unique_id' => '---' !== $remaining ? substr( $remaining, 3 ) : null, | ||
| 'unique_id' => '---' !== $remaining ? (string) substr( $remaining, 3 ) : null, | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -846,6 +870,7 @@ private function parse_directive_name( string $directive_name ): ?array { | |
| * @param string|null $default_namespace Optional. The default namespace if none is explicitly defined. | ||
| * @return array An array containing the namespace in the first item and the JSON, the reference path, or null on the | ||
| * second item. | ||
| * @phpstan-return array{ 0: string|null, 1: mixed } | ||
| */ | ||
| private function extract_directive_value( $directive_value, $default_namespace = null ): array { | ||
| if ( empty( $directive_value ) || is_bool( $directive_value ) ) { | ||
|
|
@@ -878,17 +903,46 @@ private function extract_directive_value( $directive_value, $default_namespace = | |
| * @param WP_Interactivity_API_Directives_Processor $p The directives processor instance. | ||
| * @param string $prefix The directive prefix to filter by. | ||
| * @return array An array of entries containing the directive namespace, value, suffix, and unique ID. | ||
| * @phpstan-return list<array{ | ||
| * namespace: string|null, | ||
| * value: mixed, | ||
| * suffix: string|null, | ||
| * unique_id: string|null, | ||
| * }> | ||
| */ | ||
| private function get_directive_entries( WP_Interactivity_API_Directives_Processor $p, string $prefix ) { | ||
| private function get_directive_entries( WP_Interactivity_API_Directives_Processor $p, string $prefix ): array { | ||
| $directive_attributes = $p->get_attribute_names_with_prefix( 'data-wp-' . $prefix ); | ||
| $entries = array(); | ||
| if ( null === $directive_attributes ) { | ||
| return array(); | ||
| } | ||
|
|
||
| $entries = array(); | ||
| foreach ( $directive_attributes as $attribute_name ) { | ||
| [ 'prefix' => $attr_prefix, 'suffix' => $suffix, 'unique_id' => $unique_id] = $this->parse_directive_name( $attribute_name ); | ||
| $parsed_directive = $this->parse_directive_name( $attribute_name ); | ||
| if ( null === $parsed_directive ) { | ||
| continue; | ||
| } | ||
|
|
||
| [ 'prefix' => $attr_prefix, 'suffix' => $suffix, 'unique_id' => $unique_id ] = $parsed_directive; | ||
| // Ensure it is the desired directive. | ||
| if ( $prefix !== $attr_prefix ) { | ||
| continue; | ||
| } | ||
| list( $namespace, $value ) = $this->extract_directive_value( $p->get_attribute( $attribute_name ), end( $this->namespace_stack ) ); | ||
| $attribute_value = $p->get_attribute( $attribute_name ); | ||
| if ( null === $attribute_value ) { | ||
| continue; | ||
| } | ||
| /* | ||
| * The namespace stack can hold false, which data_wp_interactive_processor() pushes for a | ||
| * `data-wp-interactive` whose namespace is invalid and which has no enclosing one to inherit. Only a | ||
| * string names a store, so anything else counts as no default namespace at all. | ||
| */ | ||
| $default_namespace = array_last( $this->namespace_stack ?? array() ); | ||
| if ( ! is_string( $default_namespace ) ) { | ||
| $default_namespace = null; | ||
| } | ||
|
|
||
| list( $namespace, $value ) = $this->extract_directive_value( $attribute_value, $default_namespace ); | ||
| $entries[] = array( | ||
| 'namespace' => $namespace, | ||
| 'value' => $value, | ||
|
|
@@ -1002,6 +1056,15 @@ private function data_wp_context_processor( WP_Interactivity_API_Directives_Proc | |
| continue; | ||
| } | ||
|
|
||
| /* | ||
| * A context with no namespace has nothing to be stored under, so the inherited context is left as it | ||
| * is. Using the namespace as an array key regardless would coerce null to an empty string, which PHP | ||
| * 8.5 deprecates, and would store the context where no reference can address it anyway. | ||
| */ | ||
| if ( null === $entry['namespace'] ) { | ||
| continue; | ||
| } | ||
|
|
||
| $context = array_replace_recursive( | ||
| $context, | ||
| array( $entry['namespace'] => is_array( $entry['value'] ) ? $entry['value'] : array() ) | ||
|
|
@@ -1017,16 +1080,19 @@ private function data_wp_context_processor( WP_Interactivity_API_Directives_Proc | |
| * associated reference. | ||
| * | ||
| * @since 6.5.0 | ||
| * @since 7.1.0 An object is resolved to whatever it serializes to for the client, a number is formatted by the | ||
| * JSON encoder, and a value which cannot be sent to the client is rejected rather than passed to | ||
| * WP_HTML_Tag_Processor::set_attribute(). | ||
| * | ||
| * @param WP_Interactivity_API_Directives_Processor $p The directives processor instance. | ||
| * @param string $mode Whether the processing is entering or exiting the tag. | ||
| */ | ||
| private function data_wp_bind_processor( WP_Interactivity_API_Directives_Processor $p, string $mode ) { | ||
| private function data_wp_bind_processor( WP_Interactivity_API_Directives_Processor $p, string $mode ): void { | ||
| if ( 'enter' === $mode ) { | ||
| $entries = $this->get_directive_entries( $p, 'bind' ); | ||
| foreach ( $entries as $entry ) { | ||
| if ( empty( $entry['suffix'] ) || null !== $entry['unique_id'] ) { | ||
| continue; | ||
| continue; | ||
| } | ||
|
|
||
| // Skip if the suffix is an event handler. | ||
|
|
@@ -1045,6 +1111,97 @@ private function data_wp_bind_processor( WP_Interactivity_API_Directives_Process | |
|
|
||
| $result = $this->evaluate( $entry ); | ||
|
|
||
| /* | ||
| * An object is resolved to whatever it serializes to. When the reference points to a value stored | ||
| * in state or context, that is the value the client receives for it when the store is hydrated. | ||
| * A derived state closure is never serialized, so there the client value comes from the derived | ||
| * state's client-side implementation instead; the resolution is still applied so that both origins | ||
| * behave the same. Round-tripping through the JSON encoder rather than calling | ||
| * JsonSerializable::jsonSerialize() directly keeps this resolution identical to the client's, | ||
| * including for an object which serializes to another serializable object. When the encoding fails | ||
| * the object is left in place, to be reported as a usage error below. Note that it rarely does | ||
| * fail: wp_json_encode() retries through _wp_json_sanity_check(), which rebuilds the object from | ||
| * its public properties and so ignores jsonSerialize() altogether. An object whose serialized form | ||
| * JSON cannot represent therefore resolves to whatever that rebuild encodes to, which is what the | ||
| * client is sent for it as well. | ||
| * | ||
| * A throwing JsonSerializable::jsonSerialize() is caught for the same reason the value is checked | ||
| * at all: a binding must not be able to abort the render. An exception escaping here would leave | ||
| * `$context_stack` and `$namespace_stack` unrestored for every later `process_directives()` call | ||
| * on this instance, so the object is treated as one which failed to encode. | ||
| */ | ||
| if ( is_object( $result ) ) { | ||
| try { | ||
| $encoded = wp_json_encode( $result ); | ||
| } catch ( Throwable $e ) { | ||
| $encoded = false; | ||
| } | ||
| if ( false !== $encoded ) { | ||
| $result = json_decode( $encoded ); | ||
| } | ||
| } | ||
|
|
||
| /* | ||
| * Only a value which can be sent to the client may be stored in an attribute value. Strings and | ||
| * booleans are passed in as-is, numbers are formatted, and everything else is rejected as a usage | ||
| * error. | ||
| * | ||
| * An object which does not serialize to a scalar is rejected even when it defines `__toString()`, | ||
| * which PHP would otherwise coerce for the string parameters of the escaping functions. Its string | ||
| * representation is not what the client evaluates this reference to, whether that is the form | ||
| * serialized into the store or the return value of a derived state's client-side implementation, | ||
| * so the two could disagree once the directive is evaluated during hydration. | ||
| */ | ||
| if ( null !== $result ) { | ||
| if ( ! is_scalar( $result ) ) { | ||
| _doing_it_wrong( | ||
| __METHOD__, | ||
| sprintf( | ||
| /* translators: %s: The attribute name. */ | ||
| __( '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.' ), | ||
| esc_html( $entry['suffix'] ) | ||
| ), | ||
| '7.1.0' | ||
| ); | ||
| $result = null; | ||
| } elseif ( is_int( $result ) || is_float( $result ) ) { | ||
| /* | ||
| * A number is formatted by the JSON encoder rather than cast to string, so that the | ||
| * attribute value matches the number the client receives for this same reference. Casting | ||
| * a float is locale-dependent before PHP 8.0, and rounds to `precision` rather than to the | ||
| * encoder's `serialize_precision`. | ||
| * | ||
| * This closes the cases which differ in practice, not every one. A float written in | ||
| * exponent notation still disagrees, since PHP encodes 1e25 as `1.0e+25` where JavaScript | ||
| * renders it as `1e+25`, as does negative zero, and an integer above the range JavaScript | ||
| * can represent exactly is rounded once it reaches the client. Casting diverged on all | ||
| * three as well, so none is a regression. | ||
| */ | ||
| $encoded = wp_json_encode( $result ); | ||
| if ( false === $encoded ) { | ||
| /* | ||
| * The encoder only rejects INF and NAN, of which JSON can represent neither. When such | ||
| * a value is stored in state, the store itself also fails to encode in its entirety, | ||
| * and the client is sent an empty script tag in place of all of its state; only | ||
| * removing the value from the state resolves that. A derived state closure returning | ||
| * one never reaches the store, so there only the binding itself is affected. | ||
| */ | ||
| _doing_it_wrong( | ||
| __METHOD__, | ||
| sprintf( | ||
| /* translators: %s: The attribute name. */ | ||
| __( 'Attempted to bind INF or NAN to the "%s" attribute. Ensure the state/context property or the derived state closure resolves to a string.' ), | ||
| esc_html( $entry['suffix'] ) | ||
| ), | ||
| '7.1.0' | ||
| ); | ||
| $result = null; | ||
| } else { | ||
| $result = $encoded; | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+1144
to
+1203
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dmsnell How does this look to you?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my only immediate response is questioning if passing numbers here is common, and whether we should call I’m reminded of some stringifying issues with CSS numbers, whereas the conversion is locale-dependent. that is, would we want to use
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My intention was to not cause anything to fail which previously passed through. There was already the case for And now in e6d704c it also handles
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 In the course of doing that, I discovered that My goal here is to make sure that there is no back-compat breakage, where |
||
|
|
||
| if ( | ||
| null !== $result && | ||
| ( | ||
|
|
||
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.
noting that pre-PHP-8.0 this would have returned
falseif the directive name was shorter than 8 bytes.👍