[boost] [beast] Automatic content-length?

[Peter Dimov via Boost]

In this code, extracted from one of the examples in the documentation,

// Set up the response, starting with the common fields
response<string_body> res;
res.version = 11;
res.set(field::server, "test");

/*...*/

// We return responses indicating an error if
// we do not recognize the request method.
res.result(status::bad_request);
res.set(field::content_type, "text/plain");
res.body = "Invalid request-method '" + req.method_string().to_string()
+ "'";

// Send the response
write(stream, res, ec);


field::content_length isn't set. Will Beast insert Content-Length based on
the body size, or will it send the response with the field missing?

The reason I ask is that I foresee forgetting the content_length to be a
common mistake, and an asymptomatic one.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Vinnie Falco via Boost]

On Sun, Jul 2, 2017 at 6:21 AM, Peter Dimov via Boost
<bo...@lists.boost.org> wrote:
> In this code, extracted from one of the examples in the documentation,

> ...

> res.body = "Invalid request-method '" + req.method_string().to_string() +
> "'";

> ...
> write(stream, res, ec);
> ...

> field::content_length isn't set.

Looks like you found a bug! One that I wrote, no less..

> Will Beast insert Content-Length based on the body size,
> or will it send the response with the field missing?

Beast will only set the content length if you ask it to, by calling
message::prepare_payload. The library does not perform any validation
on caller-provided HTTP inputs. For example if you called
msg.set("x:y", "z") that would result in a malformed message (colons
cannot appear in field names). Or if you set the content length
manually and lied about the correct value.

> The reason I ask is that I foresee forgetting the content_length to be a
> common mistake, and an asymptomatic one.

Well it wouldn't be asymptomatic in a real server since the other end
would hang, waiting for the "end of stream" to denote the end of the
HTTP message.

Thanks

[Peter Dimov via Boost]

Vinnie Falco wrote:
> > The reason I ask is that I foresee forgetting the content_length to be a
> > common mistake, and an asymptomatic one.
>
> Well it wouldn't be asymptomatic in a real server since the other end
> would hang, waiting for the "end of stream" to denote the end of the HTTP
> message.

A real server would use persistent connections, but there any many unreal
ones that just close the connection at the end of the response.

> Beast will only set the content length if you ask it to, by calling
> message::prepare_payload. The library does not perform any validation on
> caller-provided HTTP inputs.

This is completely sound from a theoretical design perspective, but
forgetting to call prepare_payload will still be very common.

Are there legitimate use cases in which one would send a response lacking
Content-Length, with a body that has a known size?

[Vinnie Falco via Boost]

On Sun, Jul 2, 2017 at 6:40 AM, Peter Dimov via Boost
<bo...@lists.boost.org> wrote:
> This is completely sound from a theoretical design perspective, but
> forgetting to call prepare_payload will still be very common.
>
> Are there legitimate use cases in which one would send a response lacking
> Content-Length, with a body that has a known size?

I can't think of one. The only case where a message with a payload
omits Content- Length is for HTTP/1.0 responses for which the payload
size is unknown. In this case the end of the message is indicated by
the end of file / end of stream.

[Peter Dimov via Boost]

> A real server would use persistent connections, but there any many unreal
> ones that just close the connection at the end of the response.

Exhibit A:
https://github.com/vinniefalco/Beast/blob/master/example/http-server-fast/http_server_fast.cpp#L155

[Vinnie Falco via Boost]

On Sun, Jul 2, 2017 at 7:31 AM, Peter Dimov via Boost
<bo...@lists.boost.org> wrote:
> Exhibit A:
> https://github.com/vinniefalco/Beast/blob/master/example/http-server-fast/http_server_fast.cpp#L155

Okay, is there something actionable here?

[Peter Dimov via Boost]

Vinnie Falco wrote:
> On Sun, Jul 2, 2017 at 7:31 AM, Peter Dimov via Boost
> <bo...@lists.boost.org> wrote:
> > Exhibit A:
> > https://github.com/vinniefalco/Beast/blob/master/example/http-server-fast/http_server_fast.cpp#L155
>
> Okay, is there something actionable here?

If "here" is this specific example, then no, it sets the Content-Length
(twice).

If "here" is Beast, then perhaps there should be a way to either fix or
detect the mistake of forgetting to set the Content-Length.

[Vinnie Falco via Boost]

On Sun, Jul 2, 2017 at 7:51 AM, Peter Dimov via Boost
<bo...@lists.boost.org> wrote:
> If "here" is Beast, then perhaps there should be a way to either fix or
> detect the mistake of forgetting to set the Content-Length.

I don't see how that's possible. We can't fix it, as the message is
const& in the serializer (and making it non const means that sending
the same message cannot happen concurrently).

There's no way to detect the mistake in `serializer`, as the Fields
concept provides no mechanism for inspecting the value of fields:
http://vinniefalco.github.io/stage/beast/review/beast/concept/Fields.html

We could add a way to extract Content-Length to the requirements of
FieldsReader:
http://vinniefalco.github.io/stage/beast/review/beast/concept/FieldsReader.html

This increases the burden of modeling FieldsReader. I'm reluctant to
it because the only use would be to call BOOST_ASSERT for this case.
Although seeing that both me and Chris made the same mistake, I might
be convinced otherwise.