hackney_http:read_size/3 returns {error, invalid_chunk_size} when a chunk-size line's CRLF is split across reads (lone trailing CR)

[lucianoengel]

Environment

• hackney 4.4.5 (also present on master)
• Erlang/OTP 29, but the faulty logic is version-independent
• Reproduced via direct hackney_http use; observed in production via HTTPoison against servers returning large chunked responses over TLS

Summary

Decoding a chunked response aborts with {error, invalid_chunk_size} when the CRLF terminator of a chunk-size line is split across two socket reads such that the buffer ends on a lone \r (the \n has not arrived yet). Instead of waiting for the next byte, hackney treats it as a malformed size and fails the whole response.

This is intermittent and gets more likely with large chunked bodies (more chunk headers → more read boundaries) and over TLS, where records fragment the byte stream at arbitrary offsets.

Root cause

In src/hackney_http.erl, read_size/3 parses the chunk-size line byte-by-byte. The only "incomplete, need more data" case it handles is an empty buffer:

read_size(<<>>, _, _) ->
  eof;

Every other partial state falls through to the catch-all:

read_size(_, _, _) ->
  {error, invalid_chunk_size}.

When a read boundary lands between the \r and \n of a size line's terminator, the buffer ends as <<"...hexdigits...", "\r">>. That lone \r:

• is not <<"\r\n", _>> (needs 2 bytes),
• is not \n, ;, space, or a hex digit,
• is not <<>>,

so it hits the catch-all and returns {error, invalid_chunk_size} — aborting the response rather than requesting more data. (read_chunk/2 already handles its analogous 1-trailing-byte case by returning eof/more; only read_size/3 is missing it.)

Minimal reproduction

Driving the parser directly, feeding a chunked body where the size line's CRLF is split on the lone \r:

%% helper that feeds segments one execute/2 call at a time
Head = <<"HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n">>,

%% whole body in one buffer -> ok
run([<<Head/binary, "5\r\nHELLO\r\n0\r\n\r\n">>]).          %% => {done, <<"HELLO">>}

%% size-line CRLF split on the lone CR: "5\r" | "\nHELLO..."  -> BUG
run([<<Head/binary, "5\r">>, <<"\nHELLO\r\n0\r\n\r\n">>]).   %% => {error, invalid_chunk_size}

The second case should also yield the body; today it errors.

Proposed fix

Treat a trailing lone \r as "need more data" (eof), mirroring the empty-buffer clause — add one clause right after it:

read_size(<<>>, _, _) ->
  eof;
read_size(<<"\r">>, _, _) ->   %% CRLF terminator split across reads; wait for the \n
  eof;

With this clause, the split terminator waits for the next read and parses correctly. I've verified this locally (the split-on-CR case then returns the body).

Happy to send a PR if useful.

(Opening a new issue per the note on #361 — this is a distinct case in the current gen_statem-based code, and a different error than the old poorly_formatted_size.)