Skip to content

UIFC-262 Add 50 MB file upload size limit to prevent DoS attacks - #349

Merged
slaemmer merged 7 commits into
masterfrom
UIFC-262-limit-file-upload-size
Dec 12, 2025
Merged

UIFC-262 Add 50 MB file upload size limit to prevent DoS attacks#349
slaemmer merged 7 commits into
masterfrom
UIFC-262-limit-file-upload-size

Conversation

@slaemmer

@slaemmer slaemmer commented Dec 1, 2025

Copy link
Copy Markdown
Contributor

https://folio-org.atlassian.net/browse/UIFC-262

  • Implement client-side file size validation (reject > 50 MB before upload)
  • Refactor handleDrop to async/await for proper error propagation
  • Add helper functions: createFileSizeErrorMessage, handlePayloadTooLargeError
  • Sanitize backend error messages (reject HTML tags, >200 chars) to prevent XSS
  • Handle HTTP 413 with fallback for malformed responses
  • Fix validation error timing (show only after touch/submit, not immediately)
  • Extract HTTP_STATUS_PAYLOAD_TOO_LARGE constant
  • Add translations: fileTooLarge, backendRejected, network
  • Update CHANGELOG

- Implement client-side file size validation (reject > 50 MB before upload)
- Refactor handleDrop to async/await for proper error propagation
- Add helper functions: createFileSizeErrorMessage, handlePayloadTooLargeError
- Sanitize backend error messages (reject HTML tags, >200 chars) to prevent XSS
- Handle HTTP 413 with fallback for malformed responses
- Fix validation error timing (show only after touch/submit, not immediately)
- Extract HTTP_STATUS_PAYLOAD_TOO_LARGE constant
- Add translations: fileTooLarge, backendRejected, network
- Update CHANGELOG
@slaemmer
slaemmer requested a review from a team as a code owner December 1, 2025 07:07
@slaemmer
slaemmer marked this pull request as draft December 1, 2025 07:08
@github-actions

github-actions Bot commented Dec 1, 2025

Copy link
Copy Markdown

Jest Unit Test Results

107 tests  +29   107 ✅ +29   1m 18s ⏱️ ±0s
 23 suites + 2     0 💤 ± 0 
  1 files   ± 0     0 ❌ ± 0 

Results for commit 17cd540. ± Comparison against base commit 6054621.

♻️ This comment has been updated with latest results.

@slaemmer
slaemmer marked this pull request as ready for review December 1, 2025 07:58
@slaemmer
slaemmer force-pushed the UIFC-262-limit-file-upload-size branch from 29e990e to f58db5f Compare December 1, 2025 13:33
- Add component tests for FileUploaderField
- Fix ReDoS vulnerability: replace /<[^>]*>/ regex with simple string checks for HTML tag detection (backendMessage.includes('<') && includes('>'))
- Fix Sonar warnings
@slaemmer
slaemmer force-pushed the UIFC-262-limit-file-upload-size branch from f58db5f to 58f385a Compare December 1, 2025 13:48
@slaemmer
slaemmer requested a review from alb3rtino December 10, 2025 13:32
@elsenhans

Copy link
Copy Markdown
Contributor

Thanks @slaemmer , looking good!

Having general questions:

  1. Why 50 MB? Is this documented/justified with someone? Can't find anything in the Jira issue.
  2. Should there a backend validation as a second layer of protection?

@slaemmer

slaemmer commented Dec 11, 2025

Copy link
Copy Markdown
Contributor Author

Thanks!

  1. Decision for 50 MB was based on this issue and the information that the biggest currently are about 22 MB and usually less than 30 MB so it should be a safe upper boundary. Should I add that reasoning as code comment?
  2. There is, it was already merged which is why you probably did not find it amongst the open PRs.

@elsenhans elsenhans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like input.value has the wrong PropType:
value: PropTypes.string
Could you please change it to object?

Comment thread src/components/Filters/FilterFile/UploadFile/FileUploaderField.js
Comment thread src/components/Filters/FilterFile/UploadFile/FileUploaderField.js Outdated
Comment thread src/components/Filters/FilterFile/UploadFile/FileUploaderField.js Outdated
Comment thread src/util/fileUtils.js Outdated
Comment thread src/components/Filters/FilterFile/UploadFile/FileUploaderField.test.js Outdated
Comment thread src/components/Filters/FilterFile/UploadFile/FileUploaderField.js Outdated
Comment thread src/components/Filters/FilterFile/UploadFile/FileUploaderField.js Outdated
Comment thread src/components/Filters/FilterFile/UploadFile/FileUploaderField.js

@elsenhans elsenhans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like input.value has the wrong PropType:
value: PropTypes.string
Could you please change it to object?

@slaemmer

Copy link
Copy Markdown
Contributor Author

Seems like input.value has the wrong PropType: value: PropTypes.string Could you please change it to object?

Good catch! Apparently it can be either string or object in code. Changed propTypes to reflect that. As this is less than ideal though, I created a bug ticket for this.

@elsenhans elsenhans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you!

@alb3rtino alb3rtino left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🥇

@sonarqubecloud

Copy link
Copy Markdown

@slaemmer
slaemmer merged commit 8f9ae13 into master Dec 12, 2025
26 checks passed
@slaemmer
slaemmer deleted the UIFC-262-limit-file-upload-size branch December 12, 2025 07:44
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