Make WordPress Core

Opened 17 hours ago

Last modified 16 hours ago

#66358 new defect (bug)

wp_json_encode() fallback ignores JsonSerializable::jsonSerialize()

Reported by: ekamran Owned by:
Priority: normal Milestone: Awaiting Review
Component: General Version: 4.1
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

When json_encode() fails, for example on a string that is not valid UTF-8, wp_json_encode() retries with the data passed through _wp_json_sanity_check(), which converts strings to UTF-8. That function does not use JsonSerializable::jsonSerialize(). It rebuilds each object from its public properties instead. An object that keeps its data in private properties is then encoded as {}.

<?php
class Example implements JsonSerializable {
        private $data = array( 'kind' => 'serialized' );

        public function jsonSerialize(): array {
                return $this->data;
        }
}

echo wp_json_encode( array( 'object' => new Example() ) ), "\n";
echo wp_json_encode(
        array(
                'object' => new Example(),
                'text'   => "caf\xE9", // Not valid UTF-8.
        )
), "\n";

Output on trunk:

{"object":{"kind":"serialized"}}
{"object":{},"text":"caf?"}

In core, this affects WP_Speculation_Rules, whose rules are in a private property, and WP_Secret, which serializes to a mask. WP_Interactivity_Dangerous_HTML works around it with getIterator() ([64501], #66246), and WP_Interactivity_API documents the current behavior where it resolves bound objects (#65740).

The fallback has worked this way since wp_json_encode() was added in [30055]. #37856 looked at the same code and was closed in favor of #47699.

Proposed change: in the retry, use the value each JsonSerializable object serializes to, as json_encode() does, with its strings converted to UTF-8. Rebuild an object from its public properties, as now, only when its serialized value still cannot be encoded: it holds INF, NAN or a resource, its jsonSerialize() throws, or objects serialize to each other in a cycle. When the retry still fails as a whole, for example when the data is deeper than $depth, run the current fallback unchanged. Data that json_encode() encodes on the first try is not affected.

Change History (1)

This ticket was mentioned in ​PR #14630 on ​WordPress/wordpress-develop by ​@ekamran.


16 hours ago
#1

  • Keywords has-patch has-unit-tests added

When json_encode() fails, for example on a string that is not valid UTF-8, wp_json_encode() retries through _wp_json_sanity_check(). That function ignores JsonSerializable::jsonSerialize() and rebuilds each object from its public properties, so an object that keeps its data in private properties is encoded as {}. In core, that affects WP_Speculation_Rules and WP_Secret.

This PR makes the retry use the value each JsonSerializable object serializes to, as json_encode() does, and keeps the current behavior wherever that value cannot be used.

What changes

  • wp_json_encode() now retries in two steps. The first uses _wp_json_sanity_check( $value, $depth, true ): strings are converted to UTF-8, and each JsonSerializable object is replaced with the value it serializes to. As in json_encode(), following an object to its serialized value costs no depth, and an object that serializes to itself is encoded from its public properties. If the first step still fails as a whole, for example when the data is deeper than $depth, the second step is the current fallback, unchanged.
  • In the first step, an object is rebuilt from its public properties on its own when its serialized value holds INF, NAN or a resource, when its jsonSerialize() throws, or when objects serialize to each other in a cycle. Other objects keep their serialized values.
  • _wp_json_sanity_check() gets a $use_json_serialize parameter that defaults to false, so direct callers of this private function get the same result as before.
  • Docs updated to match: the comment in WP_Interactivity_API that describes this fallback, the WP_Interactivity_Dangerous_HTML docblock, and the docblock of the test added for #65740.

Behavior differences

  1. In the fallback, a JsonSerializable object is encoded as its serialized value, with strings converted to UTF-8. Tests: test_uses_json_serialize_when_invalid_utf8_is_elsewhere_in_the_data, test_converts_invalid_utf8_returned_by_json_serialize, test_matches_json_encode_of_the_converted_data, test_secret_encodes_to_its_mask, test_speculation_rules_encode_to_their_rules, test_wp_bind_object_value_matches_the_client_store_when_other_state_is_not_valid_utf8, test_wp_bind_object_serializing_to_invalid_utf8_binds_the_converted_string.
  2. Only the objects whose serialized value cannot be encoded are rebuilt from their public properties. Before, every object was. Tests: test_rebuilds_only_the_object_whose_serialized_value_cannot_be_encoded, test_wp_bind_object_matches_the_client_store_when_another_object_cannot_be_serialized.
  3. The fallback now calls jsonSerialize(), once for each object it reaches, after json_encode() may already have called it. If it throws, that object is rebuilt from its public properties. Tests: test_fallback_calls_json_serialize_once_for_each_object_it_reaches, test_rebuilds_object_from_public_properties_when_json_serialize_throws, test_rebuilds_object_from_public_properties_when_json_serialize_throws_on_its_second_call.
  4. An object that implements both JsonSerializable and Traversable and serializes to itself is encoded from its public properties, as in json_encode(), instead of from its iterator. Test: test_traversable_object_serializing_to_itself_is_encoded_from_its_public_properties.
  5. When false is returned after the first step's json_encode() call failed, json_last_error() reports that call's error, which can differ from the error of the first json_encode() call. It never reports JSON_ERROR_NONE. Tests: test_json_last_error_reports_the_retry_error_when_rebuilding_exceeds_the_depth, test_returns_false_for_objects_serializing_to_each_other_through_public_properties.

Data that json_encode() encodes on the first try is not affected.

Performance

Only the fallback path changes. Measured with PHP 8.5 CLI, median of 5 runs, each data set holding one string that is not valid UTF-8:

Data trunk this PR
10,000 strings 1.84 ms 1.79 ms
10,000 integers 0.29 ms 0.63 ms
1,000 JsonSerializable objects 0.37 ms 1.09 ms
512 nested JsonSerializable objects ($depth 1032) 0.36 ms 0.37 ms

The worst case is quadratic: nested JsonSerializable objects whose serialized values all hold NAN, since each of them rebuilds its own subtree. At the default depth, 511 such levels took 56.6 ms against 0.25 ms on trunk, with the same output. When the public properties hold NAN too, so that the result is false, 512 levels took 84.6 ms against 0.36 ms (with $depth raised to 1032).

Testing

  • Counts below are test cases: one test method with one data set.
  • New tests/phpunit/tests/functions/wpJsonEncode.php (19 test methods, 29 test cases) and 4 new test methods with 11 test cases in tests/phpunit/tests/interactivity-api/wpInteractivityAPI-wp-bind.php, 40 new test cases in all. They cover the bug, WP_Secret, WP_Speculation_Rules, WP_Interactivity_Dangerous_HTML, the per-object fallback, cycles, throwing serializers, the number of jsonSerialize() calls, a comparison with json_encode() at depths 1, 2, 3 and 512, and Interactivity bindings against the hydrated store, including next to an object that serializes to INF and at the depth limit.
  • Against trunk's functions.php, 22 of the 40 new test cases fail and 18 pass. The 18 cover behavior that stays the same, such as objects whose serialized value holds INF, cycles, the default of the new parameter, the Dangerous HTML marker, and the depth limit.
  • Locally with PHP 8.3 and MySQL 9.7: the full PHPUnit suite has 0 failures and 0 errors, with the same 86 warnings and 70 skipped tests as trunk. PHPUnit reports 51 more tests than trunk (30,742 against 30,691) because the class in wpInteractivityAPI-wp-bind.php runs twice per suite run, on trunk as well, so its 11 new test cases count twice. PHPCS and PHPCompatibility on the changed files report nothing new, and PHPStan on the full project reports no errors.

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code and Codex
Used for: Investigation, code, tests, local testing and PR wording. I reviewed the reasoning, the code and the test results, and I take responsibility for the contribution.

Note: See TracTickets for help on using tickets.