diff options
| -rw-r--r-- | rebar.config | 2 | ||||
| -rw-r--r-- | src/cowboy_http.erl | 41 | ||||
| -rw-r--r-- | src/cowboy_http2.erl | 9 | ||||
| -rw-r--r-- | src/cowboy_req.erl | 1 | ||||
| -rw-r--r-- | test/rfc7540_SUITE.erl | 45 |
5 files changed, 62 insertions, 36 deletions
diff --git a/rebar.config b/rebar.config index 63ec61d..6fbd3d8 100644 --- a/rebar.config +++ b/rebar.config @@ -1,2 +1,2 @@ -{deps, [{cowlib,".*",{git,"https://github.com/ninenines/cowlib","master"}},{ranch,".*",{git,"https://github.com/ninenines/ranch","1.1.0"}}]}. +{deps, [{cowlib,".*",{git,"https://github.com/ninenines/cowlib","master"}},{ranch,".*",{git,"https://github.com/ninenines/ranch","1.2.1"}}]}. {erl_opts, [debug_info,warn_export_vars,warn_shadow_vars,warn_obsolete_guard,warn_export_all,warn_missing_spec,warn_untyped_record]}. diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl index 1582770..4608839 100644 --- a/src/cowboy_http.erl +++ b/src/cowboy_http.erl @@ -489,9 +489,13 @@ parse_hd_before_value(Buffer, State=#state{opts=Opts, in_state=PS}, H, N) -> parse_hd_value(Buffer, State, H, N, <<>>) end. -parse_hd_value(<< $\r, $\n, Rest/bits >>, S, Headers, Name, SoFar) -> - %% @todo What to do about duplicate header names. - parse_header(Rest, S, Headers#{Name => clean_value_ws_end(SoFar, byte_size(SoFar) - 1)}); +parse_hd_value(<< $\r, $\n, Rest/bits >>, S, Headers0, Name, SoFar) -> + Value = clean_value_ws_end(SoFar, byte_size(SoFar) - 1), + Headers = case maps:get(Name, Headers0, undefined) of + undefined -> Headers0#{Name => Value}; + Value0 -> Headers0#{Name => << Value0/binary, ", ", Value/binary >>} + end, + parse_header(Rest, S, Headers); parse_hd_value(<< C, Rest/bits >>, S, H, N, SoFar) -> parse_hd_value(Rest, S, H, N, << SoFar/binary, C >>). @@ -641,8 +645,8 @@ request(Buffer, State0=#state{ref=Ref, transport=Transport, in_streamid=StreamID set_request_timeout(State0#state{in_streamid=StreamID + 1, in_state=#ps_request_line{}}) end, {request, Req, State, Buffer}; - {true, SettingsPayload} -> - http2_upgrade(State0, Buffer, SettingsPayload, Req) + {true, HTTP2Settings} -> + http2_upgrade(State0, Buffer, HTTP2Settings, Req) end. %% HTTP/2 upgrade. @@ -650,15 +654,12 @@ request(Buffer, State0=#state{ref=Ref, transport=Transport, in_streamid=StreamID is_http2_upgrade(#{<<"connection">> := Conn, <<"upgrade">> := Upgrade, <<"http2-settings">> := HTTP2Settings}, 'HTTP/1.1') -> Conns = cow_http_hd:parse_connection(Conn), - io:format(user, "CONNS ~p~n", [Conns]), case {lists:member(<<"upgrade">>, Conns), lists:member(<<"http2-settings">>, Conns)} of {true, true} -> Protocols = cow_http_hd:parse_upgrade(Upgrade), - io:format(user, "PROTOCOLS ~p~n", [Protocols]), case lists:member(<<"h2c">>, Protocols) of true -> - SettingsPayload = cow_http_hd:parse_http2_settings(HTTP2Settings), - {true, SettingsPayload}; + {true, HTTP2Settings}; false -> false end; @@ -683,20 +684,26 @@ http2_upgrade(State=#state{parent=Parent, ref=Ref, socket=Socket, transport=Tran end. http2_upgrade(State=#state{parent=Parent, ref=Ref, socket=Socket, transport=Transport, - opts=Opts, handler=Handler}, Buffer, SettingsPayload, Req) -> + opts=Opts, handler=Handler}, Buffer, HTTP2Settings, Req) -> %% @todo %% However if the client sent a body, we need to read the body in full %% and if we can't do that, return a 413 response. Some options are in order. %% Always half-closed stream coming from this side. - Transport:send(Socket, cow_http:response(101, 'HTTP/1.1', maps:to_list(#{ - <<"connection">> => <<"Upgrade">>, - <<"upgrade">> => <<"h2c">> - }))), + try cow_http_hd:parse_http2_settings(HTTP2Settings) of + Settings -> + Transport:send(Socket, cow_http:response(101, 'HTTP/1.1', maps:to_list(#{ + <<"connection">> => <<"Upgrade">>, + <<"upgrade">> => <<"h2c">> + }))), - %% @todo Possibly redirect the request if it was https. - _ = cancel_request_timeout(State), - cowboy_http2:init(Parent, Ref, Socket, Transport, Opts, Handler, Buffer, SettingsPayload, Req). + %% @todo Possibly redirect the request if it was https. + _ = cancel_request_timeout(State), + cowboy_http2:init(Parent, Ref, Socket, Transport, Opts, Handler, Buffer, Settings, Req) + catch _:_ -> + error_terminate(400, State, {connection_error, protocol_error, + 'The HTTP2-Settings header contains a base64 SETTINGS payload. (RFC7540 3.2, RFC7540 3.2.1)'}) + end. %% Request body parsing. diff --git a/src/cowboy_http2.erl b/src/cowboy_http2.erl index d73c879..77a27a9 100644 --- a/src/cowboy_http2.erl +++ b/src/cowboy_http2.erl @@ -96,11 +96,11 @@ init(Parent, Ref, Socket, Transport, Opts, Handler, Buffer) -> %% @todo Add an argument for the request body. -spec init(pid(), ranch:ref(), inet:socket(), module(), cowboy:opts(), module(), binary(), binary() | undefined, cowboy_req:req()) -> ok. -init(Parent, Ref, Socket, Transport, Opts, Handler, Buffer, SettingsPayload, Req) -> +init(Parent, Ref, Socket, Transport, Opts, Handler, Buffer, _Settings, Req) -> State0 = #state{parent=Parent, ref=Ref, socket=Socket, transport=Transport, opts=Opts, handler=Handler}, preface(State0), - %% @todo SettingsPayload. + %% @todo Apply settings. %% StreamID from HTTP/1.1 Upgrade requests is always 1. %% The stream is always in the half-closed (remote) state. State = stream_handler_init(State0, 1, fin, Req), @@ -247,9 +247,8 @@ frame(State, {priority, _StreamID, _IsExclusive, _DepStreamID, _Weight}) -> frame(State, {rst_stream, StreamID, Reason}) -> stream_reset(State, StreamID, {stream_error, Reason, 'Stream reset requested by client.'}); %% SETTINGS frame. -frame(State=#state{socket=Socket, transport=Transport}, {settings, Settings}) -> +frame(State=#state{socket=Socket, transport=Transport}, {settings, _Settings}) -> %% @todo Apply SETTINGS. - io:format("settings ~p~n", [Settings]), Transport:send(Socket, cow_http2:settings_ack()), State; %% Ack for a previously sent SETTINGS frame. @@ -381,7 +380,7 @@ commands(State, StreamID, [{upgrade, _Mod, _ModState}]) -> commands(State, StreamID, [{upgrade, _Mod, _ModState}|Tail]) -> %% @todo This is an error. Not sure what to do here yet. commands(State, StreamID, Tail); -commands(State, StreamID, [stop|Tail]) -> +commands(State, StreamID, [stop|_Tail]) -> %% @todo Do we want to run the commands after a stop? stream_terminate(State, StreamID, stop). diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl index 998c2fe..8fa9927 100644 --- a/src/cowboy_req.erl +++ b/src/cowboy_req.erl @@ -384,7 +384,6 @@ read_body(Req=#{pid := Pid, streamid := StreamID}, Opts) -> end, Ref = make_ref(), Pid ! {{Pid, StreamID}, {read_body, Ref, Length}}, -% io:format("READ_BODY ~p ~p ~p ~p~n", [Pid, StreamID, Ref, Length]), receive {request_body, Ref, nofin, Body} -> {more, Body, Req}; diff --git a/test/rfc7540_SUITE.erl b/test/rfc7540_SUITE.erl index 9fcc08c..2c75fe1 100644 --- a/test/rfc7540_SUITE.erl +++ b/test/rfc7540_SUITE.erl @@ -104,7 +104,7 @@ http_upgrade_ignore_missing_http2_settings_in_connection(Config) -> {ok, <<"HTTP/1.1 200">>} = gen_tcp:recv(Socket, 12, 1000), ok. -http_upgrade_reject_zero_http2_settings_header(Config) -> +http_upgrade_ignore_zero_http2_settings_header(Config) -> doc("The HTTP Upgrade request must include " "exactly one HTTP2-Settings header field (RFC7540 3.2, RFC7540 3.2.1)"), {ok, Socket} = gen_tcp:connect("localhost", config(port, Config), [binary, {active, false}]), @@ -114,7 +114,7 @@ http_upgrade_reject_zero_http2_settings_header(Config) -> "Connection: Upgrade, HTTP2-Settings\r\n" "Upgrade: h2c\r\n" "\r\n"]), - {ok, <<"HTTP/1.1 400">>} = gen_tcp:recv(Socket, 12, 1000), + {ok, <<"HTTP/1.1 200">>} = gen_tcp:recv(Socket, 12, 1000), ok. http_upgrade_reject_two_http2_settings_header(Config) -> @@ -423,19 +423,40 @@ http_upgrade_response_half_closed(Config) -> "HTTP2-Settings: ", base64:encode(cow_http2:settings_payload(#{})), "\r\n", "\r\n"]), ok = do_recv_101(Socket), - %% Send a valid preface. - ok = gen_tcp:send(Socket, ["PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n", cow_http2:settings(#{})]), - %% Send more data on the stream to trigger an error. - ok = gen_tcp:send(Socket, cow_http2:data(1, fin, <<>>)), + %% Send a valid preface followed by an unexpected DATA frame. + ok = gen_tcp:send(Socket, [ + "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n", + cow_http2:settings(#{}), + cow_http2:data(1, fin, <<"Unexpected DATA frame.">>) + ]), %% Receive the server preface. {ok, << Len:24 >>} = gen_tcp:recv(Socket, 3, 1000), {ok, << 4:8, 0:40, _:Len/binary >>} = gen_tcp:recv(Socket, 6 + Len, 1000), - %% Receive the SETTINGS ack. - %% @todo It's possible that we receive the response before the SETTINGS ack or RST_STREAM. - {ok, << 0:24, 4:8, 1:8, 0:32 >>} = gen_tcp:recv(Socket, 9, 1000), - %% The server resets the stream with reason STREAM_CLOSED. - {ok, << 4:24, 3:8, 0:8, 1:32, 5:32 >>} = gen_tcp:recv(Socket, 13, 1000), - ok. + %% Skip the SETTINGS ack, receive the response HEADERS, DATA and RST_STREAM (streamid 1). + Received = lists:reverse(lists:foldl(fun(_, Acc) -> + case gen_tcp:recv(Socket, 9, 1000) of + {ok, << 0:24, 4:8, 1:8, 0:32 >>} -> + Acc; + {ok, << SkipLen:24, 1:8, _:8, 1:32 >>} -> + {ok, _} = gen_tcp:recv(Socket, SkipLen, 1000), + [headers|Acc]; + {ok, << SkipLen:24, 0:8, _:8, 1:32 >>} -> + {ok, _} = gen_tcp:recv(Socket, SkipLen, 1000), + [data|Acc]; + {ok, << 4:24, 3:8, 0:8, 1:32 >>} -> + %% We expect a STREAM_CLOSED reason. + {ok, << 5:32 >>} = gen_tcp:recv(Socket, 4, 1000), + [rst_stream|Acc]; + {error, _} -> + %% Can be timeouts, ignore them. + Acc + end + end, [], [1, 2, 3, 4])), + case Received of + [rst_stream] -> ok; + [headers, rst_stream] -> ok; + [headers, data, rst_stream] -> ok + end. %% Starting HTTP/2 for "https" URIs. |