rschmitt commented on PR #773: URL: https://github.com/apache/httpcomponents-client/pull/773#issuecomment-3788335849
Returning to the subject at hand, here are our findings so far: 1. Trust store system properties are always respected, irrespective of `useSystemProperties`. 2. Key store system properties are ignored by default, but it is safe to enable them. 3. Proxy configuration is disabled by default, resulting in the locus of control issues described in [HTTPCLIENT-2381](https://issues.apache.org/jira/browse/HTTPCLIENT-2381). 4. `proxyUser` and `proxyPassword` are non-standard properties and have been dropped. 5. `http.agent`, `http.keepAlive`, `http.auth.ntlm.domain`, `https.protocols`, and `https.cipherSuites` are used by Java's built-in HTTP client. The first three have been dropped, and I see no reason to keep the other two. I believe this is an exhaustive list. Now that I know exactly what we're dealing with, I think we can and should make this change in-place, without any API changes. Specifically: `useSystemProperties()` will be deprecated and have no effect, we'll drop the remaining legacy properties, we will delegate to the JDK by default for key store and proxy configuration, and we'll allow this delegation behavior to be overridden _programmatically_, rather than via system property. This results in a consistent configuration story across all the features we support: 1. HttpComponents _itself_ is always configured programmatically. 2. For a given feature, the default behavior is to delegate to the JDK implementation. This implementation may support out-of-band configuration via system properties, static methods, the `java.security` file, system-wide OS configuration, etc. 3. This delegation behavior can be overridden by a programmatic config option. It's remarkable how consistent this schema already is, and the biggest inconsistency was recently fixed in the 5.6 release when we changed the default `HostnameVerificationPolicy` to `BUILTIN`. I think that was a far more aggressive breaking change than what I'm proposing here. If we change the current defaults for key store and proxy configuration, they fit perfectly into the schema I'm describing: | Feature | Default JDK behavior | Example JDK config property | Override strategy | | ----------- | ----------- | ----------- | ----------- | | Trust store | Load from OS | `javax.net.ssl.trustStore` | Set a `TrustManager[]` | | Key store | Load nothing | `javax.net.ssl.keyStore` | Set a `KeyManager[]` | | Hostname verification | Run `HostnameChecker` from `sun.security.util` | None | Set a `HostnameVerifier` and `HostnameVerificationPolicy` | | Proxy config | Use system properties or load from OS | `java.net.useSystemProxies` | Set a `ProxySelector` or `HttpRoutePlanner` | | Client cipher suites | Send all supported cipher suites | `java.security.properties` | Set an `SSLContext` | | IP family selection | Prefer IPv6 | `java.net.preferIPv4Stack` | Set a `DetachedSocketFactory` or `DnsResolver`, call `setUnixDomainSocket`, etc | | DNS resolution | Use built-in resolver | `networkaddress.cache.ttl` | Set a `DnsResolver` | I think this would be a really good outcome. The risk of breakage is low, we don't introduce any API churn, we solve @jonenst's problem, we eliminate confusing legacy cruft, and we make client configuration more consistent. As an added bonus, our code won't read or interpret _any_ system properties other than `java.version`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
