Make WordPress Core

Opened 6 days ago

Last modified 19 hours ago

#66374 new defect (bug)

Media endpoint returns 500 when an upload is rejected for a client reason

Reported by: aryanjasala Owned by:
Priority: normal Milestone: Awaiting Review
Component: REST API Version: 4.7
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

When wp_handle_upload() or wp_handle_sideload() rejects a file, WP_REST_Attachments_Controller returns status 500 whatever the reason:

  • upload_from_file(): rest_upload_unknown_error, 500
  • upload_from_data(): rest_upload_sideload_error, 500

Most of these rejections are caused by the request, not the server:

  • a file type the site does not allow,
  • an empty or partial file,
  • a file over upload_max_filesize,
  • a file a plugin rejected in wp_handle_upload_prefilter (size, dimension and naming policy plugins do this).

Other request checks in the same controller already return 400 (rest_upload_no_data, the multisite checks in check_upload_size()). The URL sideload path added in 7.2 also returns 400 for a disallowed type, by comparing the translated message string.

Why this matters more in 7.2: since [64588], uploads from the Media Library grid, Add New Media File and the media modal go through POST /wp/v2/media instead of async-upload.php. So every upload a plugin refuses, and every disallowed type, now shows up as an HTTP 500:

  • in server access logs,
  • in APM and uptime error rates,
  • in the browser console ("Failed to load resource: the server responded with a status of 500").

On 7.1 the same refusal from async-upload.php was a 200 with an error message.

Steps to reproduce (7.2-beta1-64675)

  1. Add this mu-plugin:
    <?php
    add_filter( 'wp_handle_upload_prefilter', function ( $file ) {
            if ( false !== strpos( $file['name'], 'reject' ) ) {
                    $file['error'] = 'Rejected by a plugin.';
            }
            return $file;
    } );
    
  2. Go to Media > Add New Media File and upload reject-me.jpg. The UI shows "Rejected by a plugin.", but the request returns 500.

The same with an application password and curl:

Upload Before After
valid JPEG 201 201
reject-me.jpg (prefilter), multipart 500 400
vector.svg (type not allowed), multipart 500 400
shell.php (type not allowed), multipart 500 400
empty file, multipart 500 400
reject-me.jpg, raw body (sideload) 500 400
vector.svg, raw body (sideload) 500 400

Error codes and messages stay the same; only the status changes.

Patch

The controller cannot tell the reasons apart today: every failure reaches it as array( 'error' => $message ). The patch:

  1. _wp_handle_upload() passes an error code as a third argument to the upload error handler. The default wp_handle_upload_error() returns it as error_code. The codes:
    • upload_prefilter_error
    • upload_invalid_form
    • upload_err_ini_size, upload_err_form_size, upload_err_partial, upload_err_no_file, upload_err_no_tmp_dir, upload_err_cant_write, upload_err_extension, upload_php_error
    • upload_failed_test
    • upload_empty_file
    • upload_invalid_file_type
    • upload_dir_error
    • upload_move_failed
  2. A new WP_REST_Attachments_Controller::get_upload_error_status() returns 400 for the client errors and 500 for everything else, including no tmp dir, can't write, an uploads directory that can't be created, a file that can't be moved, and any error without a code.
  3. The URL sideload path checks upload_invalid_file_type instead of comparing the translated message.

Backward compatibility

  • The error key and every REST error code are unchanged. error_code is a new, optional key.
  • A custom upload_error_handler now receives a third argument. Userland functions ignore extra arguments. Only an internal PHP function used as the handler with a fixed argument count would complain, which seems unlikely.
  • A plugin that defines its own wp_handle_upload_error() before core does returns no error_code, so its errors keep the 500 status. Nothing changes for it.
  • Clients that treat 5xx as "retry" will no longer retry rejected uploads, which is the point. The media upload queue in @wordpress/upload-media decides retries from the error message, not the status, so it is unaffected. The Media Library chooses its text from the error code and message, so the UI is unchanged.

Open questions

  • Should the size errors (upload_err_ini_size, upload_err_form_size) be 413 instead of 400?
  • Should the type error use rest_upload_invalid_file_type everywhere, like the URL sideload path? I left the codes alone to keep the change small.

Tests

  • REST: a prefilter rejection, a disallowed type and a raw-body prefilter rejection return 400. An uploads directory that can't be created and a destination that can't be written return 500.
  • wp_handle_sideload() returns the right error_code for prefilter, empty file, type, uploads dir and move failures.
  • Eight of the ten new tests fail without the change. The two 500 tests guard the existing behavior.
  • WP_Test_REST_Attachments_Controller, Tests_Admin_IncludesFile, Tests_Media and Tests_File: 966 tests pass. The 20 warnings come from Tests_Filesystem_WpOpcacheInvalidateDirectory and are the same on trunk.
  • phpcs and PHPStan are clean on the changed files.

Use of AI tools: I used Claude Code to write the patch and tests, and draft this ticket. I reviewed the code and ran the tests and the reproduction myself.

Change History (3)

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


6 days ago
#1

  • Keywords has-patch has-unit-tests added

When wp_handle_upload() or wp_handle_sideload() rejects a file, WP_REST_Attachments_Controller returns status 500 for every reason, including ones the request caused: a file type the site does not allow, an empty or partial file, a file over the size limit, or a file a plugin rejected through a prefilter. Since [64588], uploads from the Media Library grid, Add New Media File and the media modal go through this endpoint, so each of these rejections now shows up as a server error.

This PR:

  1. Makes _wp_handle_upload() pass an error code as a third argument to the upload error handler. The default wp_handle_upload_error() returns it as error_code, for example upload_prefilter_error, upload_invalid_file_type, upload_empty_file, upload_dir_error or upload_move_failed.
  2. Adds WP_REST_Attachments_Controller::get_upload_error_status(), which returns 400 for client errors and keeps 500 for everything else, including any error without a code.
  3. Makes the URL sideload path check upload_invalid_file_type instead of comparing the translated message.

Error codes and messages in the responses are unchanged; only the status changes. Backward compatibility notes and open questions (413 for size errors?) are on the ticket.

Tests:

  • REST: a prefilter rejection, a disallowed type and a raw-body prefilter rejection return 400. An uploads directory that can't be created and a destination that can't be written return 500.
  • wp_handle_sideload() returns the right error_code for each failure.
  • Run with --group 66374. Eight of the ten new tests fail without the change.
  • WP_Test_REST_Attachments_Controller, Tests_Admin_IncludesFile, Tests_Media and Tests_File pass locally (966 tests).

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Writing the patch and tests, and drafting this description. I reviewed the code and ran the tests and the reproduction myself.

#2 @aryanjasala
6 days ago

Patch: ​PR 14810. The new tests carry @ticket 66374, so --group 66374 runs them.

#3 @aryanjasala
19 hours ago

7.2 makes this more visible. When client-side media processing is on (the default on HTTPS sites), Media > Add New and the media modal now upload through POST /wp/v2/media (media-upload-pipeline.js, new in 7.2) instead of async-upload.php.

Tested on 7.2-beta3-64990 with a wp_handle_upload_prefilter callback that rejects the file, uploading from Media > Add New:

Path Response
async-upload.php (7.1 uploader) 200, {"success":false,"data":{"message":"Rejected by a plugin."}}
POST /wp/v2/media (7.2 uploader) 500

The user still sees the plugin's message, so the screen looks the same. But on 7.2, every upload that a plugin rejects is logged as a server error, in access logs and in error monitoring.

Could this go into the 7.2 milestone? ​PR 14810 still merges cleanly with trunk, and its CI passes.

Note: See TracTickets for help on using tickets.