Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to integrate the jetty HttpClient with ProxySelector? #12645

Open
iiliev2 opened this issue Dec 16, 2024 · 6 comments
Open

How to integrate the jetty HttpClient with ProxySelector? #12645

iiliev2 opened this issue Dec 16, 2024 · 6 comments
Labels

Comments

@iiliev2
Copy link

iiliev2 commented Dec 16, 2024

Jetty 12 ee8 java 17

Moving the discussion here. Any pointer would be very helpful on how one would do this. For. ex. should we start from ProxyConfiguration.Proxy, to implement a custom one to use in org.eclipse.jetty.client.ProxyConfiguration#addProxy ? Or should the org.eclipse.jetty.client.ProxyConfiguration itself be overridden?

@sbordet
Copy link
Contributor

sbordet commented Dec 16, 2024

As I said, I'm not keen to integrate with ProxySelector, because it only matches on URIs, and we likely need to keep the API more open and match on Origin like ProxyConfiguration does now.

What we can do it is support the system properties.

We can make ProxyConfiguration extends ContainerLifeCycle, override doStart(), and detect if the system properties are set.

If they are, from ProxyConfiguration.doStart() we create and add an HttpProxy.

Lastly, we need to make ProxyConfiguration an HttpClient bean, and the proxies inside ProxyConfiguration beans of ProxyConfiguration itself, so they will show up nicely in HttpClient.dump().

@sbordet
Copy link
Contributor

sbordet commented Dec 16, 2024

To recap:

  • In HttpClient.doStart(), add proxyConfig as a bean.
  • Make ProxyConfiguration extends ContainerLifeCycle.
  • Override ProxyConfiguration.doStart() and:
    • Add the proxies as beans
    • Detect the JDK system properties and possibly add an HttpProxy with the JDK configuration.
    • call super.doStart().

@iiliev2
Copy link
Author

iiliev2 commented Dec 17, 2024

not keen to integrate with ProxySelector, because it only matches on URIs

We do not need to translate the jetty proxy configuration to the one from java.net(ProxySelector and Proxy). We need the other direction. This means, that one can choose to "downgrade" from the full capabilities of the jetty proxy configuration. The fact that it is a downgrade should be obvious, and is up to the user of jetty.
org.eclipse.jetty.client.ProxyConfiguration#match is roughly equivalent to java.net.ProxySelector#select and org.eclipse.jetty.client.ProxyConfiguration.Proxy to java.net.Proxy. Jetty seems to also support the other protocols, not just HTTP, so AFAICT there just needs to be some sort of adapter code to convert the java proxies to jetty proxies. The adapter needs to take into account that the ProxySelector selects multiple proxies, which are then tried in order until a successful connection is established. This means that the adapter would probably need an additional strategy to resolve such an ambiguity as jetty only matches(selects) one proxy.

Those properties are implementation specific to the default sun.net.spi.DefaultProxySelector. Integrating just with that specifically seems counterproductive to me. Why handicap jetty like that when there seems to be a fairly straightforward mapping between jetty and java proxies?

@sbordet
Copy link
Contributor

sbordet commented Dec 17, 2024

Why are you even using JDK's ProxySelector if you're using Jetty's HttpClient?

As I said, we can support the system properties, but there is no point in trying to use ProxySelector with Jetty's HttpClient: just use the Jetty HttpClient proxy APIs.

If I miss something, then I need a clear use case of why we should support ProxySelector by wrapping it to look like a Jetty HttpProxy, when you can use Jetty's HttpProxy directly.

@iiliev2
Copy link
Author

iiliev2 commented Dec 18, 2024

The application has many modules. The central proxy abstraction is the JDK one. Only in one area(module) we are using the jetty client. That is why we need such a bridge.

The adapter needs to take into account that the ProxySelector selects multiple proxies

This bit is the most unclear to me. If the semantic of the ProxySelector that is in use, returns multiple proxies for an Origin/URI with the intention that each should be tried until one succeeds, I am not sure how that will fit with the jetty client. If the adapter is only working on the ProxyConfiguration level, what would drive org.eclipse.jetty.client.HttpClient#send to try to make connections in succession like that.

@sbordet
Copy link
Contributor

sbordet commented Dec 18, 2024

We don't support multiple proxies for the same origin.

Considering that each proxy may speak a different protocol, possibly over a different transport, we would need to differentiate destinations based on the proxy too, which adds one more layer of complexity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

No branches or pull requests

2 participants