jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.87k stars 1.91k forks source link

Why there is no Regex Support for excludeList of nonProxyHost #8329

Open kasingal opened 2 years ago

kasingal commented 2 years ago

Code at line below should be enhanced: https://github.com/eclipse/jetty.project/blob/8102378e2d24b8117ed3a545c7579f79d35fc45a/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyConfiguration.java#L180

Generally nonProxyHosts gets defined with wildcards like *.abc.com|*.svc.local etc.. so .equals doesn't help ! And apparently Jetty Client doesn't have any support for wildcard list of nonProxyHosts !!

PS: reactor-netty handles it pretty well, refer below: https://github.com/reactor/reactor-netty/blob/main/reactor-netty-core/src/main/java/reactor/netty/transport/ProxyProvider.java#L310

joakime commented 2 years ago

What version of Jetty?

kasingal commented 2 years ago

Any version till date.. there is no support for the pattern match there on the line I just tagged to the ticket ! Better to have it enhanced in 9.4.x and higher versions !

sbordet commented 2 years ago

@kasingal we accept pull requests. Do you want to try to fix this issue and submit a pull request? Thanks!

joakime commented 2 years ago

If you decide to do a pull request, make it against branch jetty-10.0.x only, we'll handle the merging to jetty-11.0.x and jetty-12.0.x ourselves.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has been a full year without activity. It will be closed if no further activity occurs. Thank you for your contributions.

sugilite commented 1 year ago

I would like to work on this issue. Is targeting jetty-10.x still the way to go about it? Thanks

sbordet commented 1 year ago

Yes, please target branch jetty-10.0.x. Thanks!

sugilite commented 1 year ago

Hi - I opened PR #10538. First contribution to this project - feedback is always welcome. Currently only allows for wildcards at the start and end of the host. This matches the behavior of the jdk default via nonProxyHosts, see:

The patterns may start or end with a '*' for wildcards. (https://docs.oracle.com/javase/8/docs/technotes/guides/net/proxies.html)

I think this satisfies most use cases.

If we (you) also want to support wildcards in the middle maybe something like this works better:

      String hostRegex = Arrays.stream(host.split("\\*"))
        .map(s -> {
            if (s.length() > 0) {
                return Pattern.quote(s);
            }
            return s;
        })
        .collect(Collectors.joining(".*"));
gregw commented 3 months ago

See #10538 for an attempt on doing this for jetty-12. Any effort to do thiis for jetty-12.1 should use that as the basis, so that @sugilite gets some credit.