Beast: chunked parsing - interface design

[qwertzui11]

version

master / v50

code

beast::http::parser<false, beast::http::dynamic_body, beast::http::fields>
          parser;
beast::http::read_some(socket, stream_buffer, parser);
const auto body_size =
  boost::asio::buffer_size(parser.get().body.data());
if (body_size == 0)
  ; // we parsed a chunk header
else
  ; // we got a (maybe incomplete) chunk body
parser.get().body.consume(body_size);

problem

I'm trying to read a http request chunk by chunk. Previous versions of beast had the method state() which returned the state of the parser. In previous versions i checked parser.state() == beast::parser::state::chunk_header, then I knew the parser was waiting for a header and a !complete! body chunk is available. I suggest to make the state() method public again (and handle the state chunk_header0 somehow) or do something is_chunk_available. Have I missed something, is there a better for chunked reading? What do you think?
Another suggestion I got is, that read_some only shall return on a state change, or an error?
I like it, that read_some doesn't return the bytes to consume no more. :+1:

[vinniefalco]

I think what you really want is to use buffer_body instead, then you can just set up a static buffer (it could even be a stack array). To use it, just set body.data to point to some memory, and set body.size to the size of that memory. When you call read_some (or you could even call read it would work better by trying to fill the whole buffer) it will write to the memory and adjust the data and size values in the body to "consume" them. When the buffer is full you'll get error::need_buffer, and you can just reset the buffer and then call read again.

The documentation has an example:
http://vinniefalco.github.io/beast/beast/http_examples/http_relay.html

See:
http://vinniefalco.github.io/beast/beast/ref/http__buffer_body.html

Hopefully you won't need the state. Applications shouldn't care about knowing where the chunk boundaries are, any node along the path between the client and server can reframe the message payload, including removing the chunked encoding entirely (and putting in a Content-Length).

[vinniefalco]

@qwertzui11 Please consider participating in the Boost formal review for Beast, which starts on July 1st. All you need to do is sign up to the Boost developers mailing list (https://lists.boost.org/mailman/listinfo.cgi/boost) ahead of time and listen for the announcement from the review manager. Your review is vital to the health and success of the library - whether you have positive things to say, or negative things to say, feedback is the only way for Beast to get better!

[qwertzui11]

I will definitely participate, spoiler it will be very positive one :wink:
Your suggested solution seems to solve #154 . But my scenario is a bit different. I've got to work with a legacy system, where chunked http gets abused as "websocket". Every chunk is a message and I've got to read from the stream "message by message" or "chunk by chunk". I've got know where a chunk starts, ends and its size.

Hopefully you won't need the state. Applications shouldn't care about knowing where the chunk boundaries are, any node along the path between the client and server can reframe the message payload, including removing the chunked encoding entirely (and putting in a Content-Length).

I'm afraid that's the case

[qwertzui11]

Hopefully you won't need the state. Applications shouldn't care about knowing where the chunk boundaries are, any node along the path between the client and server can reframe the message payload, including removing the chunked encoding entirely (and putting in a Content-Length).

I don't think beast should handle such dirty, non-standard corner-cases.
Thx for your time, I'll solve my issue in another way.

[vinniefalco]

You can subclass basic_parser (its easy) and then you get a callback at the beginning of a chunk, it gives you the extension if any and the size in advance. See on_body
http://vinniefalco.github.io/beast/beast/http/custom_parsers.html

Check out how easy it is to subclass basic_parser
https://github.com/vinniefalco/Beast/blob/master/include/beast/http/parser.hpp#L41

[vinniefalco]

I don't think beast should handle such dirty, non-standard corner-cases.

I disagree, Beast is supposed to be foundational / low-level. We will figure this out!

[vinniefalco]

Following up, what do you think about these solutions?

[qwertzui11]

You can subclass basic_parser (its easy) and then you get a callback at the beginning of a chunk, it gives you the extension if any and the size in advance. See on_body

I really like this solution! However on_chunk never gets called. All over the parser.ipp code if(*p == ';') gets checked before any on_chunk call. I'm no expert, but I guess this has something to do with "chunk decoration"? so on_chunk never gets called without decoration, although documentation suggests otherwise. /// Called for each chunk header.
In my tests on_data gets called with chunk data or sometimes parts of a chunk.
Have I got something wrong? Shall I upload my sample code?

https://github.com/vinniefalco/Beast/blob/fde69298dce9d29c13eac272d34216e972bfb2fc/include/beast/http/impl/basic_parser.ipp#L560
https://github.com/vinniefalco/Beast/blob/fde69298dce9d29c13eac272d34216e972bfb2fc/include/beast/http/impl/basic_parser.ipp#L605

version is master (v55)
Thx for your time!

[vinniefalco]

How lame, another bug. A call to on_chunk for the case when the extension is empty is missing.

[vinniefalco]

Chunk decoration is for outgoing not incoming messages. Its how you set the extensions (and trailers).

[qwertzui11]

Chunk decoration is for outgoing not incoming messages. Its how you set the extensions (and trailers).

ah, check! :+1:

[vinniefalco]

At least the fix was easy

[qwertzui11]

I pulled an ran my tests successfully. I like the custom parser solution. I'm now able to read the stream chunk by chunk, and beasts handles all the http overhead.
For other people: I'm getting the the "chunk header" information by on_chunk and the "chunk body" via on_body
Thx again.

[vinniefalco]

GitHub will close this issue automatically when the commit is merged (it says "fix #435")