From 3b824ec8c92d48dd80ca0ffac1996eb1e7f4cede Mon Sep 17 00:00:00 2001 From: Lucas CHOLLET Date: Sun, 15 Jan 2023 17:14:11 -0500 Subject: [PATCH] AK+Test: Fix a logic error in `CircularBuffer::offset_of()` This error was introduced by 9a7accdd and had a significant impact on `BufferedFile` behavior. Hence, we started seeing crash in test262. By itself, the issue was a wrong calculation of the internal reading spans when using the `read` and `until` parameters. Which can lead to at worse crash in VERIFY and at least weird behaviors as missed needles or detections out of bounds. It was also accompanied by an erroneous test. This patch fixes the bug, the test and also provides more tests. --- AK/CircularBuffer.cpp | 6 +++--- Tests/AK/TestCircularBuffer.cpp | 37 ++++++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/AK/CircularBuffer.cpp b/AK/CircularBuffer.cpp index 7e38361a847..8c6034ccbc0 100644 --- a/AK/CircularBuffer.cpp +++ b/AK/CircularBuffer.cpp @@ -60,15 +60,15 @@ Optional CircularBuffer::offset_of(StringView needle, Optional f Array spans {}; spans[0] = next_read_span(); + auto const original_span_0_size = spans[0].size(); if (read_from > 0) spans[0] = spans[0].slice(min(spans[0].size(), read_from)); if (spans[0].size() + read_from > read_until) spans[0] = spans[0].trim(read_until - read_from); - - if (is_wrapping_around()) - spans[1] = m_buffer.span().slice(max(spans[0].size(), read_from) - spans[0].size(), min(read_until, m_used_space) - spans[0].size()); + else if (is_wrapping_around()) + spans[1] = m_buffer.span().slice(max(original_span_0_size, read_from) - original_span_0_size, min(read_until, m_used_space) - original_span_0_size); auto maybe_found = AK::memmem(spans.begin(), spans.end(), needle.bytes()); if (maybe_found.has_value()) diff --git a/Tests/AK/TestCircularBuffer.cpp b/Tests/AK/TestCircularBuffer.cpp index 8bab0233c24..5551d72d3ca 100644 --- a/Tests/AK/TestCircularBuffer.cpp +++ b/Tests/AK/TestCircularBuffer.cpp @@ -310,6 +310,41 @@ TEST_CASE(offset_of_with_until_and_after) result = circular_buffer.offset_of("o Frie"sv, 4, 10); EXPECT_EQ(result.value_or(42), 4ul); - result = circular_buffer.offset_of("el"sv, 3, 14); + result = circular_buffer.offset_of("el"sv, 3, 17); EXPECT_EQ(result.value_or(42), 15ul); } + +TEST_CASE(offset_of_with_until_and_after_wrapping_around) +{ + auto const source = "Well Hello Friends!"sv; + auto byte_buffer_or_error = ByteBuffer::copy(source.bytes()); + EXPECT(!byte_buffer_or_error.is_error()); + auto byte_buffer = byte_buffer_or_error.release_value(); + + auto circular_buffer_or_error = CircularBuffer::create_empty(19); + EXPECT(!circular_buffer_or_error.is_error()); + auto circular_buffer = circular_buffer_or_error.release_value(); + + auto written_bytes = circular_buffer.write(byte_buffer.span().trim(5)); + EXPECT_EQ(written_bytes, 5ul); + + auto result = circular_buffer.offset_of("Well "sv, 0, 5); + EXPECT_EQ(result.value_or(42), 0ul); + + written_bytes = circular_buffer.write(byte_buffer.span().slice(5)); + EXPECT_EQ(written_bytes, 14ul); + + result = circular_buffer.offset_of("Hello Friends!"sv, 5, 19); + EXPECT_EQ(result.value_or(42), 5ul); + + safe_discard(circular_buffer, 5); + + result = circular_buffer.offset_of("Hello Friends!"sv, 0, 14); + EXPECT_EQ(result.value_or(42), 0ul); + + written_bytes = circular_buffer.write(byte_buffer.span().trim(5)); + EXPECT_EQ(written_bytes, 5ul); + + result = circular_buffer.offset_of("Well "sv, 14, 19); + EXPECT_EQ(result.value_or(42), 14ul); +}