Skip to content

Issue 53027: Cannot insert a row into a sample type if Allowed File Extensions set, no file is provided #6677

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

Merged
merged 15 commits into from
Jun 10, 2025

Conversation

labkey-jeckels
Copy link
Contributor

Rationale

We are being too picky about file uploads.

Related Pull Requests

Changes

  • Skip over file form elements where no file was selected
  • Stop asserting about file extensions in makeLegalName(), which isn't necessarily creating a file
  • Improve error messaging when an illegal file extension is uploaded
  • Minor code cleanup

Copy link
Contributor

@labkey-adam labkey-adam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR, but should we consider a site validator or similar that checks existing attachments for undesired extensions?

@labkey-jeckels
Copy link
Contributor Author

Not for this PR, but should we consider a site validator or similar that checks existing attachments for undesired extensions?

I added a new metric, modules.Core.allowListCounts.FileExtension, so we can monitor the adoption of this feature. There's probably a lot that people would want to fully lock things down.

Note that I had to remove the assert that was checking for allowed extensions when writing files to the server. Otherwise things like folder exports quickly failed unless you added all of the various extensions they'd produce.

@labkey-danield labkey-danield merged commit 4a731cb into develop Jun 10, 2025
8 checks passed
@labkey-danield labkey-danield deleted the fb_allowFileExtensionAutoTest branch June 10, 2025 17:33
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.

3 participants