From a0d44026fc3243711c2047c886ad16b0b0b633ba Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Tue, 11 Oct 2022 14:38:09 +0100 Subject: [PATCH] AK+Tests: Correct off-by-one error when right-trimming text If the entire string you want to right-trim consists of characters you want to remove, we previously would incorrectly leave the first character there. For example: `trim("aaaaa", "a")` would return "a" instead of "". We can't use `i >= 0` in the loop since that would fail to detect underflow, so instead we keep `i` in the range `size .. 1` and then subtract 1 from it when reading the character. Added some trim() tests while I was at it. (And to confirm that this was the issue.) --- AK/StringUtils.cpp | 4 ++-- Tests/AK/TestStringUtils.cpp | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/AK/StringUtils.cpp b/AK/StringUtils.cpp index bc2ae22e7a..e283df0ce3 100644 --- a/AK/StringUtils.cpp +++ b/AK/StringUtils.cpp @@ -338,10 +338,10 @@ StringView trim(StringView str, StringView characters, TrimMode mode) } if (mode == TrimMode::Right || mode == TrimMode::Both) { - for (size_t i = str.length() - 1; i > 0; --i) { + for (size_t i = str.length(); i > 0; --i) { if (substring_length == 0) return ""sv; - if (!characters.contains(str[i])) + if (!characters.contains(str[i - 1])) break; --substring_length; } diff --git a/Tests/AK/TestStringUtils.cpp b/Tests/AK/TestStringUtils.cpp index 356024ce07..a06227c137 100644 --- a/Tests/AK/TestStringUtils.cpp +++ b/Tests/AK/TestStringUtils.cpp @@ -338,6 +338,17 @@ TEST_CASE(is_whitespace) EXPECT(!AK::StringUtils::is_whitespace("a\t"sv)); } +TEST_CASE(trim) +{ + EXPECT_EQ(AK::StringUtils::trim("aaa.a."sv, "."sv, TrimMode::Right), "aaa.a"sv); + EXPECT_EQ(AK::StringUtils::trim("...aaa"sv, "."sv, TrimMode::Left), "aaa"sv); + EXPECT_EQ(AK::StringUtils::trim("...aaa.a..."sv, "."sv, TrimMode::Both), "aaa.a"sv); + EXPECT_EQ(AK::StringUtils::trim("."sv, "."sv, TrimMode::Right), ""sv); + EXPECT_EQ(AK::StringUtils::trim("."sv, "."sv, TrimMode::Left), ""sv); + EXPECT_EQ(AK::StringUtils::trim("."sv, "."sv, TrimMode::Both), ""sv); + EXPECT_EQ(AK::StringUtils::trim("..."sv, "."sv, TrimMode::Both), ""sv); +} + TEST_CASE(find) { String test_string = "1234567";