Receive and ignore HTTP/2 request trailers if any

This is a first step toward properly supporting request trailers.

src/cowboy_http2.erl

@@ -420,14 +420,28 @@ frame(State0=#state{remote_window=ConnWindow, streams=Streams, lingering_streams
 frame(State, {headers, StreamID, _, _, _}) when StreamID rem 2 =:= 0 ->
 	terminate(State, {connection_error, protocol_error,
 		'HEADERS frame received with even-numbered streamid. (RFC7540 5.1.1)'});
-%% HEADERS frame received on (half-)closed stream.
-%%
-%% We always close the connection here to avoid having to decode
-%% the headers to not waste resources on non-compliant clients.
-frame(State=#state{client_streamid=LastStreamID}, {headers, StreamID, _, _, _})
+%% Either a HEADERS frame received on (half-)closed stream,
+%% or a HEADERS frame containing the trailers.
+frame(State=#state{client_streamid=LastStreamID, streams=Streams},
+		{headers, StreamID, IsFin, IsHeadFin, HeaderBlockOrFragment})
 		when StreamID =< LastStreamID ->
-	terminate(State, {connection_error, stream_closed,
-		'HEADERS frame received on a stream in closed or half-closed state. (RFC7540 5.1)'});
+	case lists:keyfind(StreamID, #stream.id, Streams) of
+		Stream = #stream{remote=nofin} when IsFin =:= fin ->
+			case IsHeadFin of
+				head_fin ->
+					stream_decode_trailers(State, Stream, HeaderBlockOrFragment);
+				head_nofin ->
+					State#state{parse_state={continuation, StreamID, IsFin, HeaderBlockOrFragment}}
+			end;
+		%% We always close the connection here to avoid having to decode
+		%% the headers to not waste resources on non-compliant clients.
+		#stream{remote=nofin} when IsFin =:= nofin ->
+			terminate(State, {connection_error, protocol_error,
+				'Trailing HEADERS frame received without the END_STREAM flag set. (RFC7540 8.1, RFC7540 8.1.2.6)'});
+		_ ->
+			terminate(State, {connection_error, stream_closed,
+				'HEADERS frame received on a stream in closed or half-closed state. (RFC7540 5.1)'})
+	end;
 %% Single HEADERS frame headers block.
 frame(State, {headers, StreamID, IsFin, head_fin, HeaderBlock}) ->
 	%% @todo We probably need to validate StreamID here and in 4 next clauses.
@@ -544,10 +558,17 @@ frame(State, {continuation, _, _, _}) ->
 	terminate(State, {connection_error, protocol_error,
 		'CONTINUATION frames MUST be preceded by a HEADERS frame. (RFC7540 6.10)'}).
 
-continuation_frame(State=#state{parse_state={continuation, StreamID, IsFin, HeaderBlockFragment0}},
+continuation_frame(State=#state{client_streamid=LastStreamID, streams=Streams,
+		parse_state={continuation, StreamID, IsFin, HeaderBlockFragment0}},
 		{continuation, StreamID, head_fin, HeaderBlockFragment1}) ->
-	stream_decode_init(State#state{parse_state=normal}, StreamID, IsFin,
-		<< HeaderBlockFragment0/binary, HeaderBlockFragment1/binary >>);
+	HeaderBlock = << HeaderBlockFragment0/binary, HeaderBlockFragment1/binary >>,
+	case StreamID > LastStreamID of
+		true -> %% New stream.
+			stream_decode_init(State#state{parse_state=normal}, StreamID, IsFin, HeaderBlock);
+		false -> %% Trailers.
+			Stream = lists:keyfind(StreamID, #stream.id, Streams),
+			stream_decode_trailers(State, Stream, HeaderBlock)
+	end;
 continuation_frame(State=#state{parse_state={continuation, StreamID, IsFin, HeaderBlockFragment0}},
 		{continuation, StreamID, head_nofin, HeaderBlockFragment1}) ->
 	State#state{parse_state={continuation, StreamID, IsFin,
@@ -1181,6 +1202,34 @@ stream_handler_init(State=#state{opts=Opts,
 			'Unhandled exception in cowboy_stream:init/3.'})
 	end.
 
+stream_decode_trailers(State=#state{decode_state=DecodeState0}, Stream, HeaderBlock) ->
+	try cow_hpack:decode(HeaderBlock, DecodeState0) of
+		{Headers, DecodeState} ->
+			stream_reject_pseudo_headers_in_trailers(State#state{decode_state=DecodeState},
+				Stream#stream{remote=fin}, Headers)
+	catch _:_ ->
+		terminate(State, {connection_error, compression_error,
+			'Error while trying to decode HPACK-encoded header block. (RFC7540 4.3)'})
+	end.
+
+stream_reject_pseudo_headers_in_trailers(State, Stream=#stream{id=StreamID}, Headers) ->
+	case has_pseudo_header(Headers) of
+		false ->
+			%% @todo There's probably a number of regular headers forbidden too.
+			%% @todo Propagate trailers.
+			after_commands(State, Stream);
+		true ->
+			stream_reset(State, StreamID, {stream_error, protocol_error,
+				'Trailer header blocks must not contain pseudo-headers. (RFC7540 8.1.2.1)'})
+	end.
+
+has_pseudo_header([]) ->
+	false;
+has_pseudo_header([{<<":", _/bits>>, _}|_]) ->
+	true;
+has_pseudo_header([_|Tail]) ->
+	has_pseudo_header(Tail).
+
 %% @todo Don't send an RST_STREAM if one was already sent.
 stream_reset(State=#state{socket=Socket, transport=Transport}, StreamID, StreamError) ->
 	Reason = case StreamError of

test/rfc7540_SUITE.erl

@@ -3173,6 +3173,114 @@ settings_initial_window_size_reject_overflow(Config) ->
 %   behavior.  These MAY be treated by an implementation as being
 %   equivalent to INTERNAL_ERROR.
 
+accept_trailers(Config) ->
+	doc("Trailing HEADERS frames must be accepted. (RFC7540 8.1)"),
+	{ok, Socket} = do_handshake(Config),
+	%% Send a request containing DATA and trailing HEADERS frames.
+	{HeadersBlock, EncodeState} = cow_hpack:encode([
+		{<<":method">>, <<"POST">>},
+		{<<":scheme">>, <<"http">>},
+		{<<":authority">>, <<"localhost">>}, %% @todo Correct port number.
+		{<<":path">>, <<"/long_polling">>},
+		{<<"trailer">>, <<"x-checksum">>}
+	]),
+	{TrailersBlock, _} = cow_hpack:encode([
+		{<<"x-checksum">>, <<"md5:4cc909a007407f3706399b6496babec3">>}
+	], EncodeState),
+	ok = gen_tcp:send(Socket, [
+		cow_http2:headers(1, nofin, HeadersBlock),
+		cow_http2:data(1, nofin, <<0:10000/unit:8>>),
+		cow_http2:headers(1, fin, TrailersBlock)
+	]),
+	%% Receive a HEADERS frame as a response.
+	{ok, << _:24, 1:8, _:40 >>} = gen_tcp:recv(Socket, 9, 6000),
+	ok.
+
+accept_trailers_continuation(Config) ->
+	doc("Trailing HEADERS and CONTINUATION frames must be accepted. (RFC7540 8.1)"),
+	{ok, Socket} = do_handshake(Config),
+	%% Send a request containing DATA and trailing HEADERS and CONTINUATION frames.
+	{HeadersBlock, EncodeState} = cow_hpack:encode([
+		{<<":method">>, <<"POST">>},
+		{<<":scheme">>, <<"http">>},
+		{<<":authority">>, <<"localhost">>}, %% @todo Correct port number.
+		{<<":path">>, <<"/long_polling">>},
+		{<<"trailer">>, <<"x-checksum">>}
+	]),
+	{TrailersBlock, _} = cow_hpack:encode([
+		{<<"x-checksum">>, <<"md5:4cc909a007407f3706399b6496babec3">>}
+	], EncodeState),
+	Len = iolist_size(TrailersBlock),
+	ok = gen_tcp:send(Socket, [
+		cow_http2:headers(1, nofin, HeadersBlock),
+		cow_http2:data(1, nofin, <<0:10000/unit:8>>),
+		<<0:24, 1:8, 0:7, 1:1, 0:1, 1:31>>,
+		<<Len:24, 9:8, 0:5, 1:1, 0:3, 1:31>>,
+		TrailersBlock
+	]),
+	%% Receive a HEADERS frame as a response.
+	{ok, << _:24, 1:8, _:40 >>} = gen_tcp:recv(Socket, 9, 6000),
+	ok.
+
+%% We reject all invalid HEADERS with a connection error because
+%% we do not want to waste resources decoding them.
+reject_trailers_nofin(Config) ->
+	doc("Trailing HEADERS frames received without the END_STREAM flag "
+		"set must be rejected with a PROTOCOL_ERROR connection error. "
+		"(RFC7540 8.1, RFC7540 8.1.2.6)"),
+	{ok, Socket} = do_handshake(Config),
+	%% Send a request containing DATA and trailing HEADERS frames.
+	%% The trailing HEADERS does not have the END_STREAM flag set.
+	{HeadersBlock, EncodeState} = cow_hpack:encode([
+		{<<":method">>, <<"POST">>},
+		{<<":scheme">>, <<"http">>},
+		{<<":authority">>, <<"localhost">>}, %% @todo Correct port number.
+		{<<":path">>, <<"/long_polling">>},
+		{<<"trailer">>, <<"x-checksum">>}
+	]),
+	{TrailersBlock, _} = cow_hpack:encode([
+		{<<"x-checksum">>, <<"md5:4cc909a007407f3706399b6496babec3">>}
+	], EncodeState),
+	ok = gen_tcp:send(Socket, [
+		cow_http2:headers(1, nofin, HeadersBlock),
+		cow_http2:data(1, nofin, <<0:10000/unit:8>>),
+		cow_http2:headers(1, nofin, TrailersBlock)
+	]),
+	%% Receive a PROTOCOL_ERROR connection error.
+	{ok, << _:24, 7:8, _:72, 1:32 >>} = gen_tcp:recv(Socket, 17, 6000),
+	ok.
+
+%% We reject all invalid HEADERS with a connection error because
+%% we do not want to waste resources decoding them.
+reject_trailers_nofin_continuation(Config) ->
+	doc("Trailing HEADERS frames received without the END_STREAM flag "
+		"set must be rejected with a PROTOCOL_ERROR connection error. "
+		"(RFC7540 8.1, RFC7540 8.1.2.6)"),
+	{ok, Socket} = do_handshake(Config),
+	%% Send a request containing DATA and trailing HEADERS and CONTINUATION frames.
+	%% The trailing HEADERS does not have the END_STREAM flag set.
+	{HeadersBlock, EncodeState} = cow_hpack:encode([
+		{<<":method">>, <<"POST">>},
+		{<<":scheme">>, <<"http">>},
+		{<<":authority">>, <<"localhost">>}, %% @todo Correct port number.
+		{<<":path">>, <<"/long_polling">>},
+		{<<"trailer">>, <<"x-checksum">>}
+	]),
+	{TrailersBlock, _} = cow_hpack:encode([
+		{<<"x-checksum">>, <<"md5:4cc909a007407f3706399b6496babec3">>}
+	], EncodeState),
+	Len = iolist_size(TrailersBlock),
+	ok = gen_tcp:send(Socket, [
+		cow_http2:headers(1, nofin, HeadersBlock),
+		cow_http2:data(1, nofin, <<0:10000/unit:8>>),
+		<<0:24, 1:8, 0:9, 1:31>>,
+		<<Len:24, 9:8, 0:5, 1:1, 0:3, 1:31>>,
+		TrailersBlock
+	]),
+	%% Receive a PROTOCOL_ERROR connection error.
+	{ok, << _:24, 7:8, _:72, 1:32 >>} = gen_tcp:recv(Socket, 17, 6000),
+	ok.
+
 headers_informational_nofin(Config) ->
 	doc("Informational HEADERS frames must not have the END_STREAM flag set. (RFC7540 8.1)"),
 	{ok, Socket} = do_handshake(Config),
@@ -3194,13 +3302,6 @@ headers_informational_nofin(Config) ->
 	{_, <<"100">>} = lists:keyfind(<<":status">>, 1, RespHeaders),
 	ok.
 
-%% (RFC7540 8.1)
-%   A HEADERS frame (and associated CONTINUATION frames) can only appear
-%   at the start or end of a stream.  An endpoint that receives a HEADERS
-%   frame without the END_STREAM flag set after receiving a final (non-
-%   informational) status code MUST treat the corresponding request or
-%   response as malformed (Section 8.1.2.6).
-%
 %% @todo This one is interesting to implement because Cowboy DOES this.
 %   A server can
 %   send a complete response prior to the client sending an entire
@@ -3261,11 +3362,31 @@ reject_unknown_pseudo_headers(Config) ->
 	{ok, << _:24, 3:8, _:8, 1:32, 1:32 >>} = gen_tcp:recv(Socket, 13, 6000),
 	ok.
 
-%% @todo Implement request trailers. reject_pseudo_headers_in_trailers(Config) ->
-%   Pseudo-header fields MUST NOT appear in trailers.
-%   Endpoints MUST treat a request or response that contains
-%   undefined or invalid pseudo-header fields as malformed
-%   (Section 8.1.2.6).
+reject_pseudo_headers_in_trailers(Config) ->
+	doc("Requests containing pseudo-headers in trailers must be rejected "
+		"with a PROTOCOL_ERROR stream error. (RFC7540 8.1.2.1, RFC7540 8.1.2.6)"),
+	{ok, Socket} = do_handshake(Config),
+	%% Send a request containing DATA and trailing HEADERS frames.
+	%% The trailing HEADERS contains pseudo-headers.
+	{HeadersBlock, EncodeState} = cow_hpack:encode([
+		{<<":method">>, <<"POST">>},
+		{<<":scheme">>, <<"http">>},
+		{<<":authority">>, <<"localhost">>}, %% @todo Correct port number.
+		{<<":path">>, <<"/long_polling">>},
+		{<<"trailer">>, <<"x-checksum">>}
+	]),
+	{TrailersBlock, _} = cow_hpack:encode([
+		{<<"x-checksum">>, <<"md5:4cc909a007407f3706399b6496babec3">>},
+		{<<":path">>, <<"/">>}
+	], EncodeState),
+	ok = gen_tcp:send(Socket, [
+		cow_http2:headers(1, nofin, HeadersBlock),
+		cow_http2:data(1, nofin, <<0:10000/unit:8>>),
+		cow_http2:headers(1, fin, TrailersBlock)
+	]),
+	%% Receive a PROTOCOL_ERROR stream error.
+	{ok, << _:24, 3:8, _:8, 1:32, 1:32 >>} = gen_tcp:recv(Socket, 13, 6000),
+	ok.
 
 reject_pseudo_headers_after_regular_headers(Config) ->
 	doc("Requests containing pseudo-headers after regular headers must be rejected "