From 47052d6134a970f40d4b7e364a3e0391be057873 Mon Sep 17 00:00:00 2001 From: Eric Voskuil Date: Fri, 16 Oct 2026 18:07:16 -0400 Subject: [PATCH] Fix json reader stall on http body trailer. --- .../network/impl/messages/json_body.ipp | 16 ++++- .../bitcoin/network/messages/json_body.hpp | 8 +++ src/messages/rpc/body.cpp | 28 ++++++-- test/messages/json_body_reader.cpp | 44 ++++++++++++ test/messages/rpc/body_reader.cpp | 68 +++++++++++++++++++ 5 files changed, 157 insertions(+), 7 deletions(-) diff --git a/include/bitcoin/network/impl/messages/json_body.ipp b/include/bitcoin/network/impl/messages/json_body.ipp index e9168f3db..4dce421e7 100644 --- a/include/bitcoin/network/impl/messages/json_body.ipp +++ b/include/bitcoin/network/impl/messages/json_body.ipp @@ -75,7 +75,21 @@ size_t CLASS::reader::put(const buffer_type& buffer, boost_code& ec) NOEXCEPT try { const auto data = pointer_cast(buffer.data()); - const auto parsed = parser_.write_some(data, buffer.size(), ec); + size_t parsed{}; + + // The parser stops after the document, beast re-feeds the remainder. + if (parser_.done()) + { + while (parsed < size && is_json_whitespace(data[parsed])) + ++parsed; + + if (parsed < size) + ec = to_json_code(json_error_t::extra_data); + } + else + { + parsed = parser_.write_some(data, size, ec); + } total_ = ceilinged_add(total_, parsed); if (!ec && total_ > expected_.value_or(max_size_t)) diff --git a/include/bitcoin/network/messages/json_body.hpp b/include/bitcoin/network/messages/json_body.hpp index 37697ff03..0815513b0 100644 --- a/include/bitcoin/network/messages/json_body.hpp +++ b/include/bitcoin/network/messages/json_body.hpp @@ -25,6 +25,14 @@ namespace libbitcoin { namespace network { namespace json { +constexpr bool is_json_whitespace(char character) NOEXCEPT +{ + return character == ' ' + || character == '\t' + || character == '\r' + || character == '\n'; +} + /// Content passed to/from reader/writer via request/response. /// `static uint64_t size(const value_type&)` must be defined for beast to /// produce `content_length`, otherwise the response is chunked. Predeter- diff --git a/src/messages/rpc/body.cpp b/src/messages/rpc/body.cpp index 994fff230..147f593c2 100644 --- a/src/messages/rpc/body.cpp +++ b/src/messages/rpc/body.cpp @@ -27,6 +27,7 @@ namespace rpc { using namespace system; using namespace network::error; +using json::is_json_whitespace; BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT) BC_PUSH_WARNING(NO_UNGUARDED_POINTERS) @@ -44,12 +45,6 @@ constexpr bool is_electrum_terminator(char character) NOEXCEPT return character == '\n'; } -constexpr bool is_json_whitespace(char character) NOEXCEPT -{ - return character == ' ' || character == '\t' || - character == '\r' || character == '\n'; -} - // rpc::body::reader // ---------------------------------------------------------------------------- // A batch is never materialized. Each read yields one element (message), with @@ -147,6 +142,9 @@ put(const buffer_type& buffer, boost_code& ec) NOEXCEPT return terminated_ && (closed_ || (!value_.batch && !opened_)); }; + // A completed element is re-fed (beast) only with its trailer. + const auto completed = started_ && base::reader::done(); + // Prologue: consume whitespace and batch delimiters until element start, // batch close, or buffer exhaustion. if (!started_ && !closed_) @@ -262,6 +260,24 @@ put(const buffer_type& buffer, boost_code& ec) NOEXCEPT } } + // Trailer: consume whitespace trailing a non-terminated singleton (http), + // other characters start the next message (stream) unless re-fed. + if (!terminated_ && !opened_ && !value_.batch && started_ && + base::reader::done()) + { + for (; index < size; ++index) + { + if (is_json_whitespace(data[index])) + continue; + + if (!completed) + break; + + ec = code{ error::jsonrpc_reader_stall }; + return index; + } + } + // Terminator: consume whitespace through the terminator, allowing padded // termination (electrum traffic shaping) and split reads. if (requires_terminator() && !has_terminator_) diff --git a/test/messages/json_body_reader.cpp b/test/messages/json_body_reader.cpp index 8eacd960d..1a2be54ef 100644 --- a/test/messages/json_body_reader.cpp +++ b/test/messages/json_body_reader.cpp @@ -66,6 +66,50 @@ BOOST_AUTO_TEST_CASE(json_body_reader__put__simple_object__success_expected_cons BOOST_REQUIRE(!ec); } +BOOST_AUTO_TEST_CASE(json_body_reader__put__split_whitespace_after_object__consumed) +{ + const std::string_view document{ R"({"key":"value"})" }; + const std::string_view whitespace{ "\r\n" }; + const asio::const_buffer buffer1{ document.data(), document.size() }; + const asio::const_buffer buffer2{ whitespace.data(), whitespace.size() }; + json::body<>::value_type body{}; + json::body<>::reader reader(body); + boost_code ec{}; + reader.init(document.size() + whitespace.size(), ec); + BOOST_REQUIRE(!ec); + + BOOST_REQUIRE_EQUAL(reader.put(buffer1, ec), document.size()); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE(reader.done()); + + BOOST_REQUIRE_EQUAL(reader.put(buffer2, ec), whitespace.size()); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE(reader.done()); + + reader.finish(ec); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE_EQUAL(body.model.at("key").as_string(), "value"); +} + +BOOST_AUTO_TEST_CASE(json_body_reader__put__split_garbage_after_object__extra_data) +{ + const std::string_view document{ R"({"key":"value"})" }; + const std::string_view garbage{ " x" }; + const asio::const_buffer buffer1{ document.data(), document.size() }; + const asio::const_buffer buffer2{ garbage.data(), garbage.size() }; + json::body<>::value_type body{}; + json::body<>::reader reader(body); + boost_code ec{}; + reader.init(document.size() + garbage.size(), ec); + BOOST_REQUIRE(!ec); + + BOOST_REQUIRE_EQUAL(reader.put(buffer1, ec), document.size()); + BOOST_REQUIRE(!ec); + + BOOST_REQUIRE_EQUAL(reader.put(buffer2, ec), 1u); + BOOST_REQUIRE(ec == error::json_error_t::extra_data); +} + BOOST_AUTO_TEST_CASE(json_body_reader__finish__simple_object__success_expected_model) { const std::string_view text{ R"({"key":"value"})" }; diff --git a/test/messages/rpc/body_reader.cpp b/test/messages/rpc/body_reader.cpp index 29f5b756c..724e44811 100644 --- a/test/messages/rpc/body_reader.cpp +++ b/test/messages/rpc/body_reader.cpp @@ -652,6 +652,74 @@ BOOST_AUTO_TEST_CASE(rpc_body_reader__put__garbage_where_terminator_expected__re BOOST_REQUIRE(ec == error::jsonrpc_reader_stall); } +BOOST_AUTO_TEST_CASE(rpc_body_reader__put__split_whitespace_after_non_terminated_singleton__consumed) +{ + const std::string_view document{ R"({"jsonrpc":"2.0","id":1,"method":"test"})" }; + const std::string_view whitespace{ "\r\n" }; + const asio::const_buffer buffer1{ document.data(), document.size() }; + const asio::const_buffer buffer2{ whitespace.data(), whitespace.size() }; + request_header header{}; + rpc::request_body::value_type body{}; + rpc::request_body::reader reader(header, body); + boost_code ec{}; + reader.init({}, ec); + BOOST_REQUIRE(!ec); + + BOOST_REQUIRE_EQUAL(reader.put(buffer1, ec), document.size()); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE(reader.done()); + + BOOST_REQUIRE_EQUAL(reader.put(buffer2, ec), whitespace.size()); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE(reader.done()); + + reader.finish(ec); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE_EQUAL(body.message.method, "test"); +} + +BOOST_AUTO_TEST_CASE(rpc_body_reader__put__split_garbage_after_non_terminated_singleton__reader_stall) +{ + const std::string_view document{ R"({"jsonrpc":"2.0","id":1,"method":"test"})" }; + const std::string_view garbage{ " x" }; + const asio::const_buffer buffer1{ document.data(), document.size() }; + const asio::const_buffer buffer2{ garbage.data(), garbage.size() }; + request_header header{}; + rpc::request_body::value_type body{}; + rpc::request_body::reader reader(header, body); + boost_code ec{}; + reader.init({}, ec); + BOOST_REQUIRE(!ec); + + BOOST_REQUIRE_EQUAL(reader.put(buffer1, ec), document.size()); + BOOST_REQUIRE(!ec); + + BOOST_REQUIRE_EQUAL(reader.put(buffer2, ec), 1u); + BOOST_REQUIRE(ec == error::jsonrpc_reader_stall); +} + +BOOST_AUTO_TEST_CASE(rpc_body_reader__put__garbage_within_non_terminated_singleton__reader_stall) +{ + const std::string_view text{ R"({"jsonrpc":"2.0","id":1,"method":"test"}x)" }; + const auto document = text.substr(0, sub1(text.size())); + const auto garbage = text.substr(document.size()); + const asio::const_buffer buffer1{ text.data(), text.size() }; + const asio::const_buffer buffer2{ garbage.data(), garbage.size() }; + request_header header{}; + rpc::request_body::value_type body{}; + rpc::request_body::reader reader(header, body); + boost_code ec{}; + reader.init({}, ec); + BOOST_REQUIRE(!ec); + + BOOST_REQUIRE_EQUAL(reader.put(buffer1, ec), document.size()); + BOOST_REQUIRE(!ec); + BOOST_REQUIRE(reader.done()); + + BOOST_REQUIRE_EQUAL(reader.put(buffer2, ec), 0u); + BOOST_REQUIRE(ec == error::jsonrpc_reader_stall); +} + // lax // ----------------------------------------------------------------------------