Browse Source

LibJS: Throw RangeError in `StringPrototype::repeat` if OOM

currently crashes with an assertion failure in `String::repeated` if
malloc can't serve a `count * input_size` sized request, so add
`String::repeated_with_error` to propagate the error.
Jess 1 year ago
parent
commit
ecb7d4b40f

+ 5 - 3
AK/String.cpp

@@ -308,13 +308,14 @@ bool String::equals_ignoring_ascii_case(StringView other) const
     return StringUtils::equals_ignoring_ascii_case(bytes_as_string_view(), other);
 }
 
-String String::repeated(String const& input, size_t count)
+ErrorOr<String> String::repeated(String const& input, size_t count)
 {
-    VERIFY(!Checked<size_t>::multiplication_would_overflow(count, input.bytes().size()));
+    if (Checked<size_t>::multiplication_would_overflow(count, input.bytes().size()))
+        return Error::from_errno(EOVERFLOW);
 
     String result;
     size_t input_size = input.bytes().size();
-    MUST(result.replace_with_new_string(count * input_size, [&](Bytes buffer) {
+    TRY(result.replace_with_new_string(count * input_size, [&](Bytes buffer) {
         if (input_size == 1) {
             buffer.fill(input.bytes().first());
         } else {
@@ -323,6 +324,7 @@ String String::repeated(String const& input, size_t count)
         }
         return ErrorOr<void> {};
     }));
+
     return result;
 }
 

+ 1 - 1
AK/String.h

@@ -79,7 +79,7 @@ public:
     static ErrorOr<String> repeated(u32 code_point, size_t count);
 
     // Creates a new String from another string, repeated N times.
-    static String repeated(String const&, size_t count);
+    static ErrorOr<String> repeated(String const&, size_t count);
 
     // Creates a new String by case-transforming this String. Using these methods require linking LibUnicode into your application.
     ErrorOr<String> to_lowercase(Optional<StringView> const& locale = {}) const;

+ 1 - 0
Userland/Libraries/LibJS/Runtime/ErrorTypes.h

@@ -230,6 +230,7 @@
     M(StringNonGlobalRegExp, "RegExp argument is non-global")                                                                           \
     M(StringRawCannotConvert, "Cannot convert property 'raw' to object from {}")                                                        \
     M(StringRepeatCountMustBe, "repeat count must be a {} number")                                                                      \
+    M(StringRepeatCountMustNotOverflow, "repeat count must not overflow")                                                               \
     M(TemporalAmbiguousMonthOfPlainMonthDay, "Accessing month of PlainMonthDay is ambiguous, use monthCode instead")                    \
     M(TemporalDifferentCalendars, "Cannot compare dates from two different calendars")                                                  \
     M(TemporalDifferentTimeZones, "Cannot compare dates from two different time zones")                                                 \

+ 5 - 1
Userland/Libraries/LibJS/Runtime/StringPrototype.cpp

@@ -776,8 +776,12 @@ JS_DEFINE_NATIVE_FUNCTION(StringPrototype::repeat)
     if (string.is_empty())
         return PrimitiveString::create(vm, String {});
 
+    auto repeated = String::repeated(string, n);
+    if (repeated.is_error())
+        return vm.throw_completion<RangeError>(ErrorType::StringRepeatCountMustNotOverflow);
+
     // 6. Return the String value that is made from n copies of S appended together.
-    return PrimitiveString::create(vm, String::repeated(string, n));
+    return PrimitiveString::create(vm, repeated.release_value());
 }
 
 // 22.1.3.19 String.prototype.replace ( searchValue, replaceValue ), https://tc39.es/ecma262/#sec-string.prototype.replace

+ 4 - 0
Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.repeat.js

@@ -22,6 +22,10 @@ test("throws correct range errors", () => {
     expect(() => {
         "foo".repeat(Infinity);
     }).toThrowWithMessage(RangeError, "repeat count must be a finite number");
+
+    expect(() => {
+        "foo".repeat(0xffffffffff);
+    }).toThrowWithMessage(RangeError, "repeat count must not overflow");
 });
 
 test("UTF-16", () => {