test(isBase64): add large-string regression tests#2681
test(isBase64): add large-string regression tests#2681easedu wants to merge 3 commits intovalidatorjs:masterfrom
Conversation
…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 Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
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.
|
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 |
|
Thanks for the review @WikiRik! Happy to simplify this PR to just the new large-string test cases and revert the implementation change. |
Per maintainer feedback: reverted isBase64.js to original regex implementation. Kept new large-string test cases for regression coverage.
|
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. |
Description
Adds regression test coverage for large base64 strings as reported
in #2573.
Tests added
All 318 tests passing, 100% statement/line/function coverage.
Checklist
Refs #2573