Support processing HTTP/1.1 headers as they arrive

[gdwoolbert3]

Introduction

This is my attempt at implementing the feature requested in Issue 452.

The motivation for the change was the X-ClickHouse-Progress header system used by the ClickHouse HTTP interface (which I strongly suspect was the reason behind the aforementioned issue).

Implementation

This PR adds a new :stream_headers option for HTTP/1.1 connections that allows headers and trailers to be emitted as responses as soon as they're available instead of buffering them until they've all arrived. A few nuances about enabling this feature:

• All complete headers from a single chunk are emitted at the same time
• It's theoretically possible to receive NO :headers responses for a given request whereas, when not streaming headers, an empty list would be emitted. This change is explicitly laid out in the Mint.HTTP documentation.

This feature is opt-in, meaning there should be no changes for existing users.

Tesing

All existing tests pass and new stream headers tests were added to the HTTP/1.1 connection test module.

Open to any and all feedback 😃

[ericmj]

The option and response shape make sense, but there’s one parsing case to fix before merging. If a socket message ends exactly after a complete header line, such as X-ClickHouse-Progress: ...\r\n, HTTP1.stream/2 buffers the header and returns no response until another byte arrives. This leaves progress updates one socket message behind, and a lone update won’t be emitted until the next header or the end of the section.

Could you add tests where a regular header and a trailer end at that boundary, with no byte from the following line, and update the parser to emit them immediately?

[gdwoolbert3]

@ericmj nice catch! If I'm understanding correctly, this appears to be a quirk of the underlying :erlang.decode_packet/3 function:

iex> :erlang.decode_packet(:httph_bin, "X-ClickHouse-Progress: foo\r\n", [])
{:more, :undefined}

iex> :erlang.decode_packet(:httph_bin, "X-ClickHouse-Progress: foo\r\nX", [])
{:ok, {:http_header, 0, "X-Clickhouse-Progress", "X-ClickHouse-Progress", "foo"}, "X"}

With that being said, the ideal fix isn't particularly clear to me. If I'm reading RFC 7230 right, I think we could naively check that the incoming data ends with \r\n and then do something to force the header to parse (ie appending a dummy byte), but that doesn't feel like the cleanest solution.

Do you have any suggestions? If not, I'll go with the aforementioned approach.

On a related note, do you have insight into whether or not the above behavior is intentional?

[wojtekmach]

one reason might be HTTP1 line folding, a header could have spanned multiple lines, https://www.rfc-editor.org/info/rfc9112/#section-5.2,

iex> :erlang.decode_packet(:httph_bin, "header: chunk1\r\n chunk2\r\n\r\n", [])
{:ok, {:http_header, 0, "Header", "header", "chunk1\r\n chunk2"}, "\r\n"}

and so when getting "header: chunk1\r\n", returning :more makes sense. That particular behaviour is obsolete in the spec however. And so yeah, it would be nice if we've gotten {:ok, {:http_header, ...}, ""} instead, but that'd probably require adding an option to packet parser and that probably has very high barrier to entry.

[ericmj]

Let's retry the :erlang.decode_packet call with data <> sentinel when it returns :more, stream_headers is enabled, and data ends in LF. Where sentinel is a single byte that doesn't trigger the legacy line folding.

[gdwoolbert3]

Pushed the fix recommended by @ericmj.

As it turns out, :erlang.decode_packet/3 allows LF in addition to CRLF as a header sep:

iex> :erlang.decode_packet(:httph_bin, "X-ClickHouse-Progress: foo\r\n", [])
{:more, :undefined}
iex> :erlang.decode_packet(:httph_bin, <<"X-ClickHouse-Progress: foo\r\n", 0>>, [])
{:ok, {:http_header, 0, "X-Clickhouse-Progress", "X-ClickHouse-Progress", "foo"}, <<0>>}
iex> :erlang.decode_packet(:httph_bin, <<"X-ClickHouse-Progress: foo\n", 0>>, [])
{:ok, {:http_header, 0, "X-Clickhouse-Progress", "X-ClickHouse-Progress", "foo"}, <<0>>}

I kept this behavior in order to maintain parity with the underlying functionality, but I'm happy to update at your discretion.