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: set limit to 250kb for whitelist #8043

Merged
merged 2 commits into from
Jan 13, 2025
Merged

Conversation

kevin9foong
Copy link
Contributor

@kevin9foong kevin9foong commented Jan 13, 2025

Problem

The large number of NRICs needed to be whitelisted resulted in two identified issues:

  1. The processing time of whitelisting took too long
  • Majority of the processing time was used to encrypt the individuals NRICs. This ensures that data remains only accessible to the form owners.
  • If the processing takes too long, FormSG automatically terminates the requests as it is not responsive.
  1. The information stored in FormSG's database went over the limits
  • The encryption process increases the encrypted NRIC size.
  • This results in the actual file size becoming larger than the limit, crossing FormSG’s database limit per form (estimated ~250k NRICs at this point)

Closes [insert issue #]

Solution

Reduce whitelist limit to 250kB to mitigate and avoid misleading admins on the file size limit from facing repeated issue.

Breaking Changes

  • No - this PR is backwards compatible

@kevin9foong kevin9foong requested a review from KenLSM January 13, 2025 10:54
@kevin9foong kevin9foong self-assigned this Jan 13, 2025
@kevin9foong kevin9foong force-pushed the fix/reduce-whitelist-limit branch from 91fe644 to bf81c71 Compare January 13, 2025 11:05
@kevin9foong kevin9foong changed the base branch from release-v6.173.2 to develop January 13, 2025 11:05
@datadog-opengovsg
Copy link

datadog-opengovsg bot commented Jan 13, 2025

Datadog Report

Branch report: fix/reduce-whitelist-limit
Commit report: 3088894
Test service: formsg

✅ 0 Failed, 555 Passed, 0 Skipped, 1m 5.64s Total duration (5m 49.07s time saved)

@kevin9foong kevin9foong force-pushed the fix/reduce-whitelist-limit branch from bf81c71 to 3088894 Compare January 13, 2025 11:16
Copy link
Contributor

@KenLSM KenLSM left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for investigating and figuring out the practical limits

@KenLSM KenLSM merged commit 03fb83d into develop Jan 13, 2025
18 of 19 checks passed
@KenLSM KenLSM deleted the fix/reduce-whitelist-limit branch January 13, 2025 14:34
KenLSM added a commit that referenced this pull request Jan 14, 2025
* fix(deps): bump libphonenumber-js from 1.11.16 to 1.11.17 in /shared (#8021)

Bumps [libphonenumber-js](https://gitlab.com/catamphetamine/libphonenumber-js) from 1.11.16 to 1.11.17.
- [Changelog](https://gitlab.com/catamphetamine/libphonenumber-js/blob/master/CHANGELOG.md)
- [Commits](https://gitlab.com/catamphetamine/libphonenumber-js/compare/v1.11.16...v1.11.17)

---
updated-dependencies:
- dependency-name: libphonenumber-js
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore: mongoose, nanoid packages update (#8000)

* chore: update mongoose, dependabot #378

* chore: update react-email-preview/nanoid

* chore: update fe/be nanoid

* feat(iframe): add frame messaging for paysg (#7979)

* feat: update iframe parent on form submission

* fix: remove localhost from trusted origins

---------

Co-authored-by: Antariksh Mahajan <antarikshmahajan@gmail.com>

* fix: only show fixed translations for supported forms (#8038)

* fix: default back to english language if form does not support multi-language

* fix: remove login translations from being fetched

* fix: set limit to 250kb for whitelist (#8043)

Co-authored-by: Ken <ken@open.gov.sg>

* chore: bump version to v6.174.0

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Antariksh Mahajan <antarikshmahajan@gmail.com>
Co-authored-by: Siddarth Nandanahosur Suresh <48427064+siddarth2824@users.noreply.github.com>
Co-authored-by: Kevin Foong <55353265+kevin9foong@users.noreply.github.com>
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.

2 participants