From f6bba4fc88ac1656806913d0f54a92df25e9e139 Mon Sep 17 00:00:00 2001 From: Juanma Rodriguez Escriche Date: Thu, 2 Jul 2026 15:28:58 +0200 Subject: [PATCH] JSON API: preserve valid sub-400 WP_Error statuses in serializable_error() --- ...serializable-error-preserve-valid-statuses | 4 +++ projects/plugins/jetpack/class.json-api.php | 4 +-- ...WPCOM_JSON_API_Serializable_Error_Test.php | 36 ++++++++++--------- 3 files changed, 25 insertions(+), 19 deletions(-) create mode 100644 projects/plugins/jetpack/changelog/fix-json-api-serializable-error-preserve-valid-statuses diff --git a/projects/plugins/jetpack/changelog/fix-json-api-serializable-error-preserve-valid-statuses b/projects/plugins/jetpack/changelog/fix-json-api-serializable-error-preserve-valid-statuses new file mode 100644 index 00000000000..96c99a99450 --- /dev/null +++ b/projects/plugins/jetpack/changelog/fix-json-api-serializable-error-preserve-valid-statuses @@ -0,0 +1,4 @@ +Significance: patch +Type: bugfix + +JSON API: Only default a missing or invalid error status to 400, and stop overwriting valid HTTP status codes below 400 that callers return via WP_Error. diff --git a/projects/plugins/jetpack/class.json-api.php b/projects/plugins/jetpack/class.json-api.php index 928a884c55e..2f6fbe69ac8 100644 --- a/projects/plugins/jetpack/class.json-api.php +++ b/projects/plugins/jetpack/class.json-api.php @@ -775,12 +775,12 @@ public static function wrap_http_envelope( $status_code, $response, $content_typ */ public static function serializable_error( $error ) { - // Always serialize a valid HTTP error status >= 400 -- never 1, a non-integer, or any sub-400 (2xx/3xx) an app could read as success. + // A missing or non-numeric status resolves to 0 and defaults to 400. Valid HTTP codes, including sub-400 ones, are preserved. $data = $error->get_error_data(); $status_code = ( is_array( $data ) && isset( $data['status_code'] ) ) ? $data['status_code'] : $data; $status_code = is_numeric( $status_code ) ? (int) $status_code : 0; - if ( $status_code < 400 ) { + if ( ! $status_code ) { $status_code = 400; } $response = array( diff --git a/projects/plugins/jetpack/tests/php/json-api/WPCOM_JSON_API_Serializable_Error_Test.php b/projects/plugins/jetpack/tests/php/json-api/WPCOM_JSON_API_Serializable_Error_Test.php index dedfd4c9f57..3fb8176ac8a 100644 --- a/projects/plugins/jetpack/tests/php/json-api/WPCOM_JSON_API_Serializable_Error_Test.php +++ b/projects/plugins/jetpack/tests/php/json-api/WPCOM_JSON_API_Serializable_Error_Test.php @@ -15,8 +15,9 @@ require_once JETPACK__PLUGIN_DIR . 'class.json-api-endpoints.php'; /** - * Tests that serializable_error() always serializes a valid HTTP error status: never - * `1`, never a non-integer, and never a `< 400` status a client could read as success. + * Tests that serializable_error() extracts a valid integer status: never `1` and never a + * non-integer. A missing or invalid status defaults to 400, while valid HTTP status codes + * (including sub-400 redirect or success codes some callers use) are preserved. * * @covers \WPCOM_JSON_API::serializable_error * @covers \WPCOM_JSON_API @@ -38,8 +39,8 @@ private function status_for( $error ): int { } /** - * A valid status_code in the data passes through unchanged -- both the - * canonical array key and a bare-integer data value. + * A valid status_code in the data passes through unchanged, both as the + * canonical array key and as a bare-integer data value. */ public function test_valid_status_passes_through() { $this->assertSame( 404, $this->status_for( new WP_Error( 'not_found', 'Nope', array( 'status_code' => 404 ) ) ) ); @@ -101,34 +102,35 @@ public static function provide_non_numeric_scalars(): array { } /** - * A success/redirect status paired with an error must never render as `< 400` - * (the crash: an app reads a 2xx as a successful, URL-less site). + * A valid sub-400 status (a redirect or success code) carried on an error is preserved, + * not coerced to 400. Existing callers return WP_Error with codes such as 100, 200, and + * 302, and this function must not change their status. * - * @param int $input Non-error status carried on the error. - * @dataProvider provide_non_error_statuses + * @param int $input Valid sub-400 status carried on the error. + * @dataProvider provide_valid_sub_400_statuses */ - #[DataProvider( 'provide_non_error_statuses' )] - public function test_non_error_status_coerced_to_400( $input ) { - $this->assertSame( 400, $this->status_for( new WP_Error( 'oops', 'Oops', array( 'status_code' => $input ) ) ) ); + #[DataProvider( 'provide_valid_sub_400_statuses' )] + public function test_valid_sub_400_status_passes_through( $input ) { + $this->assertSame( $input, $this->status_for( new WP_Error( 'oops', 'Oops', array( 'status_code' => $input ) ) ) ); } /** - * Data provider: statuses a client could read as success. + * Data provider: valid sub-400 statuses that callers return via WP_Error. * * @return array */ - public static function provide_non_error_statuses(): array { + public static function provide_valid_sub_400_statuses(): array { return array( - '200 OK' => array( 200 ), - '201 Created' => array( 201 ), - '302 Found' => array( 302 ), + '100 Continue' => array( 100 ), + '200 OK' => array( 200 ), + '302 Found' => array( 302 ), ); } /** * Codes status_header() cannot render (Cloudflare 52x, other non-standard) are a valid * integer >= 400, so they pass through unchanged. Coercing them to a renderable status is - * deliberately NOT this function's job -- that belongs at the status_header() call site; + * deliberately not this function's job. That belongs at the status_header() call site; * here we only guarantee a sane integer. * * @param int $input Unknown-to-WP status carried on the error.