From 7e98c8eaf66698fb28a8c3ac825559ce380c1624 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Fri, 18 Mar 2022 18:02:07 +0000 Subject: [PATCH] AK+Tests: Fix StringUtils::contains() being confused by repeating text Previously, case-insensitively searching the haystack "Go Go Back" for the needle "Go Back" would return false: 1. Match the first three characters. "Go ". 2. Notice that 'G' and 'B' don't match. 3. Skip ahead 3 characters, plus 1 for the outer for-loop. 4. Now, the haystack is effectively "o Back", so the match fails. Reducing the skip by 1 fixes this issue. I'm not 100% convinced this fixes all cases, but I haven't been able to find any cases where it doesn't work now. :^) --- AK/StringUtils.cpp | 3 ++- Tests/AK/TestStringUtils.cpp | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/AK/StringUtils.cpp b/AK/StringUtils.cpp index cc73f203ae..927b3b6ee1 100644 --- a/AK/StringUtils.cpp +++ b/AK/StringUtils.cpp @@ -305,7 +305,8 @@ bool contains(StringView str, StringView needle, CaseSensitivity case_sensitivit continue; for (size_t ni = 0; si + ni < str.length(); ni++) { if (to_ascii_lowercase(str_chars[si + ni]) != to_ascii_lowercase(needle_chars[ni])) { - si += ni; + if (ni > 0) + si += ni - 1; break; } if (ni + 1 == needle.length()) diff --git a/Tests/AK/TestStringUtils.cpp b/Tests/AK/TestStringUtils.cpp index bff9aa5dcd..b82a73886d 100644 --- a/Tests/AK/TestStringUtils.cpp +++ b/Tests/AK/TestStringUtils.cpp @@ -321,6 +321,10 @@ TEST_CASE(contains) EXPECT(!AK::StringUtils::contains("", test_string, CaseSensitivity::CaseInsensitive)); EXPECT(!AK::StringUtils::contains(test_string, "L", CaseSensitivity::CaseSensitive)); EXPECT(!AK::StringUtils::contains(test_string, "L", CaseSensitivity::CaseInsensitive)); + + String command_palette_bug_string = "Go Go Back"; + EXPECT(AK::StringUtils::contains(command_palette_bug_string, "Go Back", AK::CaseSensitivity::CaseSensitive)); + EXPECT(AK::StringUtils::contains(command_palette_bug_string, "gO bAcK", AK::CaseSensitivity::CaseInsensitive)); } TEST_CASE(is_whitespace)