Re: [boost] [beast] Supporting ICY 200 OK

Subject: Re: [boost] [beast] Supporting ICY 200 OK
From: Vinnie Falco (vinnie.falco_at_[hidden])
Date: 2017-10-04 13:37:26

On Tue, Oct 3, 2017 at 6:06 PM, Vinícius dos Santos Oliveira
<vini.ipsmaker_at_[hidden]> wrote:
> You could also try to standardize HTTP/1.1 and HTTP/2.0 under the same
> interface.

That is incorrect. All I'm talking about is a simple wrapper which
meets the requirements of SyncReadStream and AsyncReadStream which
checks the first incoming 3 bytes of data on the stream against the
string "ICY" and translates it into "HTTP/1.1" to allow the existing
beast parser to recognize a *slightly* non-standard response format.
Note the emphasis on the term *slightly*. I have designed a reasonable
solution to enable this niche use-case to work without incurring
significant technical debt for something that is clearly outside the
scope of rfc7230.

And you are suggesting that the same technique should be used to make
HTTP/2 appear like HTTP/1.1? Or am I not understanding your statement?

> The version attribute in the message model[1] might not make sense to all
> HTTP backends.

I'm not sure what an "HTTP backend" means, but for practical purposes
there are three versions of HTTP:

    HTTP/1.0
    HTTP/1.1
    HTTP/2

While the content of the messages is the same (start line, zero or
more fields, optional body) the semantics of the status-line and the
fields differ between versions. For example, the absence of the
Connection field in HTTP/1.1 implies keep-alive, while the absence of
the Connection field in HTTP/1.0 implies closure. While in HTTP/2 the
Connection field is illegal and may not appear at all.

The beast message container was designed from the ground up to work
with the three versions of HTTP that we have today. In order to
capture all of the information available in a message, the container
must reflect the HTTP version that is implicitly or explicitly stated
in the message. In order to empower authors to write libraries that
work with any message (a goal of Beast), the version field must be
present.

> This discussion reminds me of the difference between inductive reasoning and
> deductive reasoning. Only the immediate problem is pursued and no attention
> is given trying to solve the general problem at hand.

That is incorrect. The beast message container is designed to be HTTP/2-ready.

> Also, if beast::http::read is what I'll call, then this implies I need to
> throw my abstractions in `beast::http` namespace

That is incorrect. Beast HTTP stream operations use abstractions for
the following objects:

    stateful stream data uses the DynamicBuffer concept
     <http://www.boost.org/doc/libs/master/libs/beast/doc/html/beast/concepts/DynamicBuffer.html>

    message payload uses the Body concept
    <http://www.boost.org/doc/libs/master/libs/beast/doc/html/beast/concepts/Body.html>

    message headers use the Fields concept
    <http://www.boost.org/doc/libs/master/libs/beast/doc/html/beast/concepts/Fields.html>

    the stream uses the SyncReadStream, SyncWriteStream,
    AsyncReadStream, and AsyncWriteStream concepts from Asio:
    <http://www.boost.org/doc/libs/1_65_1/doc/html/boost_asio/reference/SyncReadStream.html>
    <http://www.boost.org/doc/libs/1_65_1/doc/html/boost_asio/reference/SyncWriteStream.html>
    <http://www.boost.org/doc/libs/1_65_1/doc/html/boost_asio/reference/AsyncReadStream.html>
    <http://www.boost.org/doc/libs/1_65_1/doc/html/boost_asio/reference/AsyncWriteStream.html>

Beast provides several general implementations for most of these
concepts, in the beast namespace, but objects may be passed from types
in any namespace. For example, beast stream operations work with
streams of type `boost::asio::ip::tcp::socket`, which is clearly not
in the `beast::http` namespace. This obvious counterexample disproves
your claim.

> However, a problem is the fact that this translation in the stream needs to
> happen to actually use the Beast message framework.

That is incorrect. The translation for converting the three character
sequence "ICY" at the beginning of a read stream, into "HTTP/1.1" is
not necessary to use the beast message container. Rather, it is
necessary in order to use the HTTP/1 parsers which come with beast.
The parser implementation strictly adheres to rfc7230. As the text
"ICY 200 OK\r\n" is not a valid status-line according to rfc7230, I'm
sure that you can understand why a strict parser would reject it.

Since the vast majority of consumers of HTTP libraries care about
reading actual valid HTTP and not SHOUTcast or Icecast MP3 streaming
server-specific modifications of the HTTP protocol, my solution for
developing a stream adapter to support the non-standard status-line is
elegant and pragmatic. In particular, it does not require modifying
beast library code which could introduce new security vulnerabilities.

> Why a “virtual HTTP stream” needs to exist beyond the virtual field?

My apologies but I am unfamiliar with these new terms "virtual HTTP
stream" and "virtual field" that you have introduced. Could you
perhaps define them so that I can discuss them on equal footing?

> Why does it need to go to memory?

My apologies but I do not understand this question. What is the "it"
that you think "needs" to "go to memory?"

> Why isn't just limited to the API?

My apologies but I do not understand this question at all. Perhaps you
could provide a small code example which illustrates your question or
counter-example?

> Boost.Http managed to expose this amount of feature without exposing a
> parser at all.

Does Boost.Http expose a feature which allows it to interpret "ICY 200
OK\r\n" as a valid HTTP status-line according to rfc7230?

Thanks