Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
In #2940383: [META] Unify file upload logic of REST and JSON:API we are wanting to upload files directly via octet-stream and Content-Disposition header, and because it doesn't use $_POST variables we can't use Symfony UploadedFile object. However, in \Drupal\file\Upload\FileUploadHandler::handleFileUpload() we call isValid() and assume we have an instance of UploadedFile.
This is most likely due to copying existing code.
Steps to reproduce
Proposed resolution
- Deprecate
\Drupal\file\Upload\UploadedFileInterface::isValid()::getError()and::getErrorMessage() - Add an \Drupal\file\Validation\UploadedFileValidator that validates uploaded files using \Drupal\file\Validation\Constraint\UploadedFileConstraint and \Drupal\file\Validation\Constraint\UploadedFileConstraintValidator
- Remove the checks for
isValid()and throwing exceptions inFileUploadHandler::handleFileUpload() - Call UploadedFileValidator::validate() and check for constraint violations instead
Remaining tasks
Do it.
User interface changes
API changes
\Drupal\file\Upload\UploadedFileInterface::isValid() ::getError() and ::getErrorMessage() are deprecated
A new UploadedFileInterface::validate() method is added, that throws \Symfony\Component\HttpFoundation\File\Exception\FileException and sub-classes.
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 3375447-nr-bot.txt | 90 bytes | needs-review-queue-bot |
Issue fork drupal-3375447
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
kim.pepperComment #4
andypostlooks good idea!
Comment #5
kim.pepperMy only other thought is to not throw exceptions but to use Symfony validators some how. Not sure how this would work. 🤔
Comment #6
smustgrave commentedSeems like a perfectly reasonable cleanup.
Comment #7
kim.pepperGoing to think about the best approach before we commit.
Comment #8
kim.pepperComment #10
kim.pepperThis approach uses a symfony validator that is not coupled with TypeData. It makes use of
DrupalTranslatorso we still getTranslatableMarkupobjects returned from the constraint violationgetMessage()method.I had a go at re-using
\Symfony\Component\Validator\Constraints\Fileand\Symfony\Component\Validator\Constraints\FileValidatorbut couldn't get the messages to match up due to missing the%fileparam.Comment #11
kim.pepperUpdated IS with current implementation.
Comment #12
kim.pepperComment #13
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #14
kim.pepperComment #15
smustgrave commentedSo reading the IS and CR can't think of a reason not to do this. Going to mark it.
Comment #16
kim.pepperBlocking a critical #2940383: [META] Unify file upload logic of REST and JSON:API therefore this is also critical.
Comment #18
larowlanLooking good, left some comments on the MR about a possible extra method and some BC issues.
Comment #19
kim.pepperAdded a deprecation test for the
$throwarg.All other feedback has been addressed.
Comment #20
smustgrave commentedAll threads have been addressed.
Comment #21
larowlanCan we update the change record to add examples for how to fix the deprecation for the $throws param on handleFileUpload and how that works for calling code.
I.e pass FALSE and inspect the errors on the return instead of expecting an exception?
Left a review on the MR too
Comment #22
kim.pepperResolved all feedback
Comment #23
smustgrave commentedAppears all threads have been resolved.
Comment #24
kim.pepperThis is blocking 4 other issues:
Comment #25
wim leersComment #26
larowlanDiscussed with @catch and @kim.pepper
Because there are still a few moving parts here, I think it makes sense to put this into 11 only, with a goal of it making it into 10.3 with all of the other issues that relate to it (this currently blocks 4 other issues that refactor file upload handling).
Because those 4 issues might require more changes to this, rather than lock ourselves into an API for 10.2, this gives us a chance to get the API right before 10.3
Marking as needs work to re-roll with the deprecations saying 10.3 instead of 10.2. But then I'm keen to get this into 11 and get onto the other issues.
Thanks for your understanding!
Comment #27
kim.pepperUpdated all deprecation messages from 10.2 to 10.3.
Moving back to RTBC.
Comment #28
larowlanCan we get a second change record for the new basic validator? Thanks
Comment #29
kim.pepperDone ✅
Comment #31
larowlanCommitted to 11.x and unpostponed the 4 issues.
Published the 2 CRs