[META] Modernise file upload logic

Removing #3221793: Move file upload validation from file.module to constraint validators from the list as I no longer feel it's a blocker for #2940383: [META] Unify file upload logic of REST and JSON:API

Added a sequence diagram to show the current state.

Mermaid definition is:

sequenceDiagram
    autonumber
    participant mf as Element\ManagedFile
    participant mfsu as file_managed_file_save_upload
    participant Request
    participant suff as _file_save_upload_from_form
    participant fsu as file_save_upload
    participant fuh as FileUploadHandler
    mf->>+mfsu: valueCallback()
    mfsu->>+Request: get files to be uploaded for form element
    Request-->>-mfsu: files from request
    mfsu->>+suff: with form element and state
    suff-->>suff: get upload validators from form
    suff->>+fsu: with upload validators
    fsu->>+Request: get files to be uploaded for form element (again)
    Request-->>-fsu: files from request
    loop for each file
    fsu->>+fuh: upload file with validators
    fuh-->>fuh: Call UploadedFile::isValid()
    fuh-->>fuh: Sanitize filename
    fuh-->>fuh: Call FileValidator::validate() with validators
    fuh-->fuh: Move uploaded file
    fuh-->fuh: Call $file->validate()
    fuh-->>fuh: Save file
    fuh-->>-fsu: upload result
    end
    fsu-->>-suff: array of fid=>file
    suff-->>-mfsu: array of fid=>file
    mfsu-->>-mf: array of fid=>file

Here's a sequence diagram of the REST file upload

Mermaid definition:

sequenceDiagram
    autonumber
    participant fur as FileUploadResource
    participant Request
    participant efm as EntityFieldManager
    participant pi as php://input
    participant fv as FileValidator
    participant fs as FileSystem
    fur->>+Request: getHeaders()
    Request-->>-fur: content disposition headers
    fur-->>fur: get filename
    fur->>+efm: getFieldDefinitions()
    efm-->>-fur: field definition
    fur-->>fur: get destination
    fur-->>fur: get upload validators from definition
    fur-->>fur: sanitize filename
    fur->>+pi: stream php://input to temp file
    pi->>-fur: file
    fur-->>fur: lock file for save
    fur-->>fur: create file entity
    fur->>+fv: validate()
    fv-->>-fur: violations
    fur-->>fur: set filename
    fur->>+fs: move file to destination
    fur-->>fur: validate file entity
    fur-->>fur: save file entity
    fur-->>fur: release lock

Here's a sequence diagram of the somewhat more complex json api file upload.

Mermaid definition:

sequenceDiagram
    autonumber
    participant fuc as FileUpload controller
    participant efm as EntityFieldManager
    participant tffu as TemporaryFileFieldUploader
    participant acm as AccessControlManager
    participant Request
    participant pi as php://input
    participant Lock
    participant mtg as MimeTypeGuesser
    participant fv as FileValidator
    participant fs as FileSystem
    fuc->>+efm: get field definitions
    efm-->>-fuc: field definition
    fuc->>+tffu: check access
    tffu->>+acm: check access
    acm-->>tffu: access result
    tffu-->>-fuc: access result
    fuc->>+tffu: parse content disposition headers
    tffu->>+Request: get headers
    Request-->>-tffu: headers
    tffu-->>-fuc: filename
    fuc->>+tffu: handle file upload for field (with definition)
    tffu-->>tffu: get destination
    tffu->>fs: prepare destination
    tffu-->>tffu: get validators
    tffu-->>tffu: santize filename
    tffu->>+pi: stream php://input to temp file
    pi->>-tffu: file
    tffu->>Lock: acquire lock for save
    tffu->>+mtg: Guess mime type
    mtg-->>-tffu: Mime type
    tffu-->>tffu: Create file entity
    tffu->>+fv: validate
    fv-->>-tffu: violations
    tffu-->>tffu: set filename
    tffu->>fs: move file
    tffu-->>tffu: validate entity
    tffu-->>tffu: Save file
    tffu->>Lock: release lock
    tffu-->>-fuc: File

Tweaking the sequence diagrams to include more details

Updating core file_managed_file_upload sequence

Adding the graphql upload sequence.

Interestingly, GraphQL uploads raise some questions:

1. Use Symfony UploadedFile: this indicates they rely on $_POST varables being populated, therefore the Content-Type header in the request uses application/x-www-form-urlencoded or multipart/form-data. Would be good to verify this?
2. It does extra validation for the extension, and dispatches the sanitize filename event twice. Would good to understand why?
3. Although it uses Symfony UploadedFile, it doesn't call UploadedFile::move() which in turn calls PHP move_uploaded_file() which is recommended. Again not sure why?

Another question on direct file uploads compared with Symfony UploadedFile / $_POST file uploads:

Are we checking for the php errors? e.g. UPLOAD_ERR_INI_SIZE, UPLOAD_ERR_FORM_SIZE, UPLOAD_ERR_PARTIAL, UPLOAD_ERR_NO_FILE, UPLOAD_ERR_EXTENSION etc?

Looks like we are just adding a FileSizeLimit / max_filesize validator. Does that cover the cases above?

Other questions:

• Should we support multiple file uploads?
• Why do we stream upload php://input when Symfony Request::getContent() does this for us?

Catching up on @Wim's blog post (now unavailable) https://web.archive.org/web/20230522044139/http://wimleers.com/blog/api-...

After reading Wim's blog post, I think I answered a few of my own questions.

Are we checking for the php errors? e.g. UPLOAD_ERR_INI_SIZE, UPLOAD_ERR_FORM_SIZE, UPLOAD_ERR_PARTIAL, UPLOAD_ERR_NO_FILE, UPLOAD_ERR_EXTENSION etc?

Looks like we are just adding a FileSizeLimit / max_filesize validator. Does that cover the cases above?

Why do we stream upload php://input when Symfony Request::getContent() does this for us?

We are intentionally wanting to avoid PHP upload size limits by using php://input. Request::getContent() looks like it just just returns fopen('php://input', 'r') for binary files anyway.

Here's a proposed plan for replacing:

• file_managed_file_save_upload
• _file_save_upload_from_form
• file_save_upload

With:

FileFormHelper:

• Replaces file_managed_file_save_upload and _file_save_upload_from_form
• provides callback for Element\ManagedFile
• handles all form interactions
• getting info from form elements
• setting form errors
• setting status messages

FileFormUploader

• Replaces file_save_upload
• is passed an array of UploadedFiles rather than having to lookup from Request again
• handles caching of uploads
• Loops through uploads with multiple files
• handles UploadedFile::isValid checks
• Calls FileUploadHandler::handleFileUpload() for each file
• catches exceptions for each file upload
• Adds status messages for each file upload
• Returns an array of fid=>file to FileFormHelper

Created child issue #3375423: Deprecate file_managed_file_save_upload(), file_save_upload() and _file_save_upload_from_form() and replace with a service

Created #3375447: Create an UploadedFile validator and deprecate error checking methods on UploadedFileInterface

Here's the proposed REST and JSON API sequence diagrams. Updated the proposed section in the IS.

@Matroskeen is there anything in the proposed architecture that could be modified to make it easier to support TUS in contrib?

Adding the sequence for \Drupal\ckeditor5\Controller\CKEditor5ImageController::upload().

As pointed out in #3372385: CKEditor file upload sets file URI prior to validation, causing validators to be unable to find the file. the order of validating the file and then moving it is the wrong way around.

It's not stated in the issue summary, but I assume the plan for CKEditor is the same for JSONAPI/Rest? If so, can we update the issue summary there too.

CKEditor5 is more like graphql in that it uses an UploadedFile object, rather than direct uploaded files via php://input. (Still thinking of the correct name for this!)

However, CKEditor5 should switch to FileUploadHandler.

Added child issues:

• #3378606: Add an UploadedFiles param converter
• #3378607: Move Direct File Upload access control to the routing layer

We could do these tasks first before trying to swap out the whole file uploading for REST and JSON API at the same time:

• #3378607: Move Direct File Upload access control to the routing layer
• #3380345: Create an InputStreamFileWriter for writing the input stream to a file
• #3380379: Create content disposition filename extractor and deprecate duplicate code in REST and JSON API

\Drupal\ckeditor5\Controller\CKEditor5ImageController uses UploadedFile from the request, so it just needs to make use of FileUploadHandler. #3388985: Make CKEditor5ImageController reuse FileUploadHandler

Created #3389447: Provide a trait to create file upload validators from file field settings

Two more issues fixed:
* #3375447: Create an UploadedFile validator and deprecate error checking methods on UploadedFileInterface
* #3380345: Create an InputStreamFileWriter for writing the input stream to a file

#3380379: Create content disposition filename extractor and deprecate duplicate code in REST and JSON API is in also.

Yeah we still need to do #3375423: Deprecate file_managed_file_save_upload(), file_save_upload() and _file_save_upload_from_form() and replace with a service which is mostly cleanup.