search

ostinelli / net-http2

NetHttp2 is an HTTP/2 client for Ruby.
MIT License
140 stars 31 forks source link

Proxy support #11

Closed nattfodd closed 7 years ago

nattfodd commented 7 years ago

Note #1 Majority of proxies do not explicitly support HTTP/2 protocol, while they successfully create tcp tunnel, so we keep HTTP/1.1 in a CONNECT request

Note #2 Proxy may return any response code starting with 2 (2xx) as a success response, not just 200.

nattfodd commented 7 years ago

Any ideas why does ruby 2.2 case fails on TravisCI? That's pretty weird

ostinelli commented 7 years ago

Thank you for this input.

Any ideas why does ruby 2.2 case fails on TravisCI? That's pretty weird

Do not worry about that. Those are integration tests and running in Travis sometimes the releasing of sockets can take too much for the next test to pass. I've restarted the job.

A quick couple of notes:

  1. Do not bump version. This is not something chosen in a PR.
  2. I'm not particularly convinced of the "hack" of opening a HTTP 1.1 connection on proxies and then using the underlying socket for a HTTP 2 connection. Is there a reason why you feel confident on this?
  3. Is there any way we can do a proxy integration tests instead of the current mocking?
nattfodd commented 7 years ago
  1. Removed.
  2. The reason I've ended up with replacing HTTP/2 with HTTP/1.1 for CONNECT directive is that in fact this version variable isn't relevant, Proxy server just may or may not support such version title, while all we need to bypass HTTP/2 data is to ask ProxyServer to open TCP tunnel for us. After that, we 'talk' with a remote desired server as usual, it does not matter if it is HTTP1.1 or HTTP2 protocol, it's just a tunnel that passes data through. 90% of all proxy servers do not support a HTTP/2 connection header yet, including popular server apps like squid, so I think It makes sense to keep 1.1 version here for proxy for a while.
  3. Otherwise, we have to write dummy-proxy server (like we have Dummy Http server here) which is somewhat much more complicated than the proxy changes itself. The change itself is about sending request to the proxy with a proper addresses with an already tested TCP socket, so we check the request string gem builds. Sure, it does not cover the case for 100% like integration test would do, but it tests the most crucial part.
ostinelli commented 7 years ago

Ok. Merged in, thank you.