From 37bf8c409ae341fdebdc062a33cd7fce7ac1f5b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Sat, 3 Oct 2020 19:03:22 +0200 Subject: Fix test suites failing due to previous breaking changes --- src/gun.erl | 24 ++++++++++++++---------- src/gun_http.erl | 44 ++++++++++++-------------------------------- test/rfc7231_SUITE.erl | 29 ++++++++++------------------- test/socks_SUITE.erl | 4 ++-- 4 files changed, 38 insertions(+), 63 deletions(-) diff --git a/src/gun.erl b/src/gun.erl index 4a0df0a..839f665 100644 --- a/src/gun.erl +++ b/src/gun.erl @@ -1494,16 +1494,20 @@ handle_common_connected_no_input({call, From}, {stream_info, StreamRef}, _, tunnel => Tunnel }}; true -> - {ok, Info0} = Protocol:stream_info(ProtoState, dereference_stream_ref(StreamRef, State)), - Info = Info0#{ref => StreamRef}, - case Intermediaries0 of - [] -> - {ok, Info}; - _ -> - Tail = maps:get(intermediaries, Info, []), - {ok, Info#{ - intermediaries => intermediaries_info(Intermediaries0, []) ++ Tail - }} + case Protocol:stream_info(ProtoState, dereference_stream_ref(StreamRef, State)) of + {ok, undefined} -> + {ok, undefined}; + {ok, Info0} -> + Info = Info0#{ref => StreamRef}, + case Intermediaries0 of + [] -> + {ok, Info}; + _ -> + Tail = maps:get(intermediaries, Info, []), + {ok, Info#{ + intermediaries => intermediaries_info(Intermediaries0, []) ++ Tail + }} + end end end }}; diff --git a/src/gun_http.erl b/src/gun_http.erl index d2a758b..e2c0f1d 100644 --- a/src/gun_http.erl +++ b/src/gun_http.erl @@ -540,13 +540,11 @@ keepalive(State=#http_state{socket=Socket, transport=Transport, out=head}, _, Ev keepalive(State, _, EvHandlerState) -> {State, EvHandlerState}. -headers(State, StreamRef, ReplyTo, Method, Host, Port, - Path, Headers, InitialFlow, EvHandler, EvHandlerState) +headers(State, StreamRef, ReplyTo, _, _, _, _, _, _, _, EvHandlerState) when is_list(StreamRef) -> - %% Because we switch protocol we may receive a StreamRef as a list. - %% But we are always the final StreamRef as HTTP/1.1. - headers(State, lists:last(StreamRef), ReplyTo, Method, Host, Port, - Path, Headers, InitialFlow, EvHandler, EvHandlerState); + ReplyTo ! {gun_error, self(), stream_ref(State, StreamRef), + {badstate, "The stream is not a tunnel."}}, + {State, EvHandlerState}; headers(State=#http_state{opts=Opts, out=head}, StreamRef, ReplyTo, Method, Host, Port, Path, Headers, InitialFlow0, EvHandler, EvHandlerState0) -> @@ -557,14 +555,11 @@ headers(State=#http_state{opts=Opts, out=head}, {new_stream(State#http_state{connection=Conn, out=Out}, StreamRef, ReplyTo, Method, Authority, Path, InitialFlow), EvHandlerState}. -%% @todo I don't think this clause is hit anymore. Same in other related callbacks. -request(State, StreamRef, ReplyTo, Method, Host, Port, - Path, Headers, Body, InitialFlow, EvHandler, EvHandlerState) +request(State, StreamRef, ReplyTo, _, _, _, _, _, _, _, _, EvHandlerState) when is_list(StreamRef) -> - %% Because we switch protocol we may receive a StreamRef as a list. - %% But we are always the final StreamRef as HTTP/1.1. - request(State, lists:last(StreamRef), ReplyTo, Method, Host, Port, - Path, Headers, Body, InitialFlow, EvHandler, EvHandlerState); + ReplyTo ! {gun_error, self(), stream_ref(State, StreamRef), + {badstate, "The stream is not a tunnel."}}, + {State, EvHandlerState}; request(State=#http_state{opts=Opts, out=head}, StreamRef, ReplyTo, Method, Host, Port, Path, Headers, Body, InitialFlow0, EvHandler, EvHandlerState0) -> @@ -652,11 +647,6 @@ transform_header_names(#http_state{opts=Opts}, Headers) -> Fun -> lists:keymap(Fun, 1, Headers) end. -data(State, StreamRef, ReplyTo, IsFin, Data, EvHandler, EvHandlerState) - when is_list(StreamRef) -> - %% Because we switch protocol we may receive a StreamRef as a list. - %% But we are always the final StreamRef as HTTP/1.1. - data(State, lists:last(StreamRef), ReplyTo, IsFin, Data, EvHandler, EvHandlerState); %% We are expecting a new stream. data(State=#http_state{out=head}, StreamRef, ReplyTo, _, _, _, EvHandlerState) -> {error_stream_closed(State, StreamRef, ReplyTo), EvHandlerState}; @@ -712,11 +702,11 @@ data(State=#http_state{socket=Socket, transport=Transport, version=Version, {error_stream_not_found(State, StreamRef, ReplyTo), EvHandlerState0} end. -connect(State, StreamRef, ReplyTo, Destination, TunnelInfo, Headers, InitialFlow) +connect(State, StreamRef, ReplyTo, _, _, _, _) when is_list(StreamRef) -> - %% Because we switch protocol we may receive a StreamRef as a list. - %% But we are always the final StreamRef as HTTP/1.1. - connect(State, lists:last(StreamRef), ReplyTo, Destination, TunnelInfo, Headers, InitialFlow); + ReplyTo ! {gun_error, self(), stream_ref(State, StreamRef), + {badstate, "The stream is not a tunnel."}}, + State; connect(State=#http_state{streams=Streams}, StreamRef, ReplyTo, _, _, _, _) when Streams =/= [] -> ReplyTo ! {gun_error, self(), stream_ref(State, StreamRef), {badstate, "CONNECT can only be used with HTTP/1.1 when no other streams are active."}}, @@ -753,11 +743,6 @@ connect(State=#http_state{socket=Socket, transport=Transport, opts=Opts, version new_stream(State, {connect, StreamRef, Destination}, ReplyTo, <<"CONNECT">>, Authority, <<>>, InitialFlow). -cancel(State, StreamRef, ReplyTo, EvHandler, EvHandlerState) - when is_list(StreamRef) -> - %% Because we switch protocol we may receive a StreamRef as a list. - %% But we are always the final StreamRef as HTTP/1.1. - cancel(State, lists:last(StreamRef), ReplyTo, EvHandler, EvHandlerState); %% We can't cancel anything, we can just stop forwarding messages to the owner. cancel(State0, StreamRef, ReplyTo, EvHandler, EvHandlerState0) -> case is_stream(State0, StreamRef) of @@ -774,11 +759,6 @@ cancel(State0, StreamRef, ReplyTo, EvHandler, EvHandlerState0) -> {error_stream_not_found(State0, StreamRef, ReplyTo), EvHandlerState0} end. -stream_info(State, StreamRef) - when is_list(StreamRef) -> - %% Because we switch protocol we may receive a StreamRef as a list. - %% But we are always the final StreamRef as HTTP/1.1. - stream_info(State, lists:last(StreamRef)); stream_info(#http_state{streams=Streams}, StreamRef) -> case lists:keyfind(StreamRef, #stream.ref, Streams) of #stream{reply_to=ReplyTo, is_alive=IsAlive} -> diff --git a/test/rfc7231_SUITE.erl b/test/rfc7231_SUITE.erl index 5ef37bd..b1941c6 100644 --- a/test/rfc7231_SUITE.erl +++ b/test/rfc7231_SUITE.erl @@ -95,16 +95,6 @@ do_proxy_init(Parent, Transport, Status, ConnectRespHeaders, Delay) -> inet:setopts(OriginSocket, [{active, true}]), do_proxy_loop(Transport, ClientSocket, OriginSocket); true -> - %% We send a 501 to the subsequent request. - {ok, _} = case Transport of - gun_tcp -> - gen_tcp:recv(ClientSocket, 0, 1000); - gun_tls -> - ssl:recv(ClientSocket, 0, 1000) - end, - ok = Transport:send(ClientSocket, << - "HTTP/1.1 501 Not Implemented\r\n" - "content-length: 0\r\n\r\n">>), timer:sleep(2000) end. @@ -171,7 +161,7 @@ do_connect_http(OriginScheme, OriginTransport, ProxyTransport) -> %% @todo Do we still need these handshake_completed messages? handshake_completed = receive_from(OriginPid), {up, http} = gun:await(ConnPid, StreamRef), - _ = gun:get(ConnPid, "/proxied"), + _ = gun:get(ConnPid, "/proxied", [], #{tunnel => StreamRef}), Data = receive_from(OriginPid), Lines = binary:split(Data, <<"\r\n">>, [global]), [<<"host: ", Authority/bits>>] = [L || <<"host: ", _/bits>> = L <- Lines], @@ -226,7 +216,7 @@ do_connect_h2(OriginScheme, OriginTransport, ProxyTransport) -> {response, fin, 200, _} = gun:await(ConnPid, StreamRef), handshake_completed = receive_from(OriginPid), {up, http2} = gun:await(ConnPid, StreamRef), - _ = gun:get(ConnPid, "/proxied"), + _ = gun:get(ConnPid, "/proxied", [], #{tunnel => StreamRef}), <<_:24, 1:8, _/bits>> = receive_from(OriginPid), #{ transport := OriginTransport, @@ -282,7 +272,7 @@ do_connect_through_multiple_proxies(OriginScheme, OriginTransport, ProxiesTransp {response, fin, 200, _} = gun:await(ConnPid, StreamRef2), handshake_completed = receive_from(OriginPid), {up, http} = gun:await(ConnPid, StreamRef2), - _ = gun:get(ConnPid, "/proxied"), + _ = gun:get(ConnPid, "/proxied", [], #{tunnel => StreamRef2}), Data = receive_from(OriginPid), Lines = binary:split(Data, <<"\r\n">>, [global]), [<<"host: ", Authority2/bits>>] = [L || <<"host: ", _/bits>> = L <- Lines], @@ -323,7 +313,7 @@ connect_delay(_) -> {response, fin, 201, _} = gun:await(ConnPid, StreamRef), handshake_completed = receive_from(OriginPid), {up, http} = gun:await(ConnPid, StreamRef), - _ = gun:get(ConnPid, "/proxied"), + _ = gun:get(ConnPid, "/proxied", [], #{tunnel => StreamRef}), Data = receive_from(OriginPid), Lines = binary:split(Data, <<"\r\n">>, [global]), [<<"host: ", Authority/bits>>] = [L || <<"host: ", _/bits>> = L <- Lines], @@ -358,7 +348,7 @@ connect_response_201(_) -> {response, fin, 201, _} = gun:await(ConnPid, StreamRef), handshake_completed = receive_from(OriginPid), {up, http} = gun:await(ConnPid, StreamRef), - _ = gun:get(ConnPid, "/proxied"), + _ = gun:get(ConnPid, "/proxied", [], #{tunnel => StreamRef}), Data = receive_from(OriginPid), Lines = binary:split(Data, <<"\r\n">>, [global]), [<<"host: ", Authority/bits>>] = [L || <<"host: ", _/bits>> = L <- Lines], @@ -405,8 +395,9 @@ do_connect_failure(Status) -> }), {request, <<"CONNECT">>, Authority, 'HTTP/1.1', _} = receive_from(ProxyPid), {response, fin, Status, Headers} = gun:await(ConnPid, StreamRef), - FailedStreamRef = gun:get(ConnPid, "/proxied"), - {response, fin, 501, _} = gun:await(ConnPid, FailedStreamRef), + %% We cannot do a request because the StreamRef is not a tunnel. + FailedStreamRef = gun:get(ConnPid, "/proxied", [], #{tunnel => StreamRef}), + {error, {stream_error, {badstate, _}}} = gun:await(ConnPid, FailedStreamRef), #{ transport := tcp, protocol := http, @@ -500,7 +491,7 @@ connect_response_ignore_transfer_encoding(_) -> {response, fin, 200, Headers} = gun:await(ConnPid, StreamRef), handshake_completed = receive_from(OriginPid), {up, http} = gun:await(ConnPid, StreamRef), - _ = gun:get(ConnPid, "/proxied"), + _ = gun:get(ConnPid, "/proxied", [], #{tunnel => StreamRef}), Data = receive_from(OriginPid), Lines = binary:split(Data, <<"\r\n">>, [global]), [<<"host: ", Authority/bits>>] = [L || <<"host: ", _/bits>> = L <- Lines], @@ -523,7 +514,7 @@ connect_response_ignore_content_length(_) -> {response, fin, 200, Headers} = gun:await(ConnPid, StreamRef), handshake_completed = receive_from(OriginPid), {up, http} = gun:await(ConnPid, StreamRef), - _ = gun:get(ConnPid, "/proxied"), + _ = gun:get(ConnPid, "/proxied", [], #{tunnel => StreamRef}), Data = receive_from(OriginPid), Lines = binary:split(Data, <<"\r\n">>, [global]), [<<"host: ", Authority/bits>>] = [L || <<"host: ", _/bits>> = L <- Lines], diff --git a/test/socks_SUITE.erl b/test/socks_SUITE.erl index f912256..f6686a3 100644 --- a/test/socks_SUITE.erl +++ b/test/socks_SUITE.erl @@ -390,7 +390,7 @@ do_socks5_through_connect_proxy(OriginScheme, OriginTransport, ProxyTransport) - {connect, <<"localhost">>, OriginPort} = receive_from(Proxy2Pid), handshake_completed = receive_from(OriginPid), Authority2 = iolist_to_binary(["localhost:", integer_to_binary(OriginPort)]), - _ = gun:get(ConnPid, "/proxied"), + _ = gun:get(ConnPid, "/proxied", [], #{tunnel => StreamRef}), Data = receive_from(OriginPid), Lines = binary:split(Data, <<"\r\n">>, [global]), [<<"host: ", Authority2/bits>>] = [L || <<"host: ", _/bits>> = L <- Lines], @@ -476,7 +476,7 @@ do_socks5_through_h2_connect_proxy(_OriginScheme, OriginTransport, ProxyScheme, state := running, tunnel := #{ transport := ProxyTransport, - protocol := http, + protocol := socks, %% @todo They're not necessarily the origin. Should be named scheme/host/port. origin_scheme := ProxyScheme, origin_host := "localhost", -- cgit v1.2.3