util/string: LineReader should only trim \r or \r\n
What the developer wrote
Authored by Matthew Zipkin
util/string: LineReader should only trim \r or \r\n
The utility can not be opinionated about CR or SP on either end of a
line it reads. That decision is up to the caller and in fact in the
case of HTTP should be allowed in some places and rejected in others.
Replace TrimStringView() with more surgical operation.
What changed, and why it matters
This commit changes a low-level string utility so it no longer strips spaces from the beginning and end of lines it reads. It now only removes the actual line-ending characters (carriage return and newline). The change is described as a correctness fix so callers can decide for themselves whether spaces are allowed. It is not presented as a security fix, but overly aggressive trimming could in principle hide or alter data in network messages such as HTTP headers.
Review all callers of LineReader to ensure they now explicitly validate or trim leading/trailing spaces where required, especially HTTP and P2P protocol parsing paths. Consider whether any existing callers relied on the old trimming behavior for security checks.
Security signals we found
Behavior change in line-reading utility used by network/protocol parsing
Commit message explicitly references HTTP parsing and caller-controlled acceptance/rejection of CR/SP
Potential for whitespace-sensitive parsing (e.g., HTTP headers, Bitcoin protocol messages) to be affected
No CVE, advisory, or vendor security framing in commit or supplied references
Evidence from the diff
LineReader::ReadLine() previously called TrimStringView() on each line, which removed leading/trailing whitespace including spaces and tabs. The patch replaces that with two RemoveSuffixView() calls that strip only a trailing ‘\n’ and then a trailing ‘\r’. Tests are updated to show spaces are preserved and that ‘\r\r\n’ yields ‘\r’ as the first line. The commit message frames this as a caller-responsibility issue, specifically mentioning HTTP parsing where CR/SP handling should be accepted or rejected by the caller.
Changed components
src/util/string.cppsrc/test/util_string_tests.cppLineReader::ReadLine()Inspect captured patch +11 / −4
diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
index 3835904d..894c592e 100644
--- a/src/test/util_string_tests.cpp
+++ b/src/test/util_string_tests.cpp
@@ -174,7 +174,7 @@ BOOST_AUTO_TEST_CASE(case_insensitive_equal_test)
BOOST_AUTO_TEST_CASE(line_reader_test)
{
{
- // Check three lines terminated by \n and \r\n, trimming whitespace
+ // Check three lines terminated by \n and \r\n, preserving whitespace
std::string_view input = "once upon a time\n there was a dog \r\nwho liked food\n";
LineReader reader(input, /*max_line_length=*/128);
BOOST_CHECK_EQUAL(reader.Consumed(), 0);
@@ -192,7 +192,7 @@ BOOST_AUTO_TEST_CASE(line_reader_test)
BOOST_CHECK(line3);
BOOST_CHECK(!line4);
BOOST_CHECK_EQUAL(line1.value(), "once upon a time");
- BOOST_CHECK_EQUAL(line2.value(), "there was a dog");
+ BOOST_CHECK_EQUAL(line2.value(), " there was a dog ");
BOOST_CHECK_EQUAL(line3.value(), "who liked food");
BOOST_CHECK_EQUAL(reader.Consumed(), 51);
BOOST_CHECK_EQUAL(reader.Remaining(), 0);
@@ -229,6 +229,13 @@ BOOST_AUTO_TEST_CASE(line_reader_test)
LineReader reader(input, /*max_line_length=*/1024);
BOOST_CHECK(!reader.ReadLine());
}
+ {
+ // Don't trim any more than \r\n
+ std::string_view input = "\r\r\n";
+ LineReader reader(input, /*max_line_length=*/1024);
+ BOOST_CHECK_EQUAL(reader.ReadLine(), "\r");
+ BOOST_CHECK(!reader.ReadLine());
+ }
{
// Even one character is too long, if it's not \n
std::string_view input = "ab\n";
diff --git a/src/util/string.cpp b/src/util/string.cpp
index 42d1418a..a203bc1b 100644
--- a/src/util/string.cpp
+++ b/src/util/string.cpp
@@ -37,8 +37,8 @@ std::optional<std::string_view> LineReader::ReadLine()
// The \n itself does not count against max_line_length.
if (c == '\n') {
const std::string_view untrimmed_line(reinterpret_cast<const char*>(std::to_address(line_start)), count);
- const std::string_view line = TrimStringView(untrimmed_line); // delete leading and trailing whitespace including \r and \n
- return line;
+ std::string_view line = RemoveSuffixView(untrimmed_line, "\n");
+ return RemoveSuffixView(line, "\r");
}
// If the character we just consumed gives us a line length greater
// than max_line_length, and we are not at the end of the line (or buffer) yet,
Why this scored 35/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.