fix(asset-manager): enforce accept filter on drag-and-dropped files#6778
Open
geonsang-jo wants to merge 1 commit into
Open
fix(asset-manager): enforce accept filter on drag-and-dropped files#6778geonsang-jo wants to merge 1 commit into
geonsang-jo wants to merge 1 commit into
Conversation
Browsers don't apply the input `accept` attribute to dropped files, so videos dropped on an image upload zone were uploaded and selectable. Filter dropped files against `accept` in FileUploader.uploadFile(). Closes GrapesJS#6032
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6032
Problem
Dropping a non-image file (e.g. a video) onto an image component's asset upload zone uploads it and lets you select it as the image
src. Reported as drag-drop-only.Root cause
OpenAssetssetsaccept="image/*"on the upload input, which filters the native file picker — but browsers do NOT enforceaccepton drag-and-dropped files.FileUploader.uploadFile()readse.dataTransfer.filesand uploads them with no validation.Fix
Filter incoming files in
uploadFile()against the input'sacceptvalue via a small pure helper (isFileAccepted), and bail out before uploading if none match. Backward-compatible: with no/*/*accept (generic asset managers), nothing is filtered. CustomuploadFileimplementations are unaffected.Tests
Unit tests for
isFileAccepted(MIME wildcard / exact / extension /*/*) and an integration test asserting a dropped video does not trigger an upload while a dropped image does.Live demo
Side-by-side StackBlitz (double-click the image to open the Asset Manager, then drop a video vs an image):
Notes on behavior
beforeUpload->falsecancel path (which also returns without emitting an event) and with the nativeacceptpicker (which silently omits non-matching files).beforeUpload(files)now receives the filteredFile[]instead of the rawFileList(type(files: any) => void | false, so type-compatible). Also, when every dropped file is rejected,beforeUploadis not invoked at all (the early return precedes it).Questions for maintainers
beforeUpload-> false) is silent, so this PR keeps rejection silent. If you'd prefer consumers be able to react (e.g. show "only image files allowed"), I'm happy to add an opt-in event following theasset:upload:*convention, e.g.asset:upload:rejectedwith{ files, accept }.accepton the singleton uploader:OpenAssets.run()only setsacceptwhen truthy and never resets it, and the uploader input is a singleton rendered once (AssetManager.render()buildsAssetsViewonly on first open). So after an image picker setsaccept="image/*", a later genericAssetManager.open()with noacceptinherits the staleimage/*— before this PR that only affected the native picker, now it also makes drag-drop silently filter non-image files there. I can reset it on each open (uploadEl.setAttribute('accept', accept || '*/*')) here or as a follow-up.