ninenines / gun

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

Add http2.remote_settings to gun:info map #247

Open bobergj opened 3 years ago

bobergj commented 3 years ago

This adds a new http2 entry to the info map, with a remote_settings property. Depends on: https://github.com/ninenines/cowlib/pull/107

bobergj commented 3 years ago

An example of what this looks like:

27> gun:info(GunPid).
#{cookie_store => undefined,
  http2 =>
      #{remote_settings =>
            #{enable_push => true,header_table_size => 4096,
              initial_window_size => 65535,max_concurrent_streams => 1,
              max_frame_size => 16384,max_header_list_size => 8000}},
  intermediaries => [],
  origin_host => "api.sandbox.push.apple.com",
  origin_port => 443,origin_scheme => <<"https">>,
  owner => <0.1097.0>,protocol => http2,
  sock_ip => {192,168,1,6},
  sock_port => 56214,
  socket =>
      {sslsocket,{gen_tcp,#Port<0.249>,tls_connection,undefined},
                 [<0.1109.0>,<0.1108.0>]},
  transport => tls}
essen commented 3 years ago

Yeah, that's not good though because of the possibility for proxies. HTTP/2 CONNECT support is about to be merged and I'll need to think a little more about how best to handle this considering there might be multiple layers of settings. Also this should not single out HTTP/2 because we will need to do the same for HTTP/3 later.

Please hold until the http2-connect branch is merged this or next week and let's resume the discussion then.

essen commented 3 years ago

Websocket over HTTP/2 will also make knowing settings useful, one way or another. Hold on a little bit more and I'll get back to this.

essen commented 3 years ago

Something like this would be more appropriate:

diff --git a/src/gun.erl b/src/gun.erl
index e441e52..7e77da3 100644
--- a/src/gun.erl
+++ b/src/gun.erl
@@ -464,6 +464,7 @@ info(ServerPid) ->
        origin_host=OriginHost,
        origin_port=OriginPort,
        intermediaries=Intermediaries,
+       protocol_state=ProtoState,
        cookie_store=CookieStore
    }} = sys:get_state(ServerPid),
    Info0 = #{
@@ -497,8 +498,17 @@ info(ServerPid) ->
            end
    end,
    case Protocol of
-       undefined -> Info;
-       _ -> Info#{protocol => Protocol:name()}
+       undefined ->
+           Info;
+       gun_http2 ->
+           Info#{
+               protocol => Protocol:name(),
+               %% @todo In the future we can generalize this to other protocols.
+               %% We can also add this to stream_info when the stream is a tunnel.
+               http2_info => gun_http2:info(ProtoState)
+           };
+       _ ->
+           Info#{protocol => Protocol:name()}
    end.

 %% We change tls_proxy into tls for intermediaries.
diff --git a/src/gun_http2.erl b/src/gun_http2.erl
index 8f0632b..52ac93b 100644
--- a/src/gun_http2.erl
+++ b/src/gun_http2.erl
@@ -33,6 +33,7 @@
 -export([connect/9]).
 -export([cancel/5]).
 -export([timeout/3]).
+-export([info/1]).
 -export([stream_info/2]).
 -export([down/1]).
 -export([ws_upgrade/11]).
@@ -1247,6 +1248,11 @@ timeout(State, {cow_http2_machine, RealStreamRef, Name}, TRef) ->
            {state, State}
    end.

+info(#http2_state{http2_machine=HTTP2Machine}) ->
+   #{
+       remote_settings => cow_http2_machine:get_remote_settings(HTTP2Machine)
+   }.
+
 stream_info(State, StreamRef) when is_reference(StreamRef) ->
    case get_stream_by_ref(State, StreamRef) of
        #stream{reply_to=ReplyTo, tunnel=#tunnel{destination=Destination,

Also this needs a test added to gun_SUITE that checks that http2_info are set, and the http2_info key needs to be added to the gun.info.asciidoc manual page.