From b6093ae2e362aa1f9cf7e87bc829b37b24250d25 Mon Sep 17 00:00:00 2001
From: Timothy Flynn <trflynn89@pm.me>
Date: Wed, 14 Apr 2021 17:46:18 -0400
Subject: [PATCH] LibJS: Implement String.prototype.substr according to the
 spec

Fixes #6325

The JavaScript on the HTML Spec site that caused the crash is:
    window.location.hash.substr(1)

Of course, window.location.hash can be the empty string. The spec allows
for calling substr(1) on an empty string, but our partial implementation
wasn't handling it properly.
---
 .../LibJS/Runtime/StringPrototype.cpp         | 31 ++++++++++---------
 .../String/String.prototype.substr.js         |  3 ++
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp b/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp
index b29a4b923d6b3c..5233d421168626 100644
--- a/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp
+++ b/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp
@@ -424,28 +424,31 @@ JS_DEFINE_NATIVE_FUNCTION(StringPrototype::substr)
         return js_string(vm, string);
 
     // FIXME: this should index a UTF-16 code_point view of the string.
-    auto string_length = (i32)string.length();
+    auto size = (i32)string.length();
 
-    auto start_argument = vm.argument(0).to_i32(global_object);
+    auto int_start = vm.argument(0).to_integer_or_infinity(global_object);
     if (vm.exception())
         return {};
+    if (Value(int_start).is_negative_infinity())
+        int_start = 0;
+    if (int_start < 0)
+        int_start = max(size + (i32)int_start, 0);
 
-    auto start = start_argument < 0 ? (string_length - -start_argument) : start_argument;
+    auto length = vm.argument(1);
 
-    auto length = string_length - start;
-    if (vm.argument_count() >= 2) {
-        auto length_argument = vm.argument(1).to_i32(global_object);
-        if (vm.exception())
-            return {};
-        length = max(0, min(length_argument, length));
-        if (vm.exception())
-            return {};
-    }
+    auto int_length = length.is_undefined() ? size : length.to_integer_or_infinity(global_object);
+    if (vm.exception())
+        return {};
+
+    if (Value(int_start).is_positive_infinity() || (int_length <= 0) || Value(int_length).is_positive_infinity())
+        return js_string(vm, String(""));
+
+    auto int_end = min((i32)(int_start + int_length), size);
 
-    if (length == 0)
+    if (int_start >= int_end)
         return js_string(vm, String(""));
 
-    auto string_part = string.substring(start, length);
+    auto string_part = string.substring(int_start, int_end - int_start);
     return js_string(vm, string_part);
 }
 
diff --git a/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.substr.js b/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.substr.js
index ade6c3c60a0c2b..b6d26f60234c76 100644
--- a/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.substr.js
+++ b/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.substr.js
@@ -1,6 +1,9 @@
 test("basic functionality", () => {
     expect(String.prototype.substr).toHaveLength(2);
 
+    expect("".substr(1)).toBe("");
+    expect("".substr()).toBe("");
+    expect("".substr(-1)).toBe("");
     expect("hello friends".substr()).toBe("hello friends");
     expect("hello friends".substr(1)).toBe("ello friends");
     expect("hello friends".substr(0, 5)).toBe("hello");