async-http icon indicating copy to clipboard operation
async-http copied to clipboard

Allow using custom endpoints for Async::HTTP::Internet

Open binarycode opened this issue 5 years ago • 7 comments

This PR introduces changes that allow passing custom Async::HTTP::Endpoint to Async::HTTP::Internet. This could be useful for example when we need to provide custom SSL options to requests.

binarycode avatar May 20 '20 21:05 binarycode

I agree with your assessment, but expanding the interface in this direction I need a little bit of time to think about it.

ioquatix avatar May 20 '20 23:05 ioquatix

The thing is I want to add support of SSL options (https://github.com/lostisland/faraday/blob/master/lib/faraday/options/ssl_options.rb) to async-http-faraday adapter. I can see 2 approaches to that:

  1. Rewrite the adapter to use lower-level Async::HTTP::Client that allows customizing endpoints. The downside of this approach is that I'll have to basically duplicate the logic of Async::HTTP::Internet in the adapter to allow for persistent connections.
  2. Modify Async::HTTP::Internet to allow passing customized endpoints - that's why I created this PR.

What approach do you think is better? Should I try going with the first one to avoid changing Async::HTTP::Internet API?

binarycode avatar May 21 '20 07:05 binarycode

Can you make a subclass and add the appropriate hooks so you can configure the SSL to your requirements in the faraday adapter?

ioquatix avatar May 21 '20 08:05 ioquatix

I guess that would be almost the same as the first approach, as most of the Async::HTTP::Internet logic is contained in the #call method that I'll need to rewrite to allow endpoint customization. Plus one of the downsides is that any upstream changes in Async::HTTP::Internet could possibly break this subclass and that will not be detected by CI, as the adapter is not pinned to a particular async-http version. Anyway I'll try this approach and make a PR to the adapter gem today.

binarycode avatar May 21 '20 09:05 binarycode

Can we expose the right internal hooks for Async::HTTP::Internet and add specs for them?

Do you imagine all clients would use the same SSL settings? I imagine something like:

internet = MyInternet.new

internet.client_options[hostname] = ssl_options

Or something like that (those names/methods had all of 30 seconds of brain power).

When you have specific SSL state, does it impact all connections or just some specific ones?

ioquatix avatar May 21 '20 09:05 ioquatix

Well if we are talking about the faraday use-case, SSL can be configured only per Faraday::Connection, so all requests made through the adapter wrapped by that connection will use the same SSL settings.

One way this could be configured in Async::HTTP::Internet is to add a new initialize option:

def initialize(**options)
  @clients = {}
  @ssl_context = options.delete(:ssl_context)
  @options = options
end

or a separate method:

attr_writer :ssl_context

And then use that instance variable to build the endpoint:

def call(method, url, headers = [], body = nil)
  endpoint = Endpoint.parse(url, nil, ssl_context: @ssl_context)

Implementing the SSL options per-host will be trickier, as the actual per-host Async::HTTP::Client caching done by the Async::HTTP::Internet is an internal implementation detail.

binarycode avatar May 21 '20 21:05 binarycode

any news on this PR? i was trying to set SSL options today also and found this PR

softbrada avatar Feb 02 '23 23:02 softbrada