aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLoïc Hoguin <[email protected]>2017-07-26 17:31:12 +0200
committerLoïc Hoguin <[email protected]>2017-07-26 17:36:41 +0200
commit4fa7aeb0fd2e68fba267b33538685a7e1e18f4aa (patch)
tree4d42118c4c1136782ea2e7cabc7a20ffd2e8dfa2
parent238ac3afc661046fb554a8bbb62d2b9912023247 (diff)
downloadcowboy-4fa7aeb0fd2e68fba267b33538685a7e1e18f4aa.tar.gz
cowboy-4fa7aeb0fd2e68fba267b33538685a7e1e18f4aa.tar.bz2
cowboy-4fa7aeb0fd2e68fba267b33538685a7e1e18f4aa.zip
Fix HTTP/2 server push
Cowboy was encoding the headers then decoding them when initializing the request. The problem is that the encoding and decoding contexts are not the same. Now, Cowboy will directly use the headers it received in the push command for the new request. This is also more efficient. I am surprised it worked at all considering the issue.
-rw-r--r--src/cowboy_http2.erl118
1 files changed, 60 insertions, 58 deletions
diff --git a/src/cowboy_http2.erl b/src/cowboy_http2.erl
index 6967bb0..23b75eb 100644
--- a/src/cowboy_http2.erl
+++ b/src/cowboy_http2.erl
@@ -332,7 +332,7 @@ frame(State=#state{client_streamid=LastStreamID}, {headers, StreamID, _, _, _})
%% 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.
- stream_init(State, StreamID, IsFin, HeaderBlock);
+ stream_decode_init(State, StreamID, IsFin, HeaderBlock);
%% HEADERS frame starting a headers block. Enter continuation mode.
frame(State, {headers, StreamID, IsFin, head_nofin, HeaderBlockFragment}) ->
State#state{parse_state={continuation, StreamID, IsFin, HeaderBlockFragment}};
@@ -340,7 +340,7 @@ frame(State, {headers, StreamID, IsFin, head_nofin, HeaderBlockFragment}) ->
frame(State, {headers, StreamID, IsFin, head_fin,
_IsExclusive, _DepStreamID, _Weight, HeaderBlock}) ->
%% @todo Handle priority.
- stream_init(State, StreamID, IsFin, HeaderBlock);
+ stream_decode_init(State, StreamID, IsFin, HeaderBlock);
%% HEADERS frame starting a headers block. Enter continuation mode.
frame(State, {headers, StreamID, IsFin, head_nofin,
_IsExclusive, _DepStreamID, _Weight, HeaderBlockFragment}) ->
@@ -411,7 +411,7 @@ frame(State, {continuation, _, _, _}) ->
continuation_frame(State=#state{parse_state={continuation, StreamID, IsFin, HeaderBlockFragment0}},
{continuation, StreamID, head_fin, HeaderBlockFragment1}) ->
- stream_init(State#state{parse_state=normal}, StreamID, IsFin,
+ stream_decode_init(State#state{parse_state=normal}, StreamID, IsFin,
<< HeaderBlockFragment0/binary, HeaderBlockFragment1/binary >>);
continuation_frame(State=#state{parse_state={continuation, StreamID, IsFin, HeaderBlockFragment0}},
{continuation, StreamID, head_nofin, HeaderBlockFragment1}) ->
@@ -529,21 +529,24 @@ commands(State0=#state{socket=Socket, transport=Transport, server_streamid=Promi
Authority = case {Scheme, Port} of
{<<"http">>, 80} -> Host;
{<<"https">>, 443} -> Host;
- _ -> [Host, $:, integer_to_binary(Port)]
+ _ -> iolist_to_binary([Host, $:, integer_to_binary(Port)])
end,
PathWithQs = case Qs of
<<>> -> Path;
_ -> [Path, $?, Qs]
end,
- Headers = Headers0#{<<":method">> => Method,
- <<":scheme">> => Scheme,
- <<":authority">> => Authority,
- <<":path">> => PathWithQs},
+ %% We need to make sure the header value is binary before we can
+ %% pass it to stream_req_init, as it expects them to be flat.
+ Headers1 = maps:map(fun(_, V) -> iolist_to_binary(V) end, Headers0),
+ Headers = Headers1#{
+ <<":method">> => Method,
+ <<":scheme">> => Scheme,
+ <<":authority">> => Authority,
+ <<":path">> => iolist_to_binary(PathWithQs)},
{HeaderBlock, EncodeState} = headers_encode(Headers, EncodeState0),
Transport:send(Socket, cow_http2:push_promise(StreamID, PromisedStreamID, HeaderBlock)),
- %% @todo iolist_to_binary(HeaderBlock) isn't optimal. Need a shortcut.
- State = stream_init(State0#state{server_streamid=PromisedStreamID + 2, encode_state=EncodeState},
- PromisedStreamID, fin, iolist_to_binary(HeaderBlock)),
+ State = stream_req_init(State0#state{server_streamid=PromisedStreamID + 2,
+ encode_state=EncodeState}, PromisedStreamID, fin, Headers),
commands(State, Stream, Tail);
commands(State=#state{socket=Socket, transport=Transport, remote_window=ConnWindow},
Stream=#stream{id=StreamID, remote_window=StreamWindow},
@@ -699,61 +702,60 @@ terminate_all_streams([#stream{id=StreamID, state=StreamState}|Tail], Reason, Ch
%% Stream functions.
-stream_init(State0=#state{ref=Ref, socket=Socket, transport=Transport, peer=Peer, decode_state=DecodeState0},
- StreamID, IsFin, HeaderBlock) ->
+stream_decode_init(State=#state{socket=Socket, transport=Transport,
+ decode_state=DecodeState0}, StreamID, IsFin, HeaderBlock) ->
%% @todo Add clause for CONNECT requests (no scheme/path).
try headers_decode(HeaderBlock, DecodeState0) of
- {Headers0=#{
- <<":method">> := Method,
- <<":scheme">> := Scheme,
- <<":authority">> := Authority,
- <<":path">> := PathWithQs}, DecodeState} ->
- State = State0#state{decode_state=DecodeState},
- Headers = maps:without([<<":method">>, <<":scheme">>, <<":authority">>, <<":path">>], Headers0),
- BodyLength = case Headers of
- _ when IsFin =:= fin ->
- 0;
- #{<<"content-length">> := <<"0">>} ->
- 0;
- #{<<"content-length">> := BinLength} ->
- Length = try
- cow_http_hd:parse_content_length(BinLength)
- catch _:_ ->
- terminate(State0, {stream_error, StreamID, protocol_error,
- 'The content-length header is invalid. (RFC7230 3.3.2)'})
- %% @todo Err should terminate here...
- end,
- Length;
- _ ->
- undefined
- end,
- {Host, Port} = cow_http_hd:parse_host(Authority),
- {Path, Qs} = cow_http:parse_fullpath(PathWithQs),
- Req = #{
- ref => Ref,
- pid => self(),
- streamid => StreamID,
- peer => Peer,
- method => Method,
- scheme => Scheme,
- host => Host,
- port => Port,
- path => Path,
- qs => Qs,
- version => 'HTTP/2',
- headers => Headers,
- has_body => IsFin =:= nofin,
- body_length => BodyLength
- },
- stream_handler_init(State, StreamID, IsFin, idle, Req);
+ {Headers=#{<<":method">> := _, <<":scheme">> := _,
+ <<":authority">> := _, <<":path">> := _}, DecodeState} ->
+ stream_req_init(State#state{decode_state=DecodeState}, StreamID, IsFin, Headers);
{_, DecodeState} ->
Transport:send(Socket, cow_http2:rst_stream(StreamID, protocol_error)),
- State0#state{decode_state=DecodeState}
+ State#state{decode_state=DecodeState}
catch _:_ ->
- terminate(State0, {connection_error, compression_error,
+ terminate(State, {connection_error, compression_error,
'Error while trying to decode HPACK-encoded header block. (RFC7540 4.3)'})
end.
+stream_req_init(State=#state{ref=Ref, peer=Peer}, StreamID, IsFin, Headers0=#{
+ <<":method">> := Method, <<":scheme">> := Scheme,
+ <<":authority">> := Authority, <<":path">> := PathWithQs}) ->
+ Headers = maps:without([<<":method">>, <<":scheme">>, <<":authority">>, <<":path">>], Headers0),
+ BodyLength = case Headers of
+ _ when IsFin =:= fin ->
+ 0;
+ #{<<"content-length">> := <<"0">>} ->
+ 0;
+ #{<<"content-length">> := BinLength} ->
+ try
+ cow_http_hd:parse_content_length(BinLength)
+ catch _:_ ->
+ terminate(State, {stream_error, StreamID, protocol_error,
+ 'The content-length header is invalid. (RFC7230 3.3.2)'})
+ end;
+ _ ->
+ undefined
+ end,
+ {Host, Port} = cow_http_hd:parse_host(Authority),
+ {Path, Qs} = cow_http:parse_fullpath(PathWithQs),
+ Req = #{
+ ref => Ref,
+ pid => self(),
+ streamid => StreamID,
+ peer => Peer,
+ method => Method,
+ scheme => Scheme,
+ host => Host,
+ port => Port,
+ path => Path,
+ qs => Qs,
+ version => 'HTTP/2',
+ headers => Headers,
+ has_body => IsFin =:= nofin,
+ body_length => BodyLength
+ },
+ stream_handler_init(State, StreamID, IsFin, idle, Req).
+
stream_handler_init(State=#state{opts=Opts,
local_settings=#{initial_window_size := RemoteWindow},
remote_settings=#{initial_window_size := LocalWindow}},