From f0cc2d01e64f489275821b56c7e4343e4aa5bf97 Mon Sep 17 00:00:00 2001
From: James Fish <james@fishcakez.com>
Date: Tue, 11 Jun 2013 21:47:26 +0100
Subject: Fix decoding of chunked body.

Previously cowboy_http:te_chunked/2 would enter an incorrect state if
it tried to parse an incomplete chunk when the length was known from the
partial chunk.

Previosuly cowboy_http:te_chunked/2 expected the trailing "\r\n" to
always be present if chunk body was present in the buffer. This is not
guaranteed and so this commit accommodates that situation.
---
 src/cowboy_http.erl | 38 ++++++++++++++++++++++++++++++++------
 test/http_SUITE.erl | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl
index 7e20615..f2defdc 100644
--- a/src/cowboy_http.erl
+++ b/src/cowboy_http.erl
@@ -963,8 +963,17 @@ te_chunked(Data, {0, Streamed}) ->
 	%% @todo We are expecting an hex size, not a general token.
 	token(Data,
 		fun (<< "\r\n", Rest/binary >>, BinLen) ->
-				Len = list_to_integer(binary_to_list(BinLen), 16),
-				te_chunked(Rest, {Len, Streamed});
+				case list_to_integer(binary_to_list(BinLen), 16) of
+					%% Final chunk is parsed in one go above. Rest would be
+					%% <<\r\n">> if complete.
+					0 when byte_size(Rest) < 2 ->
+						more;
+					%% Normal chunk. Add 2 to Len for trailing <<"\r\n">>. Note
+					%% that repeated <<"-2\r\n">> would be streamed, and
+					%% accumulated, until out of memory if Len could be -2.
+					Len when Len > 0 ->
+						te_chunked(Rest, {Len + 2, Streamed})
+				end;
 			%% Chunk size shouldn't take too many bytes,
 			%% don't try to stream forever.
 			(Rest, _) when byte_size(Rest) < 16 ->
@@ -972,11 +981,28 @@ te_chunked(Data, {0, Streamed}) ->
 			(_, _) ->
 				{error, badarg}
 		end);
-te_chunked(Data, {ChunkRem, Streamed}) when byte_size(Data) >= ChunkRem + 2 ->
-	<< Chunk:ChunkRem/binary, "\r\n", Rest/binary >> = Data,
-	{ok, Chunk, Rest, {0, Streamed + byte_size(Chunk)}};
+%% <<"\n">> from trailing <<"\r\n">>.
+te_chunked(<< "\n", Rest/binary>>, {1, Streamed}) ->
+	{ok, <<>>, Rest, {0, Streamed}};
+te_chunked(<<>>, State={1, _Streamed}) ->
+	{more, 1, <<>>, State};
+%% Remainder of chunk (if any) and as much of trailing <<"\r\n">> as possible.
+te_chunked(Data, {ChunkRem, Streamed}) when byte_size(Data) >= ChunkRem - 2 ->
+	ChunkSize = ChunkRem - 2,
+	Streamed2 = Streamed + ChunkSize,
+	case Data of
+		<< Chunk:ChunkSize/binary, "\r\n", Rest/binary >> ->
+			{ok, Chunk, Rest, {0, Streamed2}};
+		<< Chunk:ChunkSize/binary, "\r" >> ->
+			{more, 1, Chunk, {1, Streamed2}};
+		<< Chunk:ChunkSize/binary >> ->
+			{more, 2, Chunk, {2, Streamed2}}
+	end;
+%% Incomplete chunk.
 te_chunked(Data, {ChunkRem, Streamed}) ->
-	{more, ChunkRem + 2, Data, {ChunkRem, Streamed}}.
+	ChunkRem2 = ChunkRem - byte_size(Data),
+	Streamed2 = Streamed + byte_size(Data),
+	{more, ChunkRem2, Data, {ChunkRem2, Streamed2}}.
 
 %% @doc Decode an identity stream.
 -spec te_identity(Bin, TransferState)
diff --git a/test/http_SUITE.erl b/test/http_SUITE.erl
index bdff0d0..d5cf7f6 100644
--- a/test/http_SUITE.erl
+++ b/test/http_SUITE.erl
@@ -88,6 +88,8 @@
 -export([te_chunked/1]).
 -export([te_chunked_chopped/1]).
 -export([te_chunked_delayed/1]).
+-export([te_chunked_split_body/1]).
+-export([te_chunked_split_crlf/1]).
 -export([te_identity/1]).
 
 %% ct.
@@ -162,6 +164,8 @@ groups() ->
 		te_chunked,
 		te_chunked_chopped,
 		te_chunked_delayed,
+		te_chunked_split_body,
+		te_chunked_split_crlf,
 		te_identity
 	],
 	[
@@ -1281,6 +1285,52 @@ te_chunked_delayed(Config) ->
 	{ok, 200, _, Client3} = cowboy_client:response(Client2),
 	{ok, Body, _} = cowboy_client:response_body(Client3).
 
+te_chunked_split_body(Config) ->
+	Client = ?config(client, Config),
+	Body = list_to_binary(io_lib:format("~p", [lists:seq(1, 100)])),
+	Chunks = body_to_chunks(50, Body, []),
+	{ok, Client2} = cowboy_client:request(<<"GET">>,
+		build_url("/echo/body", Config),
+		[{<<"transfer-encoding">>, <<"chunked">>}], Client),
+	{ok, Transport, Socket} = cowboy_client:transport(Client2),
+	_ = [begin
+		case Chunk of
+			%% Final chunk.
+			<<"0\r\n\r\n">> ->
+				ok = Transport:send(Socket, Chunk);
+			_ ->
+				%% Chunk of form <<"9\r\nChunkBody\r\n">>.
+				[Size, ChunkBody, <<>>] =
+					binary:split(Chunk, [<<"\r\n">>], [global]),
+				PartASize = random:uniform(byte_size(ChunkBody)),
+				<<PartA:PartASize/binary, PartB/binary>> = ChunkBody,
+				ok = Transport:send(Socket, [Size, <<"\r\n">>, PartA]),
+				ok = timer:sleep(10),
+				ok = Transport:send(Socket, [PartB, <<"\r\n">>])
+		end
+	end || Chunk <- Chunks],
+	{ok, 200, _, Client3} = cowboy_client:response(Client2),
+	{ok, Body, _} = cowboy_client:response_body(Client3).
+
+te_chunked_split_crlf(Config) ->
+	Client = ?config(client, Config),
+	Body = list_to_binary(io_lib:format("~p", [lists:seq(1, 100)])),
+	Chunks = body_to_chunks(50, Body, []),
+	{ok, Client2} = cowboy_client:request(<<"GET">>,
+		build_url("/echo/body", Config),
+		[{<<"transfer-encoding">>, <<"chunked">>}], Client),
+	{ok, Transport, Socket} = cowboy_client:transport(Client2),
+	_ = [begin
+		%% <<"\r\n">> is last 2 bytes of Chunk split before or after <<"\r">>.
+		Len = byte_size(Chunk) - (random:uniform(2) - 1),
+		<<Chunk2:Len/binary, End/binary>> = Chunk,
+		ok = Transport:send(Socket, Chunk2),
+		ok = timer:sleep(10),
+		ok = Transport:send(Socket, End)
+	end || Chunk <- Chunks],
+	{ok, 200, _, Client3} = cowboy_client:response(Client2),
+	{ok, Body, _} = cowboy_client:response_body(Client3).
+
 te_identity(Config) ->
 	Client = ?config(client, Config),
 	Body = list_to_binary(io_lib:format("~p", [lists:seq(1, 100)])),
-- 
cgit v1.2.3