From 5a8373c6b9ea8bd4cf78c467da295dd5217699b2 Mon Sep 17 00:00:00 2001 From: Rodrigo Tobar Date: Fri, 17 Mar 2023 00:46:21 +0800 Subject: [PATCH] LibCore: Fix corner case for files without newlines When BufferedFile.can_read_line() was invoked on files with no newlines, t incorrectly returned a false result for this single line that, even though doesn't finish with a newline character, is still a line. Since this method is usually used in tandem with read_line(), users would miss reading this line (and hence all the file contents). This commit fixes this corner case by adding another check after a negative result from finding a newline character. This new check does the same as the check that is done *before* looking for newlines, which takes care of this problem, but only works for files that have at least one newline (hence the buffer has already been filled). A new unit test has been added that shows the use case. Without the changes in this commit the test fails, which is a testament that this commit really fixes the underlying issue. --- AK/BufferedStream.h | 5 ++++- Tests/LibCore/TestLibCoreStream.cpp | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/AK/BufferedStream.h b/AK/BufferedStream.h index c0e9512958..927c3db4d9 100644 --- a/AK/BufferedStream.h +++ b/AK/BufferedStream.h @@ -193,7 +193,10 @@ public: if (stream().is_eof()) return m_buffer.used_space() > 0; - return TRY(find_and_populate_until_any_of(Array { "\n"sv })).has_value(); + auto maybe_match = TRY(find_and_populate_until_any_of(Array { "\n"sv })); + if (maybe_match.has_value()) + return true; + return stream().is_eof() && m_buffer.used_space() > 0; } bool is_eof() const diff --git a/Tests/LibCore/TestLibCoreStream.cpp b/Tests/LibCore/TestLibCoreStream.cpp index 23003a1a11..0a81955fa7 100644 --- a/Tests/LibCore/TestLibCoreStream.cpp +++ b/Tests/LibCore/TestLibCoreStream.cpp @@ -553,6 +553,24 @@ TEST_CASE(buffered_file_tell_and_seek) } } +constexpr auto new_newlines_message = "Hi, look, no newlines"sv; + +TEST_CASE(buffered_file_without_newlines) +{ + constexpr auto filename = "/tmp/file-without-newlines"sv; + auto file_wo_newlines = Core::File::open(filename, Core::File::OpenMode::Write).release_value(); + EXPECT(!file_wo_newlines->write_until_depleted(new_newlines_message.bytes()).is_error()); + file_wo_newlines->close(); + + auto ro_file = Core::BufferedFile::create(Core::File::open(filename, Core::File::OpenMode::Read).release_value(), new_newlines_message.length() + 1).release_value(); + + auto maybe_can_read_line = ro_file->can_read_line(); + EXPECT(!maybe_can_read_line.is_error()); + EXPECT(maybe_can_read_line.release_value()); + Array buffer; + EXPECT(ro_file->read_line(buffer).release_value() == new_newlines_message); +} + constexpr auto buffered_sent_data = "Well hello friends!\n:^)\nThis shouldn't be present. :^("sv; constexpr auto first_line = "Well hello friends!"sv; constexpr auto second_line = ":^)"sv;