Issue #8329 - support wildcards in proxy exclusion hosts #10538
Conversation
Signed-off-by: Lukas Gisi <lukasgisi@gmail.com>
| HostPort hostPort = new HostPort(pattern); | ||
| String host = hostPort.getHost(); | ||
| int port = hostPort.getPort(); | ||
| String hostRegex = extractHostRegex(host); | ||
| return Pattern.matches(hostRegex, address.getHost()) && (port <= 0 || port == address.getPort()); |
There was a problem hiding this comment.
This is getting to be an expensive check to do on every request, as it will be parsing the HostPort and compiling the regex itself. Both of these operations should be able to be done in advance rather than on every call to matches.
This really should be a specialized Set for use with IncludeExcludeSet. See InetAccessHandler for an example (albeit not a great one) of how this should be done.
There was a problem hiding this comment.
Thanks for your feedback and your hints. I checked out InetAccessHandler and InetAddressSet.
Can we sensibly achieve this without breaking the current API?
Some thoughts:
At the moment clients use the HashSet<String> returned by getExcludedAddresses() directly and add (or remove) proxy exclusions by directly adding Strings to this set.
I briefly thought about instead of returning a HashSet in getExcludedAddresses we return our own class (extension of HashSet) which proxies all the add/remove operations and holds an internal set with all the Patterns (or Matchers) - this obviously has it's own problems and is most likely a pretty bad approach.
For now I added two commits: First commit which adds a HashMap to cache the Patterns for a given regex string, second one to do the same for the HostPorts.
This is not perfect at all: these maps only grow and never shrink. But I also don't think (hope) that people use thousands of included and excluded addresses. (If we would use a local HashMap as a cache in my day to day job we would use guave CacheBuilder or the newer caffeine which lets us set a max size or expiration time for entries - are there some helpers in jetty which I can use here?)
I need some more time to figure out how to best use IncludeExcludeSet here.
There was a problem hiding this comment.
@sugilite I'll have a think too about how we can avoid breaking the current API too badly. For now I've moved this to our next release cycle for consideration
…r same hosts Signed-off-by: Lukas Gisi <lukasgisi@gmail.com>
… patterns Signed-off-by: Lukas Gisi <lukasgisi@gmail.com>
There was a problem hiding this comment.
This is better, but:
- it still does not use the IncludeExclude mechanism
- It allows wild cards on exclude but not include
- It should use the existing InetAddressPattern class (which may need to be enhanced?)
Ultimately it should be really similar to InetAccessHandler.
@joakime do you have time to help @sugilite get this PR ready this cycle?
Proxy configuration: Allow for "*" wildcard at the start and the end of excluded hosts.