ninenines / gun

HTTP/1.1, HTTP/2, Websocket client (and more) for Erlang/OTP.
ISC License
891 stars 232 forks source link

gun:connect expects proxy server to reply with HTTP/1.1, some servers respond with HTTP/1.0 #303

Closed JesseStimpson closed 1 year ago

JesseStimpson commented 1 year ago

Hello,

I'm attempting to connect to an HTTP proxy that is running goproxy (the project is from Stripe, but I am not affiliated with them) and encountering a failure in HTTP version incompatibility. Here is my client code

fun() ->
    application:ensure_all_started(gun),
    {ok, ConnPid} = gun:open("my-https-proxy.example.com", 443,
        #{protocols => [http], transport => tls, tls_opts => [{verify, verify_none}]}),
    {ok, http} = gun:await_up(ConnPid),
    ConnectRef = gun:connect(ConnPid,
        #{host => "www.google.com",
          port => 443,
          protocols => [http],
          transport => tls,
          tls_opts => [{verify, verify_none}]
          }),
    {response, fin, 200, _} = gun:await(ConnPid, ConnectRef),
    GetRef = gun:get(ConnPid, "/", #{}, #{tunnel => ConnectRef}),
    Result = gun:await(ConnPid, GetRef),
    gun:close(Result),
    Result
end().

This results in the following error

=CRASH REPORT==== 9-Nov-2022::21:42:16.365258 ===
  crasher:
    initial call: gun:init/1
    pid: <0.196.0>
    registered_name: []
    exception error: no function clause matching
                     gun_http:handle_connect(<<>>,
                                             {http_state,
                                              {sslsocket,
                                               {gen_tcp,#Port<0.16>,
                                                tls_connection,undefined},
                                               [<0.199.0>,<0.198.0>]},
                                              gun_tls,#{},'HTTP/1.1',
                                              keepalive,<<>>,undefined,
                                              [{stream,
                                                {connect,
                                                 #Ref<0.3830098819.398983169.252231>,
                                                 #{host => "www.google.com",
                                                   port => 443,
                                                   protocols => [http],
                                                   tls_opts =>
                                                    [{verify,verify_none}],
                                                   transport => tls}},
                                                <0.187.0>,infinity,
                                                <<"CONNECT">>,
                                                ["www.google.com",58,
                                                 <<"443">>],
                                                <<>>,true,undefined}],
                                              head,
                                              {0,0},
                                              head},
                                             undefined,gun_default_event_h,
                                             undefined,'HTTP/1.0',200,[]) (/home/jstimpson/gun/src/gun_http.erl, line 311)
      in function  gun:handle_common_connected_no_input/4 (/home/jstimpson/gun/src/gun.erl, line 1408)
      in call from gen_statem:loop_state_callback/11 (gen_statem.erl, line 1203)
    ancestors: [gun_conns_sup,gun_sup,<0.161.0>]
    message_queue_len: 0
    messages: []
    links: [<0.163.0>]
    dictionary: []
    trap_exit: false
    status: running
    heap_size: 17731
    stack_size: 29
    reductions: 38687
  neighbours:

The function gun_http:handle_connect/8 is expecting 'HTTP/1.1', but my proxy is returning HTTP/1.0, leading to the crash.

For reference, here's a link to where goproxy is specifying 1.0 .

I'm not familiar enough with the HTTP spec to know if goproxy is serving the wrong version or not, but if I change gun_http:handle_connect/8 to allow 'HTTP/1.0', my request works as expected, so I wanted to check in here.

I can submit a PR if you'd like.

Thanks!

essen commented 1 year ago

Yes they're wrong as far as the spec goes, though I'm sure they have their reasons. This came up before I believe (though not necessarily with this proxy) and the conclusion was that it would be OK to accept HTTP/1.0 but that it would have to be disabled by default and enabled through an option. Best place I believe is in the http_opts of the original connection, something like allow_http10_connect or a better name.

essen commented 1 year ago

Right I think I remember that it's historical, initially it was done over HTTP/1.0 long ago and then it was standardized in HTTP/1.1. New proxies should definitely not use HTTP/1.0 though.

essen commented 1 year ago

Fixed, thanks!