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

Fix max file size limit #21004

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

michelpmcdonald
Copy link
Contributor

@michelpmcdonald michelpmcdonald commented Feb 27, 2025

Summary

No feature flag, bug fix.

Currently, benefits intake has a per PDF max file size limit documented as 100 MB.
Internally our code uses 1,000,000 bytes as the max file size, however 100 MB is actually 1,048,576 bytes.

To reproduce, send in a 99 MB PDF file, we reject it even though it is less than 100 MB

The solution was is to change our max file size limit in bytes to 1,048,576 and adjust other places in the code(mostly unit test) that used decimal kb sizing(1000 bytes per kb) to binary kb sizing(1024 bytes per kb).

I did confirm with central mail(EMMS API), the recipient of these files that they are using binary\1024 bytes per kb and now we should be in alignment with them.

I don't think this will negatively impact consumers considering that we are now allowing slightly bigger files as intended.

Team banapeels, we own this code

Related issue(s)

https://jira.devops.va.gov/browse/API-45306

Testing done

-We already had pretty good unit test coverage, but they had to be adjusted because they were set up on decimal sizing where 1000 bytes equals a kilobyte, so i changed them to the proper 1024 bytes equals a kilobyte

What areas of the site does it impact?

Benefits Intake uploads

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog (if applicable)

Requested Feedback

(OPTIONAL)What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?

@va-vfs-bot va-vfs-bot temporarily deployed to API-45306-bi-max-file-size-limit/main/main February 27, 2025 16:39 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to API-45306-bi-max-file-size-limit/main/main March 7, 2025 15:30 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to API-45306-bi-max-file-size-limit/main/main March 7, 2025 17:02 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to API-45306-bi-max-file-size-limit/main/main March 7, 2025 18:02 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
banana-peels Lighthouse Banana Peels Team benefits-intake Lighthouse Benefits Intake API Lighthouse lighthouse require-backend-approval test-passing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants