From 424c7eaa408105af7fc8129d262cb5d4ab70631b Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Mon, 5 Jul 2021 21:01:05 -0400 Subject: [PATCH] LibJS: Fix replaceAll crash for overlapping search string positions The implementation of String.prototype.replaceAll cannot use AK's implementation of String::find_all when finding the indices of the search string in the source string. String::find_all will return indices [0, 1] for String("aaa").find_all("aa") - i.e. it returns overlapping results. This is not allowed by the JavaScript specification for replaceAll. --- Userland/Libraries/LibJS/Runtime/StringPrototype.cpp | 11 +++++++++-- .../builtins/String/String.prototype.replaceAll.js | 5 +++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp b/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp index 9dbe65f48e..d94161723a 100644 --- a/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp @@ -877,9 +877,16 @@ JS_DEFINE_NATIVE_FUNCTION(StringPrototype::replace_all) if (vm.exception()) return {}; - Vector match_positions = string.find_all(search_string); - size_t end_of_last_match = 0; + Vector match_positions; + size_t advance_by = max(1u, search_string.length()); + auto position = string.find(search_string); + while (position.has_value()) { + match_positions.append(*position); + position = string.find(search_string, *position + advance_by); + } + + size_t end_of_last_match = 0; StringBuilder result; for (auto position : match_positions) { diff --git a/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.replaceAll.js b/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.replaceAll.js index eab1d9d1ea..11aaab2371 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.replaceAll.js +++ b/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.replaceAll.js @@ -59,6 +59,11 @@ test("basic regex replacement", () => { expect("abc123def".replaceAll(/\D/g, "*")).toBe("***123***"); expect("123abc456".replaceAll(/\D/g, "*")).toBe("123***456"); + + expect("aaab a a aac".replaceAll("aa", "z")).toBe("zab a a zc"); + expect("aaab a a aac".replaceAll("aa", "a")).toBe("aab a a ac"); + expect("aaab a a aac".replaceAll("a", "a")).toBe("aaab a a aac"); + expect("aaab a a aac".replaceAll("a", "z")).toBe("zzzb z z zzc"); }); test("functional regex replacement", () => {