From 907fb90ee0da9dfed7574d763e2685b71ee95b7e Mon Sep 17 00:00:00 2001 From: Hinnerk Altenburg Date: Fri, 18 Sep 2026 09:49:36 +0200 Subject: [PATCH] HTTP API: Use the actual HTTP status code in `download_url()` error codes. `download_url()` returned a `WP_Error` with the code `http_404` for every non-200 response, so a 403, 406 or 500 was indistinguishable from a real 404 without digging into the error data. The error code now reflects the actual status code (`http_403`, `http_500`, ...), matching how Site Health names the same kind of error. A response without a status code keeps `http_404`. Trac ticket: https://core.trac.wordpress.org/ticket/60564 --- src/wp-admin/includes/file.php | 11 ++- tests/phpunit/tests/admin/includesFile.php | 104 +++++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/src/wp-admin/includes/file.php b/src/wp-admin/includes/file.php index ef5a5025bef22..4834a890a486a 100644 --- a/src/wp-admin/includes/file.php +++ b/src/wp-admin/includes/file.php @@ -1150,6 +1150,9 @@ function wp_handle_sideload( &$file, $overrides = false, $time = null ) { * @since 2.5.0 * @since 5.2.0 Signature Verification with SoftFail was added. * @since 5.9.0 Support for Content-Disposition filename was added. + * @since 7.2.0 The error code for a non-200 response now reflects the actual + * HTTP status code (`http_403`, `http_500`, ...) instead of always + * being `http_404`. * * @param string $url The URL of the file to download. * @param int $timeout The timeout for the request to download the file. @@ -1217,7 +1220,13 @@ function download_url( $url, $timeout = 300, $signature_verification = false ) { unlink( $tmpfname ); - return new WP_Error( 'http_404', trim( wp_remote_retrieve_response_message( $response ) ), $data ); + /* + * Name the error after the actual status code, e.g. `http_403` or `http_500`. + * A response without a status code keeps the historical `http_404`. + */ + $error_code = $response_code ? 'http_' . $response_code : 'http_404'; + + return new WP_Error( $error_code, trim( wp_remote_retrieve_response_message( $response ) ), $data ); } $content_disposition = wp_remote_retrieve_header( $response, 'Content-Disposition' ); diff --git a/tests/phpunit/tests/admin/includesFile.php b/tests/phpunit/tests/admin/includesFile.php index fab002c5e5e2c..dc3d408c3b633 100644 --- a/tests/phpunit/tests/admin/includesFile.php +++ b/tests/phpunit/tests/admin/includesFile.php @@ -36,6 +36,7 @@ public function test_get_home_path() { /** * @ticket 43329 + * @ticket 60564 * * @covers ::download_url */ @@ -45,6 +46,7 @@ public function test_download_url_non_200_response_code() { $error = download_url( 'test_download_url_non_200' ); $this->assertWPError( $error ); + $this->assertSame( 'http_418', $error->get_error_code() ); $this->assertSame( array( 'code' => 418, @@ -84,6 +86,108 @@ public function __return_5() { return 5; } + /** + * Tests that the error code of a non-200 response reflects the actual HTTP status code. + * + * @ticket 60564 + * @dataProvider data_download_url_should_use_the_response_code_in_the_error_code + * + * @covers ::download_url + * + * @param int|string $response_code The HTTP status code returned by the mocked request. + * @param string $expected_error_code The expected `WP_Error` code. + */ + public function test_download_url_should_use_the_response_code_in_the_error_code( $response_code, $expected_error_code ) { + $filter = static function ( $response, $parsed_args ) use ( $response_code ) { + file_put_contents( $parsed_args['filename'], 'Error body' ); + + return array( + 'response' => array( + 'code' => $response_code, + 'message' => 'Error message', + ), + ); + }; + + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $error = download_url( 'test_download_url_non_200' ); + + remove_filter( 'pre_http_request', $filter ); + + $this->assertWPError( $error ); + $this->assertSame( $expected_error_code, $error->get_error_code() ); + $this->assertSame( 'Error message', $error->get_error_message() ); + $this->assertSame( + array( + 'code' => $response_code, + 'body' => 'Error body', + ), + $error->get_error_data() + ); + } + + /** + * Data provider for test_download_url_should_use_the_response_code_in_the_error_code(). + * + * @return array[] + */ + public function data_download_url_should_use_the_response_code_in_the_error_code() { + return array( + '403 Forbidden' => array( + 'response_code' => 403, + 'expected_error_code' => 'http_403', + ), + '404 Not Found' => array( + 'response_code' => 404, + 'expected_error_code' => 'http_404', + ), + '406 Not Acceptable' => array( + 'response_code' => 406, + 'expected_error_code' => 'http_406', + ), + '500 Internal Server Error' => array( + 'response_code' => 500, + 'expected_error_code' => 'http_500', + ), + '503 Service Unavailable' => array( + 'response_code' => 503, + 'expected_error_code' => 'http_503', + ), + ); + } + + /** + * Tests that a response without a status code keeps the historical `http_404` error code. + * + * A `pre_http_request` filter may short-circuit the request with a response + * that carries no `response` array at all, in which case + * `wp_remote_retrieve_response_code()` returns an empty string. + * + * @ticket 60564 + * + * @covers ::download_url + */ + public function test_download_url_should_fall_back_to_http_404_without_a_response_code() { + $filter = static function ( $response, $parsed_args ) { + file_put_contents( $parsed_args['filename'], 'Error body' ); + + return array( + 'headers' => array(), + 'body' => '', + ); + }; + + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $error = download_url( 'test_download_url_non_200' ); + + remove_filter( 'pre_http_request', $filter ); + + $this->assertWPError( $error ); + $this->assertSame( 'http_404', $error->get_error_code() ); + } + /** * @ticket 38231 * @dataProvider data_download_url_should_respect_filename_from_content_disposition_header