Problem/Motivation
While trying to test with the clamav module, I realized that the files uploaded via the media_bulk_upload_dropzonejs submodule do not carry out the necessary file validations during uploads before creating a File entity, this shouldn't be the case.
Steps to reproduce
- Enable the
media_bulk_upload_dropzonejssubmodule - Enable clamav with an invalid scan mechanism and "Block unchecked files" ticked. Or alternatively implement the
hook_file_validatehook and always have it return an error.
Proposed resolution
Make use of existing APIs to create a valid file entity.
Borrowing from existing Dropzone module examples or how core handles file uploads via file_save_upload with the FileUploadHandler service.
Remaining tasks
Provide issue fork/patch.
User interface changes
N/A
API changes
Added new array context containing the following to the hook_media_bulk_upload_file_ids_alter hook:
- media_bulk_upload_form: the current MediaBulkUploadForm instance.
- form: the current form array.
- form_state: the current form state.
- media_bulk_config: the Media Bulk Config entity.
Data model changes
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 3332945-media_bulk_upload-dropzonejs-upload-validation-13.patch | 12.87 KB | falco010 |
| #10 | interdiff_reroll_7-10.txt | 9.01 KB | codebymikey |
| #10 | 3332945-media_bulk_upload-dropzonejs-upload-validation-10.diff | 14.08 KB | codebymikey |
Issue fork media_bulk_upload-3332945
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:
Comments
Comment #3
codebymikey commentedAttached a patch adding validation support to the previous dropzone commit.
Comment #4
codebymikey commentedComment #5
codebymikey commentedRemoved changes to
media_bulk_upload_dropzonejs.info.ymlfrom the patch file since it couldn't be applied.Comment #6
codebymikey commentedAdd support for validating the created Media entity as well, providing better integration with media_duplicates.
Comment #7
codebymikey commentedIncorporated the fix from #3302559: Add support to create new taxonomy terms (entities) during bulk upload into this issue since it also required support for entity validations of the created entity references.
Comment #8
kristen polSeems similar to or duplicate of:
#3291228: Add form validation to inform other module to valid the new uploaded files
which is much simpler. I haven't reviewed this code yet though.
@codebymikey Would you please provide your thoughts on differences with the other issue?
Comment #9
codebymikey commentedThis implementation is a little more verbose, but only because it tries as much as possible to make all the same API calls a normal File upload would normally call (e.g. all the dropzone-isms with
\Drupal\dropzonejs\DropzoneJsUploadSave::createFileensuring that the uploaded files with potential executable extensions are still properly escaped and validated as seen fit by whichever UploadSave service the site is choosing to run).This variation also needed to support validation on the Media entity level as well which that version didn't.
Comment #10
codebymikey commentedRerolled the patch.
Comment #11
codebymikey commentedComment #12
kristen polI assume this is ready for review so changing status.
Comment #13
falco010We were having issues with incompatibility with the media_duplicates module. (Which can disallow a media duplicate in the upload)
The patches here solved those issues for us. However, the latest MR or patch could not be applied to the latest 3.0.4 release.
Therefore, here is a reroll which should apply to 3.0.4
Comment #14
jannakha commentedResolve merge conflicts manually