Check invariants in parse_op:

This adds more assertions, comments, and clarification.
This commit is contained in:
Vinnie Falco
2016-10-28 12:56:10 -04:00
parent 95a00e5f06
commit 1a53f52e34
4 changed files with 45 additions and 18 deletions

View File

@@ -1,6 +1,12 @@
1.0.0-b18 1.0.0-b18
HTTP
* Check invariants in parse_op:
* Clean up message docs * Clean up message docs
Extras
* unit_test::suite fixes: * unit_test::suite fixes:
- New overload of fail() specifies file and line - New overload of fail() specifies file and line
- BEAST_EXPECTS only evaluates the reason string on a failure - BEAST_EXPECTS only evaluates the reason string on a failure

View File

@@ -857,12 +857,17 @@ std::size_t
read_size_helper(basic_streambuf< read_size_helper(basic_streambuf<
Allocator> const& streambuf, std::size_t max_size) 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(); auto const avail = streambuf.capacity() - streambuf.size();
if (avail > 0) if (avail > 0)
return std::min(avail, max_size); return std::min(avail, max_size);
// Try to have just one new block allocated
constexpr std::size_t low = 512; constexpr std::size_t low = 512;
if (streambuf.alloc_size_ > low) if (streambuf.alloc_size_ > low)
return std::min(max_size, streambuf.alloc_size_); return std::min(max_size, streambuf.alloc_size_);
// ...but enforce a 512 byte minimum.
return std::min(max_size, low); return std::min(max_size, low);
} }

View File

@@ -32,7 +32,7 @@ class parse_op
DynamicBuffer& db; DynamicBuffer& db;
Parser& p; Parser& p;
Handler h; Handler h;
bool started = false; bool got_some = false;
bool cont; bool cont;
int state = 0; int state = 0;
@@ -46,6 +46,7 @@ class parse_op
, cont(boost_asio_handler_cont_helpers:: , cont(boost_asio_handler_cont_helpers::
is_continuation(h)) is_continuation(h))
{ {
BOOST_ASSERT(! p.complete());
} }
}; };
@@ -113,6 +114,7 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again)
{ {
case 0: case 0:
{ {
// Parse any bytes left over in the buffer
auto const used = auto const used =
d.p.write(d.db.data(), ec); d.p.write(d.db.data(), ec);
if(ec) if(ec)
@@ -124,8 +126,10 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again)
return; return;
} }
if(used > 0) if(used > 0)
d.started = true; {
d.db.consume(used); d.got_some = true;
d.db.consume(used);
}
if(d.p.complete()) if(d.p.complete())
{ {
// call handler // call handler
@@ -134,30 +138,41 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again)
bind_handler(std::move(*this), ec, 0)); bind_handler(std::move(*this), ec, 0));
return; return;
} }
// Buffer must be empty,
// otherwise parse should be complete
BOOST_ASSERT(d.db.size() == 0);
d.state = 1; d.state = 1;
break; break;
} }
case 1: case 1:
{
// read // read
d.state = 2; d.state = 2;
d.s.async_read_some(d.db.prepare( auto const size =
read_size_helper(d.db, 65536)), read_size_helper(d.db, 65536);
std::move(*this)); BOOST_ASSERT(size > 0);
d.s.async_read_some(
d.db.prepare(size), std::move(*this));
return; return;
}
// got data // got data
case 2: case 2:
{ {
if(ec == boost::asio::error::eof) 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 // call handler
d.state = 99; d.state = 99;
break; 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 = {}; ec = {};
d.p.write_eof(ec); d.p.write_eof(ec);
BOOST_ASSERT(ec || d.p.complete()); BOOST_ASSERT(ec || d.p.complete());
@@ -171,6 +186,7 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again)
d.state = 99; d.state = 99;
break; break;
} }
BOOST_ASSERT(bytes_transferred > 0);
d.db.commit(bytes_transferred); d.db.commit(bytes_transferred);
auto const used = d.p.write(d.db.data(), ec); auto const used = d.p.write(d.db.data(), ec);
if(ec) if(ec)
@@ -179,8 +195,10 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again)
d.state = 99; d.state = 99;
break; break;
} }
if(used > 0) // The parser must either consume
d.started = true; // bytes or generate an error.
BOOST_ASSERT(used > 0);
d.got_some = true;
d.db.consume(used); d.db.consume(used);
if(d.p.complete()) if(d.p.complete())
{ {
@@ -188,6 +206,9 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again)
d.state = 99; d.state = 99;
break; break;
} }
// If the parse is not complete,
// all input must be consumed.
BOOST_ASSERT(used == bytes_transferred);
d.state = 1; d.state = 1;
break; break;
} }
@@ -228,7 +249,7 @@ parse(SyncReadStream& stream, DynamicBuffer& dynabuf,
"DynamicBuffer requirements not met"); "DynamicBuffer requirements not met");
static_assert(is_Parser<Parser>::value, static_assert(is_Parser<Parser>::value,
"Parser requirements not met"); "Parser requirements not met");
bool started = false; bool got_some = false;
for(;;) for(;;)
{ {
auto used = auto used =
@@ -237,7 +258,7 @@ parse(SyncReadStream& stream, DynamicBuffer& dynabuf,
return; return;
dynabuf.consume(used); dynabuf.consume(used);
if(used > 0) if(used > 0)
started = true; got_some = true;
if(parser.complete()) if(parser.complete())
break; break;
dynabuf.commit(stream.read_some( dynabuf.commit(stream.read_some(
@@ -247,7 +268,7 @@ parse(SyncReadStream& stream, DynamicBuffer& dynabuf,
return; return;
if(ec == boost::asio::error::eof) if(ec == boost::asio::error::eof)
{ {
if(! started) if(! got_some)
return; return;
// Caller will see eof on next read. // Caller will see eof on next read.
ec = {}; ec = {};

View File

@@ -403,7 +403,6 @@ public:
{ {
streambuf sb{10}; streambuf sb{10};
BEAST_EXPECT(sb.alloc_size() == 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, 1) == 1);
BEAST_EXPECT(read_size_helper(sb, 10) == 10); BEAST_EXPECT(read_size_helper(sb, 10) == 10);
BEAST_EXPECT(read_size_helper(sb, 20) == 20); BEAST_EXPECT(read_size_helper(sb, 20) == 20);
@@ -416,22 +415,18 @@ public:
{ {
streambuf sb(1000); streambuf sb(1000);
BEAST_EXPECT(sb.alloc_size() == 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, 1) == 1);
BEAST_EXPECT(read_size_helper(sb, 1000) == 1000); BEAST_EXPECT(read_size_helper(sb, 1000) == 1000);
BEAST_EXPECT(read_size_helper(sb, 2000) == 1000); BEAST_EXPECT(read_size_helper(sb, 2000) == 1000);
sb.prepare(3); 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, 1) == 1);
BEAST_EXPECT(read_size_helper(sb, 1000) == 1000); BEAST_EXPECT(read_size_helper(sb, 1000) == 1000);
BEAST_EXPECT(read_size_helper(sb, 2000) == 1000); BEAST_EXPECT(read_size_helper(sb, 2000) == 1000);
sb.commit(3); 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, 1) == 1);
BEAST_EXPECT(read_size_helper(sb, 1000) == 997); BEAST_EXPECT(read_size_helper(sb, 1000) == 997);
BEAST_EXPECT(read_size_helper(sb, 2000) == 997); BEAST_EXPECT(read_size_helper(sb, 2000) == 997);
sb.consume(2); 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, 1) == 1);
BEAST_EXPECT(read_size_helper(sb, 1000) == 997); BEAST_EXPECT(read_size_helper(sb, 1000) == 997);
BEAST_EXPECT(read_size_helper(sb, 2000) == 997); BEAST_EXPECT(read_size_helper(sb, 2000) == 997);