Skip to content

Commit 87a75e8

Browse files
benoitcbenoitc
authored
Merge pull request #912 from benoitc/fix/chunk-size-split-crlf
Wait for the LF when a chunk-size line splits on the CR
2 parents 1c34f0b + 6a5db75 commit 87a75e8

3 files changed

Lines changed: 92 additions & 5 deletions

File tree

‎NEWS.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,17 @@
11
# NEWS
22

3+
unreleased
4+
----------
5+
6+
### Fixed
7+
8+
- Chunked decoding no longer fails with `{error, invalid_chunk_size}` when
9+
the CRLF terminating a chunk-size line is split across two socket reads
10+
(buffer ending on a lone `\r`). The parser now waits for the `\n` (#901).
11+
- A malformed chunk-size line or chunk terminator now fails cleanly with
12+
`{error, invalid_chunk_size}` or `{error, poorly_formatted_chunked_size}`
13+
instead of crashing the parser with a `case_clause` error.
14+
315
4.7.0 - 2026-07-17
416
------------------
517

‎src/hackney_http.erl‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -493,9 +493,12 @@ content_decode(ContentDecode, Data, St) ->
493493

494494
%% @doc Decode a stream of chunks.
495495
-spec te_chunked(binary(), any())
496-
-> more | {ok, binary(), {non_neg_integer(), non_neg_integer()}}
497-
| {ok, binary(), binary(), {non_neg_integer(), non_neg_integer()}}
498-
| {done, non_neg_integer(), binary()} | {error, badarg}.
496+
-> done
497+
| {chunk_done, binary()}
498+
| {chunk_ok, binary(), binary()}
499+
| more
500+
| {more, {non_neg_integer(), non_neg_integer()}}
501+
| {error, invalid_chunk_size | poorly_formatted_chunked_size}.
499502
te_chunked(<<>>, _) ->
500503
done;
501504
te_chunked(Data, _) ->
@@ -507,10 +510,14 @@ te_chunked(Data, _) ->
507510
{ok, Chunk, Rest1} ->
508511
{chunk_ok, Chunk, Rest1};
509512
eof ->
510-
{more, {byte_size(Rest), Size}}
513+
{more, {byte_size(Rest), Size}};
514+
{error, _} = Error ->
515+
Error
511516
end;
512517
eof ->
513-
more
518+
more;
519+
{error, _} = Error ->
520+
Error
514521
end.
515522

516523
%% @doc Decode an identity stream.
@@ -535,6 +542,11 @@ read_size(Data) ->
535542

536543
read_size(<<>>, _, _) ->
537544
eof;
545+
%% The size line's CRLF terminator is split across two reads and the buffer
546+
%% ends on the lone \r: wait for the \n instead of treating the line as
547+
%% malformed (issue #901).
548+
read_size(<<"\r">>, _, _) ->
549+
eof;
538550

539551
read_size(<<"\r\n", Rest/binary>>, Size, Len) when Len > 0 ->
540552
{ok, Size, Rest};

‎test/hackney_http_tests.erl‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,69 @@ parse_chunked_response_trailers_test() ->
6767
{more, P4} = hackney_http:execute(P3, <<"0\r\nFoo: ">>),
6868
?assertEqual({done, <<>>}, hackney_http:execute(P4, <<"Bar\r\n\r\n">>)).
6969

70+
%% Issue #901: the CRLF terminating a chunk-size line is split across two
71+
%% reads, leaving the buffer on a lone \r. The parser must wait for the \n
72+
%% instead of failing with invalid_chunk_size.
73+
parse_chunked_size_crlf_split_test() ->
74+
?assertEqual({done, <<"HELLO">>},
75+
run_chunked([<<"5\r">>, <<"\nHELLO\r\n0\r\n\r\n">>])).
76+
77+
parse_chunked_last_chunk_crlf_split_test() ->
78+
?assertEqual({done, <<"HELLO">>},
79+
run_chunked([<<"5\r\nHELLO\r\n0\r">>, <<"\n\r\n">>])).
80+
81+
%% Exhaustive read-boundary sweep: a two-chunk body with a trailer, split at
82+
%% every possible byte position, must always decode to the same body.
83+
parse_chunked_split_sweep_test() ->
84+
Body = <<"5\r\nHELLO\r\n6\r\n WORLD\r\n0\r\nX-Trail: 1\r\n\r\n">>,
85+
[?assertEqual({done, <<"HELLO WORLD">>},
86+
run_chunked([binary:part(Body, 0, I),
87+
binary:part(Body, I, byte_size(Body) - I)]))
88+
|| I <- lists:seq(0, byte_size(Body))].
89+
90+
%% A genuinely malformed size line must fail cleanly, not crash te_chunked.
91+
parse_chunked_invalid_size_test() ->
92+
?assertEqual({error, invalid_chunk_size},
93+
run_chunked([<<"5\rX\nHELLO\r\n0\r\n\r\n">>])).
94+
95+
%% A malformed chunk terminator must fail cleanly as well
96+
%% (read_chunk's error was unhandled in te_chunked).
97+
parse_chunked_bad_chunk_terminator_test() ->
98+
?assertEqual({error, poorly_formatted_chunked_size},
99+
run_chunked([<<"5\r\nHELLOXX0\r\n\r\n">>])).
100+
101+
%% Drive a chunked response body through the parser, feeding Segments as
102+
%% separate socket reads. Contract: after {ok, Chunk, P}, drain buffered data
103+
%% with execute/1 until the parser asks for more, only then feed the next
104+
%% segment. Returns {done, Body} when the response completed with no
105+
%% leftover bytes.
106+
run_chunked(Segments) ->
107+
P0 = hackney_http:parser([response]),
108+
{_, _, _, _, P1} = hackney_http:execute(P0, <<"HTTP/1.1 200 OK\r\n">>),
109+
{_, _, P2} = hackney_http:execute(P1, <<"Transfer-Encoding: chunked\r\n">>),
110+
{headers_complete, P3} = hackney_http:execute(P2, <<"\r\n">>),
111+
feed_chunked(P3, Segments, <<>>).
112+
113+
feed_chunked(_Parser, [], Acc) ->
114+
{incomplete, Acc};
115+
feed_chunked(Parser, [Seg | Rest], Acc) ->
116+
chunked_step(hackney_http:execute(Parser, Seg), Rest, Acc).
117+
118+
chunked_step({ok, Chunk, Parser}, Segments, Acc) ->
119+
chunked_step(hackney_http:execute(Parser), Segments, <<Acc/binary, Chunk/binary>>);
120+
chunked_step({more, Parser}, Segments, Acc) ->
121+
feed_chunked(Parser, Segments, Acc);
122+
chunked_step({more, Parser, _Buffer}, Segments, Acc) ->
123+
feed_chunked(Parser, Segments, Acc);
124+
chunked_step({done, <<>>}, _Segments, Acc) ->
125+
{done, Acc};
126+
chunked_step({done, Leftover}, _Segments, Acc) ->
127+
{done_with_leftover, Acc, Leftover};
128+
chunked_step(done, _Segments, Acc) ->
129+
{done, Acc};
130+
chunked_step({error, Reason}, _Segments, _Acc) ->
131+
{error, Reason}.
132+
70133
%% Issue #697: Handle non-standard decimal status codes (e.g., 401.1 from IIS)
71134
parse_response_decimal_status_code_test() ->
72135
Response = <<"HTTP/1.1 401.1 Access Denied">>,

0 commit comments

Comments
 (0)