-
Notifications
You must be signed in to change notification settings - Fork 8
body_read_stream implementation - WORK IN PROGRESS #57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
body_read_stream implementation - WORK IN PROGRESS #57
Conversation
|
||
#include <boost/http_io/detail/config.hpp> | ||
#include <boost/http_proto/request_parser.hpp> | ||
#include <boost/http_proto/response_parser.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should be able to get away with just parser.hpp instead of these other two
#include <boost/http_proto/response_parser.hpp> | ||
#include <boost/asio/async_result.hpp> | ||
#include <boost/system/error_code.hpp> | ||
#include <boost/system/result.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think result.hpp is going to help?
|
||
template<class UnderlyingAsyncReadStream> | ||
body_read_stream<UnderlyingAsyncReadStream>::body_read_stream( | ||
const rts::context& rts_ctx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the context is needed
namespace http_io { | ||
|
||
template<class UnderlyingAsyncReadStream> | ||
class body_read_stream { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: add get_executor()
|
||
namespace detail { | ||
|
||
template <class MutableBufferSequence, class UnderlyingAsyncReadStream> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just Stream
would suffice instead of UnderlyingAsyncReadStreamToUseForTheNextLayer?
An automated preview of the documentation is available at https://57.http-io.prtest2.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2025-10-20 18:03:41 UTC |
An automated preview of the documentation is available at https://57.beast2.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2025-10-22 09:45:21 UTC |
void(system::error_code, std::size_t)) CompletionToken> | ||
BOOST_ASIO_INITFN_AUTO_RESULT_TYPE(CompletionToken, | ||
void(system::error_code, std::size_t)) | ||
async_read( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this class needs to satisfy Asio's AsyncReadStream type requirements, then the async_read
function is unnecessary.
class body_read_stream_op : public asio::coroutine { | ||
|
||
AsyncReadStream& us_; | ||
const MutableBufferSequence& mb_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the operation should make a copy of the provided buffer, otherwise, the call site must manage the buffer's lifetime, which is cumbersome and inconsistent with the Asio.
Like this could is UB currently:
brs.async_read_some(
buf.prepare(buf.capacity()), // <--- would be destroyed after call
[&](system::error_code ec, std::size_t n)
{
});
TEST_SUITE( | ||
body_read_stream_test, | ||
"boost.http_io.body_read_stream.hello_world"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MungoG, now that we have test::stream
available, we could simplify the unit test to something like this. I'm not sure how much of the existing source code it covers, but it should be a good starting point.
struct body_read_stream_test
{
core::string_view const msg =
"HTTP/1.1 200 OK\r\n"
"Content-Length: 3\r\n"
"\r\n"
"abc";
public:
void
testAsyncReadSome()
{
boost::asio::io_context ioc;
rts::context rts_ctx;
http_proto::install_parser_service(rts_ctx, {});
test::stream ts(ioc, msg);
http_proto::response_parser pr(rts_ctx);
pr.reset();
pr.start();
// limit async_read_some for better coverage
ts.read_size(1);
body_read_stream<test::stream> brs(ts, pr);
char storage[8];
buffers::flat_buffer buf(storage, sizeof(storage));
// read 1 octet of body
brs.async_read_some(
buf.prepare(buf.capacity()),
[&](system::error_code ec, std::size_t n)
{
BOOST_TEST(! ec.failed());
BOOST_TEST_EQ(n, 1);
buf.commit(n);
});
test::run(ioc);
BOOST_TEST(pr.got_header());
BOOST_TEST(! pr.is_complete());
BOOST_TEST_EQ(
buffers::size(pr.pull_body()), 0);
// read the remaining body
// asio::async_read repeatedly calls async_read_some
// until all supplied buffers are filled, or an error occurs.
asio::async_read(
brs,
buf.prepare(buf.capacity()),
[&](system::error_code ec, std::size_t n)
{
BOOST_TEST_EQ(ec, asio::error::eof);
BOOST_TEST_EQ(n, 2);
buf.commit(n);
});
test::run(ioc);
BOOST_TEST(pr.got_header());
BOOST_TEST(pr.is_complete());
// no body octets should remain
BOOST_TEST_EQ(
buffers::size(pr.pull_body()), 0);
BOOST_TEST(
core::string_view(
static_cast<const char*>(buf.data().data()),
buf.data().size()) == "abc");
}
void
run()
{
testAsyncReadSome();
}
};
Implementation for the body_read_stream part of #55