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

Add file type validation #1154

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add file type validation #1154

wants to merge 3 commits into from

Conversation

DavidBiddle
Copy link
Contributor

@DavidBiddle DavidBiddle commented Jan 23, 2025

What problem does this pull request solve?

Trello card: https://trello.com/c/Nr7OQkk2/2092-limit-the-file-types-that-can-be-uploaded

Adds:

  • backend validation to restrict the files we accept to the following MIME types:
    • text/csv (.csv)
    • image/jpeg (.jpg, .jpeg)
    • image/png (.png)
    • application/rtf (.rtf)
    • text/plain (.txt)
    • application/pdf (.pdf)
    • application/json (.json)
    • application/vnd.openxmlformats-officedocument.spreadsheetml.sheet (.xlsx)
    • application/msword (.doc)
    • application/vnd.openxmlformats-officedocument.wordprocessingml.document (.docx)
    • application/vnd.oasis.opendocument.text (.odt)
  • an accept attribute on the file input with the same MIME types above. This will provide hints to the user's operating system on what kinds of files they should choose.

Testing instructions

If you want to bypass the accept attribute on a Mac in order to test the backend validation, you can:

  • click 'Browse', click Show Options in the Finder window that pops up, then change 'Format' to 'All files'.

Screenshots

When selecting a file, you should see hints about allowed files - in Finder on a Mac it looks like this:
image

After submitting a file of the wrong type, you should see this error message:
The selected file must be a CSV, JPEG, JPG, PNG, XLSX, DOC, DOCX, PDF, JSON, ODT, RTF or TXT

Things to consider when reviewing

  • Ensure that you consider the wider context.
  • Does it work when run on your machine?
  • Is it clear what the code is doing?
  • Do the commit messages explain why the changes were made?
  • Are there all the unit tests needed?
  • Has all relevant documentation been updated?

@DavidBiddle DavidBiddle force-pushed the add-file-type-validation branch 2 times, most recently from e9e587f to 6d57689 Compare January 23, 2025 17:16
Adds validation to check that the uploaded file has one of the expected
MIME types.
Adds the ['accept' attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file#accept)
to the file upload input. This doesn't prevent the user from uploading
other file types, but it provides hints to their operating system on
what kind of files we accept, so it reduces the chances of a user
uploading a disallowed file type accidentally.

Uses the same list of allowed filetypes as the backend validation, so
they should stay in sync.
In the file upload feature test, we were using a TempFile. This failed
after the file type validation was added, because the TempFile returns a
 of  which trips the validation.

This commit replaces the TempFile with a real, empty text file, which is
created in the  folder and deleted after the test completes. This
should make the test closer to real use of the feature.
@DavidBiddle DavidBiddle force-pushed the add-file-type-validation branch from 6d57689 to d5b995f Compare January 24, 2025 13:02
@DavidBiddle DavidBiddle marked this pull request as ready for review January 24, 2025 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant