From 3f4e90f32bf563ff8e58a2b6cd6d84464c003113 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Mon, 12 Apr 2021 12:38:43 -0400 Subject: [PATCH] AK: Fix StringView::find_last_of for one-character views The find_last_of implementations were breaking out of the search loop too early for single-character string views. This caused a crash in CookieJar setting a cookie on google.com - CookieJar::default_path knew there was at least one "/" in a string view, but find_last_of returned nullopt, so dereferencing the optional caused a crash. Fixes #6273 --- AK/StringView.cpp | 12 ++++++------ AK/Tests/TestStringView.cpp | 6 ++++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/AK/StringView.cpp b/AK/StringView.cpp index 50a4c3b677..079cb71875 100644 --- a/AK/StringView.cpp +++ b/AK/StringView.cpp @@ -294,20 +294,20 @@ Optional StringView::find_first_of(const StringView& view) const Optional StringView::find_last_of(char c) const { - for (size_t pos = m_length; --pos > 0;) { - if (m_characters[pos] == c) - return pos; + for (size_t pos = m_length; pos != 0; --pos) { + if (m_characters[pos - 1] == c) + return pos - 1; } return {}; } Optional StringView::find_last_of(const StringView& view) const { - for (size_t pos = m_length - 1; --pos > 0;) { - char c = m_characters[pos]; + for (size_t pos = m_length; pos != 0; --pos) { + char c = m_characters[pos - 1]; for (char view_char : view) { if (c == view_char) - return pos; + return pos - 1; } } return {}; diff --git a/AK/Tests/TestStringView.cpp b/AK/Tests/TestStringView.cpp index 3a69070073..48a5709fbc 100644 --- a/AK/Tests/TestStringView.cpp +++ b/AK/Tests/TestStringView.cpp @@ -164,6 +164,12 @@ TEST_CASE(find_last_of) EXPECT_EQ(test_string_view.find_last_of('3').has_value(), false); EXPECT_EQ(test_string_view.find_last_of("fghi").has_value(), false); + + test_string_view = "/"; + EXPECT_EQ(test_string_view.find_last_of('/').has_value(), true); + EXPECT_EQ(test_string_view.find_last_of('/').value(), 0U); + EXPECT_EQ(test_string_view.find_last_of("/").has_value(), true); + EXPECT_EQ(test_string_view.find_last_of("/").value(), 0U); } TEST_CASE(split_view)