Re: [boost] [review][beast] Five-days Remaining

Subject: Re: [boost] [review][beast] Five-days Remaining
From: Vinnie Falco (vinnie.falco_at_[hidden])
Date: 2017-07-06 23:49:02

On Thu, Jul 6, 2017 at 4:23 PM, Gavin Lambert via Boost
<boost_at_[hidden]> wrote:
> I said that being based on a *stream* abstraction is a good thing.  As that
> is lower-level than a *socket* abstraction, and could be used on other
> streams that aren't sockets

Thanks for clarifying, I think I misread. Beast HTTP and WebSockets
are both based on Asio *stream* abstractions, not socket abstractions.
Beast does not use socket abstractions anywhere, subject to the caveat
that stream must indicate the end of the connection/data by delivering
`boost::asio::error::eof` from a call to read_some or async_read_some.

> (eg. a non-socket-based networking library on
> some other platform, or for simulated conversations in unit tests).

Yes! This is exactly how the Beast tests work:
<https://github.com/vinniefalco/Beast/blob/78a065ba39836d91d7e70d93de7f9140f518083b/extras/beast/test/pipe_stream.hpp#L30>

I will add two examples to the documentation of SyncReadStream and
SyncWriteStream adapters which allow std::istream and std::ostream
derived classes respectively to operate with unmodified Beast HTTP
stream algorithms. I allude to them (bottom of page) because I have
not written them yet, but they will be written:
<http://vinniefalco.github.io/stage/beast/review/beast/using_http/buffer_oriented_serializing.html#beast.using_http.buffer_oriented_serializing.write_to_std_ostream>

> ...fundamentally this doesn't even
> need to be based on streams and it could be based on byte blob containers

This is true for synchronous algorithms, but those are trivial. Blob
containers by themselves prescribe no specifications for asynchronous
operation. That's where the real bread and butter is: it is Beast's
asynchronous algorithms which adhere to the Extensible Asynchronous
Model and Executors (asio::io_service is an early version of this
concept) that provide the lion's share of value.

> The HTTP code acts on streams.  The WebSocket code acts on sockets, not
> streams.  (Evidence: nowhere in the HTTP docs could I find a method that
> used anything other than an abstract stream as a parameter.  Whereas in the
> WebSocket docs the very first thing talks about handing over ownership of a
> socket to the WebSocket stream class, and this is the basis for everything
> else.)  This is clearly a higher level than the HTTP code.

Ah! I see the confusion now. Both HTTP and WebSocket in Beast operate
on stream algorithms. However, a WebSocket session has state whose
lifetime must be managed. HTTP/1 does not. Beast's interface for HTTP
operations uses simple free functions, while the WebSocket interface
uses a different approach. It offers a wrapper which wraps your
SyncStream or AsyncStream. Depending on how you declare the wrapper,
it is either a non-owning reference, or an owning wrapper. Examples:

// non-owning
boost::asio::ip::tcp::socket sock{ios};
beast::websocket::stream<boost::asio::ip::tcp::socket&> ws{sock};

// owning
boost::asio::ip::tcp::socket sock{ios};
beast::websocket::stream<boost::asio::ip::tcp::socket> ws{std::move(sock)};

That the examples show ip::tcp::socket is only because those are the
streams which come with Asio (plus ssl::stream). Beast WebSocket
wrappers work with any synchronous or asynchronous stream.

With beast::websocket::stream, the implementation does not know that
you are working with a socket. It doesn't know anything about IP
addresses, endpoint_type, making connections, accepting connections,
or any of that. Its the callers responsibility to provide an
already-connected stream object that meets the requirements, socket or
otherwise.

Although HTTP/1 is stateless, HTTP/2 is not. When Beast gains an
HTTP/2 implementation (a planned feature) it will use the same wrapper
style as websocket:

    namespace beast {
    namespace http {

    /** An HTTP/2 stream.

        @tparam Stream A type meeting the requirements of
                        AsyncReadStream  and     AsyncWriteStream
    */
    template<class Stream>
    class stream
    {
       BOOST_STATIC_ASSERT(
           is_async_read_stream<Stream>::value &&
           is_async_write_stream<Stream>::value);

        Stream stream_;

    public:
       template<class... Args>
       stream(Args&&... args)
           : stream_(std::forward<Args>(args)...)
       {
       }
       ...
    };

    } // http
    } // beast

Due to the requirements of HTTP/2 (over-engineered and overly complex
in my opinion) it is not possible to provide a synchronous
implementation. And unfortunately the implementation must depart from
Beast's philosophy of leaving some things up to the user, a full
HTTP/2 implementation must take over the "read pump" in order to
enforce stream prioritization.

> What I was suggesting is that the WebSocket parsing and formatting code be
> kept in Beast (eg. the code that generates the byte blob that represents a
> frame; as this would be stateless stream-based similar to the HTTP code) but
> the part that deals with sockets should be split to a separate library
> Beast.WebSocket (or whatever).

That makes the library objectively worse; users overwhelmingly want to
operate WebSockets with sockets and ssl streams. And
beast::websocket::stream already works with stream concepts (not
sockets).

Thanks