From 84b4128d068cddc37bbb45e755044183f3e9cd97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Sun, 29 Apr 2018 11:29:26 +0200 Subject: Receive and ignore HTTP/2 request trailers if any This is a first step toward properly supporting request trailers. --- src/cowboy_http2.erl | 69 +++++++++++++++++++---- test/rfc7540_SUITE.erl | 145 +++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 192 insertions(+), 22 deletions(-) diff --git a/src/cowboy_http2.erl b/src/cowboy_http2.erl index 9bd1bda..bb075ed 100644 --- a/src/cowboy_http2.erl +++ b/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 diff --git a/test/rfc7540_SUITE.erl b/test/rfc7540_SUITE.erl index 85e83a5..3d8bac4 100644 --- a/test/rfc7540_SUITE.erl +++ b/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>>, + <>, + 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>>, + <>, + 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 " -- cgit v1.2.3