diff --git a/CHANGELOG.md b/CHANGELOG.md index f3980aac..8152ecaa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ 1.0.0-b18 +HTTP + +* Check invariants in parse_op: * Clean up message docs + +Extras + * unit_test::suite fixes: - New overload of fail() specifies file and line - BEAST_EXPECTS only evaluates the reason string on a failure diff --git a/include/beast/core/impl/basic_streambuf.ipp b/include/beast/core/impl/basic_streambuf.ipp index f59d00df..e1f51e50 100644 --- a/include/beast/core/impl/basic_streambuf.ipp +++ b/include/beast/core/impl/basic_streambuf.ipp @@ -857,12 +857,17 @@ std::size_t read_size_helper(basic_streambuf< Allocator> const& streambuf, std::size_t max_size) { + BOOST_ASSERT(max_size >= 1); + // If we already have an allocated + // buffer, try to fill that up first auto const avail = streambuf.capacity() - streambuf.size(); if (avail > 0) return std::min(avail, max_size); + // Try to have just one new block allocated constexpr std::size_t low = 512; if (streambuf.alloc_size_ > low) return std::min(max_size, streambuf.alloc_size_); + // ...but enforce a 512 byte minimum. return std::min(max_size, low); } diff --git a/include/beast/http/impl/parse.ipp b/include/beast/http/impl/parse.ipp index 006d9848..6a717de5 100644 --- a/include/beast/http/impl/parse.ipp +++ b/include/beast/http/impl/parse.ipp @@ -32,7 +32,7 @@ class parse_op DynamicBuffer& db; Parser& p; Handler h; - bool started = false; + bool got_some = false; bool cont; int state = 0; @@ -46,6 +46,7 @@ class parse_op , cont(boost_asio_handler_cont_helpers:: is_continuation(h)) { + BOOST_ASSERT(! p.complete()); } }; @@ -113,6 +114,7 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again) { case 0: { + // Parse any bytes left over in the buffer auto const used = d.p.write(d.db.data(), ec); if(ec) @@ -124,8 +126,10 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again) return; } if(used > 0) - d.started = true; - d.db.consume(used); + { + d.got_some = true; + d.db.consume(used); + } if(d.p.complete()) { // call handler @@ -134,30 +138,41 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again) bind_handler(std::move(*this), ec, 0)); return; } + // Buffer must be empty, + // otherwise parse should be complete + BOOST_ASSERT(d.db.size() == 0); d.state = 1; break; } case 1: + { // read d.state = 2; - d.s.async_read_some(d.db.prepare( - read_size_helper(d.db, 65536)), - std::move(*this)); + auto const size = + read_size_helper(d.db, 65536); + BOOST_ASSERT(size > 0); + d.s.async_read_some( + d.db.prepare(size), std::move(*this)); return; + } // got data case 2: { if(ec == boost::asio::error::eof) { - if(! d.started) + // If we haven't processed any bytes, + // give the eof to the handler immediately. + if(! d.got_some) { // call handler d.state = 99; break; } - // Caller will see eof on next read. + // Feed the eof to the parser to complete + // the parse, and call the handler. The + // next call to parse will deliver the eof. ec = {}; d.p.write_eof(ec); BOOST_ASSERT(ec || d.p.complete()); @@ -171,6 +186,7 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again) d.state = 99; break; } + BOOST_ASSERT(bytes_transferred > 0); d.db.commit(bytes_transferred); auto const used = d.p.write(d.db.data(), ec); if(ec) @@ -179,8 +195,10 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again) d.state = 99; break; } - if(used > 0) - d.started = true; + // The parser must either consume + // bytes or generate an error. + BOOST_ASSERT(used > 0); + d.got_some = true; d.db.consume(used); if(d.p.complete()) { @@ -188,6 +206,9 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again) d.state = 99; break; } + // If the parse is not complete, + // all input must be consumed. + BOOST_ASSERT(used == bytes_transferred); d.state = 1; break; } @@ -228,7 +249,7 @@ parse(SyncReadStream& stream, DynamicBuffer& dynabuf, "DynamicBuffer requirements not met"); static_assert(is_Parser::value, "Parser requirements not met"); - bool started = false; + bool got_some = false; for(;;) { auto used = @@ -237,7 +258,7 @@ parse(SyncReadStream& stream, DynamicBuffer& dynabuf, return; dynabuf.consume(used); if(used > 0) - started = true; + got_some = true; if(parser.complete()) break; dynabuf.commit(stream.read_some( @@ -247,7 +268,7 @@ parse(SyncReadStream& stream, DynamicBuffer& dynabuf, return; if(ec == boost::asio::error::eof) { - if(! started) + if(! got_some) return; // Caller will see eof on next read. ec = {}; diff --git a/test/core/basic_streambuf.cpp b/test/core/basic_streambuf.cpp index bf1c4d39..00b468dd 100644 --- a/test/core/basic_streambuf.cpp +++ b/test/core/basic_streambuf.cpp @@ -403,7 +403,6 @@ public: { streambuf sb{10}; BEAST_EXPECT(sb.alloc_size() == 10); - BEAST_EXPECT(read_size_helper(sb, 0) == 0); BEAST_EXPECT(read_size_helper(sb, 1) == 1); BEAST_EXPECT(read_size_helper(sb, 10) == 10); BEAST_EXPECT(read_size_helper(sb, 20) == 20); @@ -416,22 +415,18 @@ public: { streambuf sb(1000); BEAST_EXPECT(sb.alloc_size() == 1000); - BEAST_EXPECT(read_size_helper(sb, 0) == 0); BEAST_EXPECT(read_size_helper(sb, 1) == 1); BEAST_EXPECT(read_size_helper(sb, 1000) == 1000); BEAST_EXPECT(read_size_helper(sb, 2000) == 1000); sb.prepare(3); - BEAST_EXPECT(read_size_helper(sb, 0) == 0); BEAST_EXPECT(read_size_helper(sb, 1) == 1); BEAST_EXPECT(read_size_helper(sb, 1000) == 1000); BEAST_EXPECT(read_size_helper(sb, 2000) == 1000); sb.commit(3); - BEAST_EXPECT(read_size_helper(sb, 0) == 0); BEAST_EXPECT(read_size_helper(sb, 1) == 1); BEAST_EXPECT(read_size_helper(sb, 1000) == 997); BEAST_EXPECT(read_size_helper(sb, 2000) == 997); sb.consume(2); - BEAST_EXPECT(read_size_helper(sb, 0) == 0); BEAST_EXPECT(read_size_helper(sb, 1) == 1); BEAST_EXPECT(read_size_helper(sb, 1000) == 997); BEAST_EXPECT(read_size_helper(sb, 2000) == 997);