Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V8: Validate uploaded files based on the configuration of the FileUpload property configuration #10987

Merged
merged 3 commits into from Sep 2, 2021

Conversation

nikolajlauridsen
Copy link
Contributor

This fixes #10897.

Turns out that file extensions of uploaded files weren't validated at all based on the editor configuration, only the configuration in umbracoSettings.config. I've added extra validation so we now validate on both.

Testing

  1. Create a document type with 2 UploadField properties, one that allows all extensions, and one that only accepts csv.
  2. Try and create a content node, and ensure that:
    1. Only csv files are allowed in the limited upload field
    2. Other filetypes are allowed in the unlimited filetypes field
    3. Try adding your csv to the disallowedUploadFiles property of umbracoSettings.config, and ensure that you are no longer allowed to upload csv files, even to the upload field where you specified csv
    4. Try adding <allowedUploadFiles>csv</allowedUploadFiles> and ensure that only csv files are allowed, even in the upload field where everything is allowed

@nikolajlauridsen nikolajlauridsen changed the base branch from v8/contrib to v8/dev August 30, 2021 12:29
@elit0451
Copy link
Member

elit0451 commented Sep 2, 2021

The changes look good and my tests reveal the desired functionality.

One thing I noticed is that when we add a file type to disallowedUploadFiles, if the file has been previously selected and we Save or Save and publish, we will get the validation error but it won't prevent saving any new changes. Refreshing the page will clear the validation errors, and it looks like there isn't any problem with the files uploaded. Do we want to remove the file as we do when we re-upload and then Save and publish?

Same thing with a different file type previously uploaded than the one specified in allowedUploadFiles

Co-authored-by: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com>
@nikolajlauridsen
Copy link
Contributor Author

nikolajlauridsen commented Sep 2, 2021

Hmm, I'm not sure if I'm misunderstanding, but I can't seem to reproduce it, if I select a file, then update the disallowedUploadFiles, and wait for the process to reboot, it will show the error and not upload the image, from my testing it doesn't seem to upload the file either, are you sure the site had restarted for the changes in the config to take effect?

Gif for clarification
File upload

@elit0451
Copy link
Member

elit0451 commented Sep 2, 2021

After we figured out that the changes are just saved and not published when the validation errors I spoke about appear and the current workflow makes sense, I am happy to merge.

Nice work 💪

@elit0451 elit0451 merged commit c090afe into v8/dev Sep 2, 2021
@elit0451 elit0451 deleted the v8/bugfix/file-upload branch September 2, 2021 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

File Upload extensions isn't validated
2 participants