From 21ac431facd2ad3dedc2fd9057cb3a177254c01b Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Wed, 21 Feb 2024 19:42:27 -0700 Subject: [PATCH] AK: Allow reading from EOF buffered streams better in read_line() If the BufferedStream is able to fill its entire circular buffer in populate_read_buffer() and is later asked to read a line or read until a delimiter, it could erroneously return EMSGSIZE if the caller's buffer was smaller than the internal buffer. In this case, all we really care about is whether the caller's buffer is big enough for however much data we're going to copy into it. Which needs to take into account the candidate. --- AK/BufferedStream.h | 3 ++- Tests/AK/TestMemoryStream.cpp | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/AK/BufferedStream.h b/AK/BufferedStream.h index 2c1450d119..49c6df84ca 100644 --- a/AK/BufferedStream.h +++ b/AK/BufferedStream.h @@ -87,7 +87,8 @@ public: auto const candidate = TRY(find_and_populate_until_any_of(candidates, buffer.size())); if (stream().is_eof()) { - if (buffer.size() < m_buffer.used_space()) { + if ((candidate.has_value() && candidate->offset + candidate->size > buffer.size()) + || (!candidate.has_value() && buffer.size() < m_buffer.used_space())) { // Normally, reading from an EOFed stream and receiving bytes // would mean that the stream is no longer EOF. However, it's // possible with a buffered stream that the user is able to read diff --git a/Tests/AK/TestMemoryStream.cpp b/Tests/AK/TestMemoryStream.cpp index 306cab6908..0b7784b4a5 100644 --- a/Tests/AK/TestMemoryStream.cpp +++ b/Tests/AK/TestMemoryStream.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include #include @@ -270,3 +271,30 @@ TEST_CASE(fixed_memory_read_in_place) EXPECT_EQ(characters_again, some_words.bytes()); EXPECT(mutable_stream.is_eof()); } + +TEST_CASE(buffered_memory_stream_read_line) +{ + auto array = Array {}; + + // First line: 8 bytes, second line: 24 bytes + array.fill('A'); + array[7] = '\n'; + array[31] = '\n'; + + auto memory_stream = make(array.span(), FixedMemoryStream::Mode::ReadOnly); + + // Buffer for buffered seekable is larger than the stream, so stream goes EOF immediately on read + auto buffered_stream = TRY_OR_FAIL(InputBufferedSeekable::create(move(memory_stream), 64)); + + // Buffer is only 16 bytes, first read succeeds, second fails + auto buffer = TRY_OR_FAIL(ByteBuffer::create_zeroed(16)); + + auto read_bytes = TRY_OR_FAIL(buffered_stream->read_line(buffer)); + + EXPECT_EQ(read_bytes, "AAAAAAA"sv); + + auto read_or_error = buffered_stream->read_line(buffer); + + EXPECT(read_or_error.is_error()); + EXPECT_EQ(read_or_error.error().code(), EMSGSIZE); +}