Skip to content

Commit ab404c4

Browse files
rschmittrschmitt
committed
[HTTPCLIENT-2381] Respect system properties by default
The practical effect of this change is that default JVM configuration for key stores (for mutual TLS) and proxy selection will be respected by default. This change has two goals. First, applications and libraries built on the client will work with proxies much more reliably, since their authors no longer need to opt in to a non-default setting in order for things like proxies to be configurable in the usual manner (i.e. without code changes). Second, this change, along with the change in 5.6 to make `BUILTIN` the default `HostnameVerificationPolicy`, makes the client's configuration philosophy fully consistent across all supported features. To wit: 1. HttpComponents _itself_ is always configured programmatically and does not directly read system properties to obtain config values. 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, either to directly customize the JDK-supplied implementation or to supply an alternate implementation. This table shows what this philosophy looks like concretely across various features: | 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` | See also the previous discussion at: #773
1 parent 6af0624 commit ab404c4

File tree

22 files changed

+63
-279
lines changed

22 files changed

+63
-279
lines changed

‎httpclient5-fluent/src/main/java/org/apache/hc/client5/http/fluent/Executor.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,12 @@ static CloseableHttpClient GET_CLASSIC_CLIENT() {
7777
if (CLIENT == null) {
7878
CLIENT = HttpClientBuilder.create()
7979
.setConnectionManager(PoolingHttpClientConnectionManagerBuilder.create()
80-
.useSystemProperties()
8180
.setMaxConnPerRoute(100)
8281
.setMaxConnTotal(200)
8382
.setDefaultConnectionConfig(ConnectionConfig.custom()
8483
.setValidateAfterInactivity(TimeValue.ofSeconds(10))
8584
.build())
8685
.build())
87-
.useSystemProperties()
8886
.evictExpiredConnections()
8987
.evictIdleConnections(TimeValue.ofMinutes(1))
9088
.build();
@@ -105,15 +103,13 @@ static CloseableHttpClient GET_ASYNC_CLIENT() {
105103
if (ASYNC_CLIENT == null) {
106104
ASYNC_CLIENT = new ClassicToAsyncAdaptor(HttpAsyncClientBuilder.create()
107105
.setConnectionManager(PoolingAsyncClientConnectionManagerBuilder.create()
108-
.useSystemProperties()
109106
.setMaxConnPerRoute(100)
110107
.setMaxConnTotal(200)
111108
.setMessageMultiplexing(true)
112109
.setDefaultConnectionConfig(ConnectionConfig.custom()
113110
.setValidateAfterInactivity(TimeValue.ofSeconds(10))
114111
.build())
115112
.build())
116-
.useSystemProperties()
117113
.evictExpiredConnections()
118114
.evictIdleConnections(TimeValue.ofMinutes(1))
119115
.build(), Timeout.ofMinutes(5));

‎httpclient5-sse/src/main/java/org/apache/hc/client5/http/sse/SseExecutor.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,10 @@ static CloseableHttpAsyncClient getSharedClient() {
103103
if (c == null) {
104104
c = HttpAsyncClientBuilder.create()
105105
.setConnectionManager(PoolingAsyncClientConnectionManagerBuilder.create()
106-
.useSystemProperties()
107106
.setMaxConnPerRoute(100)
108107
.setMaxConnTotal(200)
109108
.setMessageMultiplexing(true)
110109
.build())
111-
.useSystemProperties()
112110
.evictExpiredConnections()
113111
.evictIdleConnections(TimeValue.ofMinutes(1))
114112
.build();

‎httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/example/ClientSseExample.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ public static void main(final String[] args) throws Exception {
6767

6868
final PoolingAsyncClientConnectionManager connMgr =
6969
PoolingAsyncClientConnectionManagerBuilder.create()
70-
.useSystemProperties()
7170
.setMessageMultiplexing(true) // HTTP/2 stream multiplexing
7271
.setMaxConnPerRoute(32)
7372
.setMaxConnTotal(256)
@@ -84,7 +83,6 @@ public static void main(final String[] args) throws Exception {
8483
.setPushEnabled(false)
8584
.setMaxConcurrentStreams(256)
8685
.build())
87-
.useSystemProperties()
8886
.evictExpiredConnections()
8987
.evictIdleConnections(TimeValue.ofMinutes(1))
9088
.build();

‎httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/example/ClientSseH2Example.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@ public static void main(final String[] args) throws Exception {
145145

146146
final PoolingAsyncClientConnectionManager connMgr =
147147
PoolingAsyncClientConnectionManagerBuilder.create()
148-
.useSystemProperties()
149148
.setMessageMultiplexing(true)
150149
.setMaxConnPerRoute(1)
151150
.setMaxConnTotal(4)
@@ -161,7 +160,6 @@ public static void main(final String[] args) throws Exception {
161160
.setPushEnabled(false)
162161
.setMaxConcurrentStreams(Math.max(64, streamCount * 8))
163162
.build())
164-
.useSystemProperties()
165163
.evictExpiredConnections()
166164
.evictIdleConnections(TimeValue.ofMinutes(1))
167165
.build();

‎httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/example/SsePerfClient.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ public static void main(final String[] args) throws Exception {
107107

108108
final PoolingAsyncClientConnectionManager connMgr =
109109
PoolingAsyncClientConnectionManagerBuilder.create()
110-
.useSystemProperties()
111110
.setMessageMultiplexing(true)
112111
.setMaxConnPerRoute(16)
113112
.setMaxConnTotal(16)
@@ -123,7 +122,6 @@ public static void main(final String[] args) throws Exception {
123122
.setPushEnabled(false)
124123
.setMaxConcurrentStreams(Math.max(64, streams * 4))
125124
.build())
126-
.useSystemProperties()
127125
.evictExpiredConnections()
128126
.evictIdleConnections(TimeValue.ofMinutes(1))
129127
.build();

‎httpclient5-sse/src/test/java/org/apache/hc/client5/http/sse/example/performance/SsePerfClient.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ public static void main(final String[] args) throws Exception {
9090

9191
final PoolingAsyncClientConnectionManager connMgr =
9292
PoolingAsyncClientConnectionManagerBuilder.create()
93-
.useSystemProperties()
9493
.setMessageMultiplexing(true) // enable H2 multiplexing if negotiated
9594
.setMaxConnPerRoute(Math.max(64, connections))
9695
.setMaxConnTotal(Math.max(128, connections))
@@ -107,7 +106,6 @@ public static void main(final String[] args) throws Exception {
107106
.setPushEnabled(false)
108107
.setMaxConcurrentStreams(512)
109108
.build())
110-
.useSystemProperties()
111109
.evictExpiredConnections()
112110
.evictIdleConnections(TimeValue.ofMinutes(1))
113111
.build();

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@
5555
import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy;
5656
import org.apache.hc.client5.http.impl.DefaultRedirectStrategy;
5757
import org.apache.hc.client5.http.impl.DefaultSchemePortResolver;
58-
import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider;
5958
import org.apache.hc.client5.http.impl.auth.BasicSchemeFactory;
6059
import org.apache.hc.client5.http.impl.auth.BearerSchemeFactory;
6160
import org.apache.hc.client5.http.impl.auth.DigestSchemeFactory;
@@ -201,7 +200,6 @@ private ExecInterceptorEntry(
201200
private boolean evictIdleConnections;
202201
private TimeValue maxIdleTime;
203202

204-
private boolean systemProperties;
205203
private boolean automaticRetriesDisabled;
206204
private boolean redirectHandlingDisabled;
207205
private boolean cookieManagementDisabled;
@@ -669,13 +667,13 @@ public final H2AsyncClientBuilder setDefaultConnectionConfig(final ConnectionCon
669667
}
670668

671669
/**
672-
* Use system properties when creating and configuring default
673-
* implementations.
670+
* Ignored.
674671
*
672+
* @deprecated This method is now redundant and calls to it can be removed.
675673
* @return this instance.
676674
*/
675+
@Deprecated
677676
public final H2AsyncClientBuilder useSystemProperties() {
678-
this.systemProperties = true;
679677
return this;
680678
}
681679

@@ -964,20 +962,12 @@ public CloseableHttpAsyncClient build() {
964962

965963
CredentialsProvider credentialsProviderCopy = this.credentialsProvider;
966964
if (credentialsProviderCopy == null) {
967-
if (systemProperties) {
968-
credentialsProviderCopy = new SystemDefaultCredentialsProvider();
969-
} else {
970-
credentialsProviderCopy = new BasicCredentialsProvider();
971-
}
965+
credentialsProviderCopy = new SystemDefaultCredentialsProvider();
972966
}
973967

974968
TlsStrategy tlsStrategyCopy = this.tlsStrategy;
975969
if (tlsStrategyCopy == null) {
976-
if (systemProperties) {
977-
tlsStrategyCopy = DefaultClientTlsStrategy.createSystemDefault();
978-
} else {
979-
tlsStrategyCopy = DefaultClientTlsStrategy.createDefault();
980-
}
970+
tlsStrategyCopy = DefaultClientTlsStrategy.createDefault();
981971
}
982972

983973
final MultihomeConnectionInitiator connectionInitiator = new MultihomeConnectionInitiator(ioReactor, dnsResolver);

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientBuilder.java‎

Lines changed: 6 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,13 @@
6565
import org.apache.hc.client5.http.impl.DefaultUserTokenHandler;
6666
import org.apache.hc.client5.http.impl.IdleConnectionEvictor;
6767
import org.apache.hc.client5.http.impl.NoopUserTokenHandler;
68-
import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider;
6968
import org.apache.hc.client5.http.impl.auth.BasicSchemeFactory;
7069
import org.apache.hc.client5.http.impl.auth.BearerSchemeFactory;
7170
import org.apache.hc.client5.http.impl.auth.DigestSchemeFactory;
7271
import org.apache.hc.client5.http.impl.auth.ScramSchemeFactory;
7372
import org.apache.hc.client5.http.impl.auth.SystemDefaultCredentialsProvider;
7473
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
7574
import org.apache.hc.client5.http.impl.routing.DefaultProxyRoutePlanner;
76-
import org.apache.hc.client5.http.impl.routing.DefaultRoutePlanner;
7775
import org.apache.hc.client5.http.impl.routing.SystemDefaultRoutePlanner;
7876
import org.apache.hc.client5.http.nio.AsyncClientConnectionManager;
7977
import org.apache.hc.client5.http.protocol.H2RequestPriority;
@@ -137,20 +135,6 @@
137135
* separate connections leased from the connection pool.
138136
* </p>
139137
* <p>
140-
* When a particular component is not explicitly set this class will
141-
* use its default implementation. System properties will be taken
142-
* into account when configuring the default implementations when
143-
* {@link #useSystemProperties()} method is called prior to calling
144-
* {@link #build()}.
145-
* </p>
146-
* <ul>
147-
* <li>http.proxyHost</li>
148-
* <li>http.proxyPort</li>
149-
* <li>https.proxyHost</li>
150-
* <li>https.proxyPort</li>
151-
* <li>http.nonProxyHosts</li>
152-
* </ul>
153-
* <p>
154138
* Please note that some settings used by this class can be mutually
155139
* exclusive and may not apply when building {@link CloseableHttpAsyncClient}
156140
* instances.
@@ -252,7 +236,6 @@ private ExecInterceptorEntry(
252236
private boolean evictIdleConnections;
253237
private TimeValue maxIdleTime;
254238

255-
private boolean systemProperties;
256239
private boolean automaticRetriesDisabled;
257240
private boolean redirectHandlingDisabled;
258241
private boolean cookieManagementDisabled;
@@ -776,13 +759,13 @@ public final HttpAsyncClientBuilder setDefaultRequestConfig(final RequestConfig
776759
}
777760

778761
/**
779-
* Use system properties when creating and configuring default
780-
* implementations.
762+
* Ignored.
781763
*
764+
* @deprecated This method is now redundant and calls to it can be removed.
782765
* @return this instance.
783766
*/
767+
@Deprecated
784768
public final HttpAsyncClientBuilder useSystemProperties() {
785-
this.systemProperties = true;
786769
return this;
787770
}
788771

@@ -1010,11 +993,7 @@ public AsyncClientConnectionManager getConnManager() {
1010993
public CloseableHttpAsyncClient build() {
1011994
AsyncClientConnectionManager connManagerCopy = this.connManager;
1012995
if (connManagerCopy == null) {
1013-
final PoolingAsyncClientConnectionManagerBuilder connectionManagerBuilder = PoolingAsyncClientConnectionManagerBuilder.create();
1014-
if (systemProperties) {
1015-
connectionManagerBuilder.useSystemProperties();
1016-
}
1017-
connManagerCopy = connectionManagerBuilder.build();
996+
connManagerCopy = PoolingAsyncClientConnectionManagerBuilder.create().build();
1018997
}
1019998

1020999
ConnectionKeepAliveStrategy keepAliveStrategyCopy = this.keepAliveStrategy;
@@ -1166,11 +1145,9 @@ public CloseableHttpAsyncClient build() {
11661145
routePlannerCopy = new DefaultProxyRoutePlanner(proxy, schemePortResolverCopy);
11671146
} else if (this.proxySelector != null) {
11681147
routePlannerCopy = new SystemDefaultRoutePlanner(schemePortResolverCopy, this.proxySelector);
1169-
} else if (systemProperties) {
1148+
} else {
11701149
final ProxySelector defaultProxySelector = ProxySelector.getDefault();
11711150
routePlannerCopy = new SystemDefaultRoutePlanner(schemePortResolverCopy, defaultProxySelector);
1172-
} else {
1173-
routePlannerCopy = new DefaultRoutePlanner(schemePortResolverCopy);
11741151
}
11751152
}
11761153

@@ -1276,11 +1253,7 @@ public CloseableHttpAsyncClient build() {
12761253

12771254
CredentialsProvider credentialsProviderCopy = this.credentialsProvider;
12781255
if (credentialsProviderCopy == null) {
1279-
if (systemProperties) {
1280-
credentialsProviderCopy = new SystemDefaultCredentialsProvider();
1281-
} else {
1282-
credentialsProviderCopy = new BasicCredentialsProvider();
1283-
}
1256+
credentialsProviderCopy = new SystemDefaultCredentialsProvider();
12841257
}
12851258

12861259
return new InternalHttpAsyncClient(

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClients.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,12 @@ public static CloseableHttpAsyncClient createDefault() {
8989
/**
9090
* Creates {@link CloseableHttpAsyncClient} instance with default
9191
* configuration and system properties.
92+
*
93+
* @deprecated This is now a synonym for {@link #createDefault}; call that instead.
9294
*/
95+
@Deprecated
9396
public static CloseableHttpAsyncClient createSystem() {
94-
return HttpAsyncClientBuilder.create().useSystemProperties().build();
97+
return createDefault();
9598
}
9699

97100
/**
@@ -112,11 +115,13 @@ public static CloseableHttpAsyncClient createHttp2Default() {
112115
}
113116

114117
/**
115-
* Creates HTTP/2 {@link CloseableHttpAsyncClient} instance with default configuration and
116-
* system properties optimized for HTTP/2 protocol and message multiplexing.
118+
* Creates HTTP/2 {@link CloseableHttpAsyncClient} instance with default configuration.
119+
*
120+
* @deprecated This is now a synonym for {@link #createHttp2Default}; call that instead.
117121
*/
122+
@Deprecated
118123
public static CloseableHttpAsyncClient createHttp2System() {
119-
return H2AsyncClientBuilder.create().useSystemProperties().build();
124+
return createHttp2Default();
120125
}
121126

122127
private static HttpProcessor createMinimalProtocolProcessor() {

0 commit comments

Comments
 (0)