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, 500upload_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)
- 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; } );
- 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:
_wp_handle_upload()passes an error code as a third argument to the upload error handler. The defaultwp_handle_upload_error()returns it aserror_code. The codes:upload_prefilter_errorupload_invalid_formupload_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_errorupload_failed_testupload_empty_fileupload_invalid_file_typeupload_dir_errorupload_move_failed
- 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. - The URL sideload path checks
upload_invalid_file_typeinstead of comparing the translated message.
Backward compatibility
- The
errorkey and every REST error code are unchanged.error_codeis a new, optional key. - A custom
upload_error_handlernow 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 noerror_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-mediadecides 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_typeeverywhere, 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 righterror_codefor 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_MediaandTests_File: 966 tests pass. The 20 warnings come fromTests_Filesystem_WpOpcacheInvalidateDirectoryand 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
#3
@
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.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
When
wp_handle_upload()orwp_handle_sideload()rejects a file,WP_REST_Attachments_Controllerreturns 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:
_wp_handle_upload()pass an error code as a third argument to the upload error handler. The defaultwp_handle_upload_error()returns it aserror_code, for exampleupload_prefilter_error,upload_invalid_file_type,upload_empty_file,upload_dir_errororupload_move_failed.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.upload_invalid_file_typeinstead 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:
wp_handle_sideload()returns the righterror_codefor each failure.--group 66374. Eight of the ten new tests fail without the change.WP_Test_REST_Attachments_Controller,Tests_Admin_IncludesFile,Tests_MediaandTests_Filepass 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.