aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLoïc Hoguin <[email protected]>2018-04-30 13:47:33 +0200
committerLoïc Hoguin <[email protected]>2018-04-30 13:47:33 +0200
commit8d1f468ac00427184c094392bb5f7a465ab04364 (patch)
tree51530013099ca99e30bc3285472afd44a19cd8d5
parent17349fafc2b64cf424e7bae4e33d3e6bd9deec6e (diff)
downloadcowboy-8d1f468ac00427184c094392bb5f7a465ab04364.tar.gz
cowboy-8d1f468ac00427184c094392bb5f7a465ab04364.tar.bz2
cowboy-8d1f468ac00427184c094392bb5f7a465ab04364.zip
Reject HTTP/2 requests with a body size different than content-length
-rw-r--r--src/cowboy_http2.erl82
-rw-r--r--test/rfc7540_SUITE.erl98
2 files changed, 155 insertions, 25 deletions
diff --git a/src/cowboy_http2.erl b/src/cowboy_http2.erl
index bb075ed..0e3a7bb 100644
--- a/src/cowboy_http2.erl
+++ b/src/cowboy_http2.erl
@@ -62,6 +62,9 @@
remote = nofin :: cowboy_stream:fin(),
%% Remote flow control window (how much we accept to receive).
remote_window :: integer(),
+ %% Size expected and read from the request body.
+ remote_expected_size = undefined :: undefined | non_neg_integer(),
+ remote_read_size = 0 :: non_neg_integer(),
%% Unparsed te header. Used to know if we can send trailers.
te :: undefined | binary()
}).
@@ -386,20 +389,8 @@ frame(State0=#state{remote_window=ConnWindow, streams=Streams, lingering_streams
#stream{remote_window=StreamWindow} when StreamWindow < DataLen ->
stream_reset(State, StreamID, {stream_error, flow_control_error,
'DATA frame overflowed the stream flow control window. (RFC7540 6.9, RFC7540 6.9.1)'});
- Stream = #stream{state=flush, remote=nofin, remote_window=StreamWindow} ->
- after_commands(State, Stream#stream{remote=IsFin, remote_window=StreamWindow - DataLen});
- Stream = #stream{state=StreamState0, remote=nofin, remote_window=StreamWindow} ->
- try cowboy_stream:data(StreamID, IsFin, Data, StreamState0) of
- {Commands, StreamState} ->
- commands(State, Stream#stream{state=StreamState, remote=IsFin,
- remote_window=StreamWindow - DataLen}, Commands)
- catch Class:Exception ->
- cowboy_stream:report_error(data,
- [StreamID, IsFin, Data, StreamState0],
- Class, Exception, erlang:get_stacktrace()),
- stream_reset(State, StreamID, {internal_error, {Class, Exception},
- 'Unhandled exception in cowboy_stream:data/4.'})
- end;
+ Stream = #stream{remote=nofin} ->
+ data_frame(State, Stream, IsFin, DataLen, Data);
#stream{remote=fin} ->
stream_reset(State, StreamID, {stream_error, stream_closed,
'DATA frame received for a half-closed (remote) stream. (RFC7540 5.1)'});
@@ -558,6 +549,50 @@ frame(State, {continuation, _, _, _}) ->
terminate(State, {connection_error, protocol_error,
'CONTINUATION frames MUST be preceded by a HEADERS frame. (RFC7540 6.10)'}).
+data_frame(State, Stream0=#stream{id=StreamID, remote_window=StreamWindow,
+ remote_read_size=StreamRead}, IsFin, DataLen, Data) ->
+ Stream = Stream0#stream{remote=IsFin,
+ remote_window=StreamWindow - DataLen,
+ remote_read_size=StreamRead + DataLen},
+ case is_body_size_valid(Stream) of
+ true ->
+ data_frame(State, Stream, IsFin, Data);
+ false ->
+ stream_reset(after_commands(State, Stream), StreamID, {stream_error, protocol_error,
+ 'The total size of DATA frames is different than the content-length. (RFC7540 8.1.2.6)'})
+ end.
+
+%% We ignore DATA frames for streams that are flushing out data.
+data_frame(State, Stream=#stream{state=flush}, _, _) ->
+ after_commands(State, Stream);
+data_frame(State, Stream=#stream{id=StreamID, state=StreamState0}, IsFin, Data) ->
+ try cowboy_stream:data(StreamID, IsFin, Data, StreamState0) of
+ {Commands, StreamState} ->
+ commands(State, Stream#stream{state=StreamState}, Commands)
+ catch Class:Exception ->
+ cowboy_stream:report_error(data,
+ [StreamID, IsFin, Data, StreamState0],
+ Class, Exception, erlang:get_stacktrace()),
+ stream_reset(State, StreamID, {internal_error, {Class, Exception},
+ 'Unhandled exception in cowboy_stream:data/4.'})
+ end.
+
+%% It's always valid when no content-length header was specified.
+is_body_size_valid(#stream{remote_expected_size=undefined}) ->
+ true;
+%% We didn't finish reading the body but the size is already larger than expected.
+is_body_size_valid(#stream{remote=nofin, remote_expected_size=Expected,
+ remote_read_size=Read}) when Read > Expected ->
+ false;
+is_body_size_valid(#stream{remote=nofin}) ->
+ true;
+is_body_size_valid(#stream{remote=fin, remote_expected_size=Expected,
+ remote_read_size=Expected}) ->
+ true;
+%% We finished reading the body and the size read is not the one expected.
+is_body_size_valid(_) ->
+ false.
+
continuation_frame(State=#state{client_streamid=LastStreamID, streams=Streams,
parse_state={continuation, StreamID, IsFin, HeaderBlockFragment0}},
{continuation, StreamID, head_fin, HeaderBlockFragment1}) ->
@@ -1071,6 +1106,9 @@ headers_to_map([{Name, Value}|Tail], Acc0) ->
stream_req_init(State, StreamID, IsFin, Headers, PseudoHeaders) ->
case Headers of
+ #{<<"content-length">> := BinLength} when BinLength =/= <<"0">>, IsFin =:= fin ->
+ stream_malformed(State, StreamID,
+ 'HEADERS frame with the END_STREAM flag contains a non-zero content-length. (RFC7540 8.1.2.6)');
_ when IsFin =:= fin ->
stream_req_init(State, StreamID, IsFin, Headers, PseudoHeaders, 0);
#{<<"content-length">> := <<"0">>} ->
@@ -1185,13 +1223,14 @@ stream_handler_init(State=#state{opts=Opts,
local_settings=#{initial_window_size := RemoteWindow},
remote_settings=#{initial_window_size := LocalWindow}},
StreamID, RemoteIsFin, LocalIsFin,
- Req=#{method := Method, headers := Headers}) ->
+ Req=#{method := Method, headers := Headers, body_length := BodyLength}) ->
try cowboy_stream:init(StreamID, Req, Opts) of
{Commands, StreamState} ->
commands(State#state{client_streamid=StreamID},
#stream{id=StreamID, state=StreamState,
method=Method, remote=RemoteIsFin, local=LocalIsFin,
local_window=LocalWindow, remote_window=RemoteWindow,
+ remote_expected_size=BodyLength,
te=maps:get(<<"te">>, Headers, undefined)},
Commands)
catch Class:Exception ->
@@ -1205,13 +1244,22 @@ stream_handler_init(State=#state{opts=Opts,
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_trailers_is_body_size_valid(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_trailers_is_body_size_valid(State, Stream=#stream{id=StreamID}, Headers) ->
+ case is_body_size_valid(Stream) of
+ true ->
+ stream_reject_pseudo_headers_in_trailers(State, Stream, Headers);
+ false ->
+ stream_reset(after_commands(State, Stream), StreamID, {stream_error, protocol_error,
+ 'The total size of DATA frames is different than the content-length. (RFC7540 8.1.2.6)'})
+ end.
+
stream_reject_pseudo_headers_in_trailers(State, Stream=#stream{id=StreamID}, Headers) ->
case has_pseudo_header(Headers) of
false ->
@@ -1219,7 +1267,7 @@ stream_reject_pseudo_headers_in_trailers(State, Stream=#stream{id=StreamID}, Hea
%% @todo Propagate trailers.
after_commands(State, Stream);
true ->
- stream_reset(State, StreamID, {stream_error, protocol_error,
+ stream_reset(after_commands(State, Stream), StreamID, {stream_error, protocol_error,
'Trailer header blocks must not contain pseudo-headers. (RFC7540 8.1.2.1)'})
end.
diff --git a/test/rfc7540_SUITE.erl b/test/rfc7540_SUITE.erl
index adc84b5..509d1d3 100644
--- a/test/rfc7540_SUITE.erl
+++ b/test/rfc7540_SUITE.erl
@@ -3778,14 +3778,96 @@ reject_many_pseudo_header_path(Config) ->
% before being passed into a non-HTTP/2 context, such as an HTTP/1.1
% connection, or a generic HTTP server application.
-%% (RFC7540 8.1.2.6)
-% A request or response that includes a payload body can include a
-% content-length header field. A request or response is also malformed
-% if the value of a content-length header field does not equal the sum
-% of the DATA frame payload lengths that form the body. A response
-% that is defined to have no payload, as described in [RFC7230],
-% Section 3.3.2, can have a non-zero content-length header field, even
-% though no content is included in DATA frames.
+reject_data_size_smaller_than_content_length(Config) ->
+ doc("Requests that have a content-length header whose value does not "
+ "match the total length of the DATA frames must be rejected with "
+ "a PROTOCOL_ERROR stream error. (RFC7540 8.1.2.6)"),
+ {ok, Socket} = do_handshake(Config),
+ %% Send a HEADERS frame with a content-length header different
+ %% than the sum of the DATA frame sizes.
+ {HeadersBlock, _} = cow_hpack:encode([
+ {<<":method">>, <<"POST">>},
+ {<<":scheme">>, <<"http">>},
+ {<<":authority">>, <<"localhost">>}, %% @todo Correct port number.
+ {<<":path">>, <<"/long_polling">>},
+ {<<"content-length">>, <<"12">>}
+ ]),
+ ok = gen_tcp:send(Socket, [
+ cow_http2:headers(1, nofin, HeadersBlock),
+ cow_http2:data(1, fin, <<"Hello!">>)
+ ]),
+ %% Receive a PROTOCOL_ERROR stream error.
+ {ok, << _:24, 3:8, _:8, 1:32, 1:32 >>} = gen_tcp:recv(Socket, 13, 6000),
+ ok.
+
+reject_data_size_larger_than_content_length(Config) ->
+ doc("Requests that have a content-length header whose value does not "
+ "match the total length of the DATA frames must be rejected with "
+ "a PROTOCOL_ERROR stream error. (RFC7540 8.1.2.6)"),
+ {ok, Socket} = do_handshake(Config),
+ %% Send a HEADERS frame with a content-length header different
+ %% than the sum of the DATA frame sizes.
+ {HeadersBlock, _} = cow_hpack:encode([
+ {<<":method">>, <<"POST">>},
+ {<<":scheme">>, <<"http">>},
+ {<<":authority">>, <<"localhost">>}, %% @todo Correct port number.
+ {<<":path">>, <<"/long_polling">>},
+ {<<"content-length">>, <<"12">>}
+ ]),
+ ok = gen_tcp:send(Socket, [
+ cow_http2:headers(1, nofin, HeadersBlock),
+ cow_http2:data(1, nofin, <<"Hello! World! Universe!">>),
+ cow_http2:data(1, fin, <<"Multiverse!">>)
+ ]),
+ %% Receive a PROTOCOL_ERROR stream error.
+ {ok, << _:24, 3:8, _:8, 1:32, 1:32 >>} = gen_tcp:recv(Socket, 13, 6000),
+ ok.
+
+reject_content_length_without_data(Config) ->
+ doc("Requests that have a content-length header whose value does not "
+ "match the total length of the DATA frames must be rejected with "
+ "a PROTOCOL_ERROR stream error. (RFC7540 8.1.2.6)"),
+ {ok, Socket} = do_handshake(Config),
+ %% Send a HEADERS frame with a content-length header different
+ %% than the sum of the DATA frame sizes.
+ {HeadersBlock, _} = cow_hpack:encode([
+ {<<":method">>, <<"POST">>},
+ {<<":scheme">>, <<"http">>},
+ {<<":authority">>, <<"localhost">>}, %% @todo Correct port number.
+ {<<":path">>, <<"/long_polling">>},
+ {<<"content-length">>, <<"12">>}
+ ]),
+ ok = gen_tcp:send(Socket, cow_http2:headers(1, fin, HeadersBlock)),
+ %% Receive a PROTOCOL_ERROR stream error.
+ {ok, << _:24, 3:8, _:8, 1:32, 1:32 >>} = gen_tcp:recv(Socket, 13, 6000),
+ ok.
+
+reject_data_size_different_than_content_length_with_trailers(Config) ->
+ doc("Requests that have a content-length header whose value does not "
+ "match the total length of the DATA frames must be rejected with "
+ "a PROTOCOL_ERROR stream error. (RFC7540 8.1.2.6)"),
+ {ok, Socket} = do_handshake(Config),
+ %% Send a HEADERS frame with a content-length header different
+ %% than the sum of the DATA frame sizes.
+ {HeadersBlock, EncodeState} = cow_hpack:encode([
+ {<<":method">>, <<"POST">>},
+ {<<":scheme">>, <<"http">>},
+ {<<":authority">>, <<"localhost">>}, %% @todo Correct port number.
+ {<<":path">>, <<"/long_polling">>},
+ {<<"content-length">>, <<"12">>},
+ {<<"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, <<"Hello!">>),
+ 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_duplicate_content_length_header(Config) ->
doc("A request with duplicate content-length headers must be rejected "