fix(string): add maximum length limit to JsString to prevent OOM crashes#5277
fix(string): add maximum length limit to JsString to prevent OOM crashes#5277abhinavs1920 wants to merge 3 commits intoboa-dev:mainfrom
Conversation
Test262 conformance changes
Tested main commit: |
Signed-off-by: Abhinav Sharma <abhinavs1920bpl@gmail.com>
Signed-off-by: Abhinav Sharma <abhinavs1920bpl@gmail.com>
|
Heyy @jedel1043 @nekevss @!
This shows up in multiple places around string concatenation, and similar For now, I’ve kept it as-is to stay consistent with the current style. But I was thinking, has there been any discussion around introducing a small helper (or trait) to reduce this kind of boilerplate? Something like:
Totally understand if the verbosity is intentional for clarity, just wanted to check if this is something worth exploring further as a follow-up. Thanks :) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5277 +/- ##
===========================================
+ Coverage 47.24% 59.63% +12.39%
===========================================
Files 476 589 +113
Lines 46892 63494 +16602
===========================================
+ Hits 22154 37865 +15711
- Misses 24738 25629 +891 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Abhinav Sharma <abhinavs1920bpl@gmail.com>
there kinda is but its not a lot better. see Line 70 in cd1a386 |
I thought there were other places where a new |
This Pull Request fixes/closes #4409 .
It changes the following:
MAX_STRING_LENGTHconstant in the string crate.JsString::concatandconcat_array) fallible and check the limit before attempting allocation.+operator fast path andConcatToStringopcode to properly propagate theRangeError.String.prototype.concataccordingly.Why this approach
JsResult+?error handling patterns.