Skip to content

test(isBase64): add large-string regression tests#2681

Open
easedu wants to merge 3 commits intovalidatorjs:masterfrom
easedu:fix/isBase64-stack-overflow
Open

test(isBase64): add large-string regression tests#2681
easedu wants to merge 3 commits intovalidatorjs:masterfrom
easedu:fix/isBase64-stack-overflow

Conversation

@easedu
Copy link
Contributor

@easedu easedu commented Mar 7, 2026

Description

Adds regression test coverage for large base64 strings as reported
in #2573.

Tests added

  • ~1MB valid base64 string
  • ~1MB invalid base64 string (injected invalid char)
  • ~1MB valid base64 string with urlSafe option

All 318 tests passing, 100% statement/line/function coverage.

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)
  • References provided in PR (where applicable)

Refs #2573

…ck overflow

V8's regex engine uses internal recursion proportional to string length,
causing 'Maximum call stack size exceeded' on large inputs. Replace all
4 regex patterns with iterative character-code validation that runs in
O(n) time with O(1) stack usage.

Closes validatorjs#2573
@codecov
Copy link

codecov bot commented Mar 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (b1aea75) to head (a962534).

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #2681   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          114       114           
  Lines         2595      2595           
  Branches       659       659           
=========================================
  Hits          2595      2595           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The dataLen === 0 check was unreachable: length % 4 guard and
paddingCount > 2 guard already prevent this condition. Removing
resolves the partial coverage flag from Codecov.
@WikiRik
Copy link
Member

WikiRik commented Mar 7, 2026

I think we already solved this, this addition doesn't make the code more readable for me. This library doesn't need to be very performant, it mostly should not crash on long strings. We can merge the new test if that's useful

@easedu
Copy link
Contributor Author

easedu commented Mar 7, 2026

Thanks for the review @WikiRik! Happy to simplify this PR to just the new large-string test cases and revert the implementation change.
I'll update shortly.

Per maintainer feedback: reverted isBase64.js to original regex
implementation. Kept new large-string test cases for regression
coverage.
@easedu
Copy link
Contributor Author

easedu commented Mar 7, 2026

Updated — reverted the implementation change and kept only the new large-string test cases.

Note: the tests pass here because ~1MB strings are within V8's regex stack limit in most environments. The original issue (#2573) manifests with larger strings or smaller stack limits. The tests still add useful regression coverage.

Let me know if anything else needs adjustment.

@easedu easedu changed the title fix(isBase64): replace regex with iterative validation to prevent stack overflow test(isBase64): add large-string regression tests Mar 7, 2026
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