Re: [boost] Boost.HTTPKit, a new library from the makers of Beast!

2017-10-12 9:45 GMT-03:00 Vinnie Falco via Boost :

On Wed, Oct 11, 2017 at 9:47 PM, Vinícius dos Santos Oliveira wrote:

...

I took the liberty to convert the Tufão project that you've mentioned to use the Boost.Beast parser: https://github.com/vinipsmaker/tufao/commit/ 56e27d3b77d617ad1b4aea377f993592bc2c0d77

Would you say you like the new result better?

It seems pretty reasonable to me.

...

Would you say that I've misused your parser to favour my approach? How would you have done it in this case?

Misused? I don't think so. The only meaningful change I would make is that I would have simply called basic_parser::is_keep_alive() instead of re-implementing the logic for interpreting the Connection header.

...

Would you go beyond and accept the idea that the spaghetti effect is inherent to the callback-based approach of push parsers?

This is where we are venturing into the world of opinion. It seems like you have a general aversion to callbacks. But there is a reason Beast's parser is written this way. Recognize that there are two primary consumers of the parser:

1. Stream algorithms such as beast::http::read_some

Such as async_read_some, which are useless if you want to use other networking APIs (which is _the_ reason for the desire of separate parser API to start with). 2. Consumers of structured HTTP elements (e.g. fields)

The Beast design separates these concerns. Public member functions of `basic_parser` provide the interface needed for stream algorithms, while calls to the derived class provide the structured HTTP elements. I don't think it is a good idea to combine these into one interface

Point 1 is _useless_ if you are providing a _parser_ API. You don't know which networking API the user will use. It's that simple. There is no point 1. which you have done in your parser. The reason is that this

unnecessary coupling pointlessly complicates the writing of the stream algorithm.

Not coupling. Point 1 is not even there. Why would you design a parser with a specific networking API in mind? Message consuming/producing is the work of a higher layer. Anyone who wants to write an algorithm to feed the parser

from some source of incoming bytes now has to care about tokens. This is evident from your documentation:

http://boostgsoc14.github.io/boost.http/#parsing_tutorial1

In your example you declare a class `my_socket_consumer`. It has a single function `on_socket_callback` which is called repeatedly with incoming data. Not shown in your example is the stream algorithm (the function which interacts with the actual socket to retrieve the data). However, we know that this stream algorithm must be aware of the concrete type `my_socket_consumer` and that it needs to call `on_socket_callback` with an `asio::buffer`. A signature for this stream algorithm might look like this:

template<class SyncReadStream> void read(SyncReadStream& stream, my_socket_consumer& consumer);

The stream algorithms you provide were of no use to convert Tufão to use your parser. And I don't recall writing stream algorithms to use the parser. Observe that this stream algorithm can only ever work with that

specific consumer type. In your example, `my_socket_consumer` handles HTTP requests. Therefore, this stream algorithm can now only handle HTTP requests.

Same function to handle request and responses: https://github.com/BoostGSoC14/boost.http/blob/5ea65c64467e689cc9b67b8e66da6... There is a little of TMP in the lines above, but so do you have to use templates if you want to generalize your algorithm over the `isRequest` template parameter. In order to receive a response, a new stream algorithm

must be written. Compare this to the equivalent signature of a Beast styled stream algorithm:

template void read(SyncReadStream& stream, basic_parser& parser);

This allows an author to create a stream algorithm which works not just for requests which store their data as data members in a class (`my_socket_consumer`) but for any parser, thanks to the CRTP design.

What can your stream algorithms do besides feed data to the parser + consumer? Just a few messages ago I've showed concrete examples of how powerful the other model is. In the other model, you could even inject new tokens (in a virtual way, without the need to read the stream twice and inject new HTTP data at the appropriate points). For example, if I create a parser by subclassing

`beast::http::basic_parser` with an implementation that discards headers I don't care about, then it will work with the stream algorithm described above without requiring modification to that algorithm.

And if you change your networking API, the stream algorithm becomes pretty useless. This element that I've coded, in the other side, will work okay in any "stream model" that you wish: https://github.com/BoostGSoC14/boost.http/blob/9908fe06d4b2364ce18ea9b416264... It is interesting to note that your `my_socket_consumer` is

roughly equivalent to the beast::http::parser class (which is derived from beast::http::basic_parser):

<https://github.com/boostorg/beast/blob/f09b2d3e1c9d383e5d0f57b1bf8895 68cf27c39f/include/boost/beast/http/parser.hpp#L45>

Both of these classes store incoming structured HTTP elements in a container for holding HTTP message data. However note that unlike `beast::http::parser`, `my_socket_consumer` also has to know about buffers:

void on_socket_callback(asio::buffer data) { .... buffer.push_back(data); request_reader.set_buffer(buffer);

It might not be evident to casual readers but the implementation of `my_socket_consumer` has to know that the parser needs the serialized version of the message to be entirely contained in a single contiguous buffer.

Nope. Go read again. In my opinion this is a design flaw because it does not

enforce a separation of concerns. The handling of structured HTTP elements should not concern itself with the need to assemble the incoming message into a single contiguous buffer; that responsibility lies with the stream algorithm.

The design decision in Beast is to keep the interfaces used by stream algorithms separate from the interface used by consumers of HTTP tokens. Furthermore the design creates a standard interface so that stream algorithms can work with any instance of `basic_parser`, including both requests and responses, and for any user-defined derived class.

http://www.boost.org/doc/libs/develop/doc/html/boost_asio/reference/AsyncRea... Stream concepts that borrow ASIO concepts... that's so lame. You think the users want access to a parser to implement an ASIO API? They will just use Boost.Beast instead using a parser directly. -- Vinícius dos Santos Oliveira https://vinipsmaker.github.io/