Skip to content

Commit

Permalink
fix: buffer preparation in serializer
Browse files Browse the repository at this point in the history
  • Loading branch information
@ashtum
ashtum committed Feb 28, 2024
1 parent e1b4aa1 commit af59fce
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 76 deletions.
89 changes: 44 additions & 45 deletions src/serializer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,59 +138,57 @@ prepare() ->

if(st_ == style::source)
{
if(! is_chunked_)
if(more_)
{
auto rv = src_->read(
tmp0_.prepare(
tmp0_.capacity() -
tmp0_.size()));
tmp0_.commit(rv.bytes);
if(rv.ec.failed())
return rv.ec;
more_ = ! rv.finished;
}
else
{
if((tmp0_.capacity() -
tmp0_.size()) >
chunked_overhead_)
if(! is_chunked_)
{
auto dest = tmp0_.prepare(18);
write_chunk_header(dest, 0);
tmp0_.commit(18);
auto rv = src_->read(
tmp0_.prepare(
tmp0_.capacity() -
2 - // CRLF
5 - // final chunk
tmp0_.size()));
tmp0_.prepare(tmp0_.capacity()));
tmp0_.commit(rv.bytes);
// VFALCO FIXME!
//if(rv.bytes == 0)
//tmp0_.uncommit(18); // undo
if(rv.ec.failed())
return rv.ec;
if(rv.bytes > 0)
{
// rewrite with correct size
write_chunk_header(
dest, rv.bytes);
// terminate chunk
tmp0_.commit(
buffers::buffer_copy(
tmp0_.prepare(2),
buffers::const_buffer(
"\r\n", 2)));
}
if(rv.finished)
more_ = ! rv.finished;
}
else
{
if(tmp0_.capacity() > chunked_overhead_)
{
tmp0_.commit(
buffers::buffer_copy(
tmp0_.prepare(5),
buffers::const_buffer(
"0\r\n\r\n", 5)));
auto dest = tmp0_.prepare(18);
write_chunk_header(dest, 0);
tmp0_.commit(18);
auto rv = src_->read(
tmp0_.prepare(
tmp0_.capacity() -
2 - // CRLF
5));// final chunk
tmp0_.commit(rv.bytes);
// VFALCO FIXME!
//if(rv.bytes == 0)
//tmp0_.uncommit(18); // undo
if(rv.ec.failed())
return rv.ec;

Check warning on line 169 in src/serializer.cpp

View check run for this annotation

Codecov / codecov/patch

src/serializer.cpp#L169 

Added line #L169 was not covered by tests
if(rv.bytes > 0)
{
// rewrite with correct size
write_chunk_header(
dest, rv.bytes);
// terminate chunk
tmp0_.commit(
buffers::buffer_copy(
tmp0_.prepare(2),
buffers::const_buffer(
"\r\n", 2)));
}
if(rv.finished)
{
tmp0_.commit(
buffers::buffer_copy(
tmp0_.prepare(5),
buffers::const_buffer(
"0\r\n\r\n", 5)));
}
more_ = ! rv.finished;
}
more_ = ! rv.finished;
}
}

Expand Down Expand Up @@ -474,6 +472,7 @@ start_source(

hp_ = &out_[0];
*hp_ = { m.ph_->cbuf, m.ph_->size };
more_ = true;
}

auto
Expand Down
74 changes: 43 additions & 31 deletions test/unit/serializer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ struct serializer_test
{
struct test_source : source
{
test_source()
: s_("12345")
test_source(core::string_view s)
: s_(s)
{
}

Expand Down Expand Up @@ -81,11 +81,18 @@ struct serializer_test
{
std::string s;
auto cbs = sr.prepare().value();
s.resize(buffers::buffer_size(cbs));
// We limit buffer consumption to necessitate
// multiple calls to serializer::prepare and
// serializer::consume, allowing tests to cover
// state management within these functions
auto consumed = (std::min)(
std::size_t{256},
buffers::buffer_size(cbs));
s.resize(consumed);
buffers::buffer_copy(
buffers::mutable_buffer(
&s[0], s.size()), cbs);
sr.consume(s.size());
&s[0], consumed), cbs);
sr.consume(consumed);
return s;
}

Expand Down Expand Up @@ -118,10 +125,10 @@ struct serializer_test
sr.start(res);
sr.start(res, buffers::const_buffer{});
sr.start(res, buffers::mutable_buffer{});
sr.start(res, test_source{});
sr.start(res, test_source{"12345"});
sr.start(res, make_const(buffers::const_buffer{}));
sr.start(res, make_const(buffers::mutable_buffer{}));
sr.start(res, make_const(test_source{}));
sr.start(res, make_const(test_source{"12345"}));

serializer(65536);
#ifdef BOOST_HTTP_PROTO_HAS_ZLIB
Expand Down Expand Up @@ -189,19 +196,21 @@ struct serializer_test
BOOST_TEST(s == expected);
};

template<class Source>
template<class Source, class F>
void
check_src(
core::string_view headers,
Source&& src,
core::string_view expected)
F const& f)
{
response res(headers);
serializer sr;
// we limit the buffer size of the serializer, requiring
// it to make multiple calls to source::read
serializer sr(1024);
sr.start(res, std::forward<
Source>(src));
std::string s = read(sr);
BOOST_TEST(s == expected);
f(s);
};

void
Expand Down Expand Up @@ -256,32 +265,35 @@ struct serializer_test
check_src(
"HTTP/1.1 200 OK\r\n"
"Server: test\r\n"
"Content-Length: 5\r\n"
"Content-Length: 2048\r\n"
"\r\n",
test_source{},
//--------------------------
"HTTP/1.1 200 OK\r\n"
"Server: test\r\n"
"Content-Length: 5\r\n"
"\r\n"
"12345");
test_source{std::string(2048, '*')},
[](core::string_view s){
BOOST_TEST(s ==
"HTTP/1.1 200 OK\r\n"
"Server: test\r\n"
"Content-Length: 2048\r\n"
"\r\n" +
std::string(2048, '*'));
});

// source chunked
check_src(
"HTTP/1.1 200 OK\r\n"
"Server: test\r\n"
"Transfer-Encoding: chunked\r\n"
"\r\n",
test_source{},
//--------------------------
"HTTP/1.1 200 OK\r\n"
"Server: test\r\n"
"Transfer-Encoding: chunked\r\n"
"\r\n"
"0000000000000005\r\n"
"12345"
"\r\n"
"0\r\n\r\n");
test_source{std::string(2048, '*')},
[](core::string_view s){
core::string_view expected_header =
"HTTP/1.1 200 OK\r\n"
"Server: test\r\n"
"Transfer-Encoding: chunked\r\n"
"\r\n";
BOOST_TEST(s.starts_with(expected_header));
s.remove_prefix(expected_header.size());
BOOST_TEST(std::count(s.begin(), s.end(), '*') == 2048);
});
}

void
Expand All @@ -295,7 +307,7 @@ struct serializer_test
"Expect: 100-continue\r\n"
"Content-Length: 5\r\n"
"\r\n");
sr.start(req, test_source{});
sr.start(req, test_source{"12345"});
std::string s;
system::result<
serializer::const_buffers_type> rv;
Expand Down Expand Up @@ -375,7 +387,7 @@ struct serializer_test
"\r\n";
serializer sr;
response res(sv);
sr.start(res, test_source{});
sr.start(res, test_source{"12345"});
auto s = read(sr);
BOOST_TEST(s ==
"HTTP/1.1 200 OK\r\n"
Expand Down

0 comments on commit af59fce

Please sign in to comment.