From 77c3f08afe39dae92c65e8e40010451e369cb1c8 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Thu, 17 Jul 2025 15:03:03 +0200 Subject: [PATCH 1/3] HTTPCLIENT-2381 Enable automatic mapping of HTTP(S)_PROXY and NO_PROXY environment variables to standard JDK proxy system properties via new EnvironmentProxyConfigurer and make HttpClientBuilder use it by default --- httpclient5/pom.xml | 5 + .../config/EnvironmentProxyConfigurer.java | 194 ++++++++++++++++++ .../http/impl/classic/HttpClientBuilder.java | 36 ++++ .../EnvironmentProxyConfigurerTest.java | 124 +++++++++++ pom.xml | 6 + 5 files changed, 365 insertions(+) create mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurer.java create mode 100644 httpclient5/src/test/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurerTest.java diff --git a/httpclient5/pom.xml b/httpclient5/pom.xml index ff3992fd50..a57f480e4d 100644 --- a/httpclient5/pom.xml +++ b/httpclient5/pom.xml @@ -118,6 +118,11 @@ zstd-jni test + + com.github.stefanbirkner + system-lambda + test + diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurer.java b/httpclient5/src/main/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurer.java new file mode 100644 index 0000000000..fefa023eda --- /dev/null +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurer.java @@ -0,0 +1,194 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ +package org.apache.hc.client5.http.config; + +import java.net.URI; +import java.security.AccessController; +import java.security.PrivilegedAction; + +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.ThreadingBehavior; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + *

EnvironmentProxyConfigurer

+ * + *

+ * Many *nix shells, container runtimes, and CI systems expose an outbound + * proxy exclusively via the environment variables {@code HTTP_PROXY}, + * {@code HTTPS_PROXY}, and {@code NO_PROXY}. The JDK, however, expects the + * equivalent system properties + * ({@code http.proxyHost}, {@code http.proxyPort}, &c.) when it resolves a + * proxy through {@link java.net.ProxySelector} or performs authentication via + * {@link java.net.Authenticator}. + *

+ * + *

+ * EnvironmentProxyConfigurer is a small, opt-in + * helper that copies those variables to the standard properties once, at + * application start-up. It is not invoked automatically by + * HttpClient. Call it explicitly if you want the mapping: + *

+ * + * 
{@code
+ * EnvironmentProxyConfigurer.apply();   // one-liner
+ * CloseableHttpClient client = HttpClientBuilder.create()
+ *         .useSystemProperties()        // default behaviour
+ *         .build();
+ * }
+ * + *

Mapping rules

+ *
    + *
  • {@code HTTP_PROXY} → {@code http.proxyHost}, + * {@code http.proxyPort}, {@code http.proxyUser}, + * {@code http.proxyPassword}
    • + *
  • {@code HTTPS_PROXY} → {@code https.proxyHost}, + * {@code https.proxyPort}, {@code https.proxyUser}, + * {@code https.proxyPassword}
    • + *
  • {@code NO_PROXY} → {@code http.nonProxyHosts}, + * {@code https.nonProxyHosts}  (commas are converted to the + * ‘|’ separator required by the JDK)
    • + *
  • Lower-case aliases ({@code http_proxy}, {@code https_proxy}, + * {@code no_proxy}) are recognised as well.
    • + *
+ * + *

Design notes

+ *
    + *
  • Idempotent: if a target property is already set + * (e.g. via {@code -Dhttp.proxyHost=…}) it is left untouched.
    • + *
  • Thread-safe: all reads and writes are wrapped in + * {@code AccessController.doPrivileged} and synchronise only on the + * global {@link System} properties map.
    • + *
+ * + *

Warning

+ *

+ * Calling {@link #apply()} changes JVM-wide system properties. The new proxy + * settings therefore apply to all libraries and threads in the same + * process. Invoke this method only if your application really needs to + * inherit proxy configuration from the environment and you are aware that + * other components may be affected. + *

+ * + *

+ * The class is {@linkplain org.apache.hc.core5.annotation.Contract stateless} + * and safe to call multiple times; subsequent invocations are no-ops once the + * copy has succeeded. + *

+ * + * @since 5.6 + */ +@Contract(threading = ThreadingBehavior.STATELESS) +public final class EnvironmentProxyConfigurer { + + /** + * Logger associated to this class. + */ + private static final Logger LOG = LoggerFactory.getLogger(EnvironmentProxyConfigurer.class); + + private EnvironmentProxyConfigurer() { + } + + public static void apply() { + configureForScheme("http", "HTTP_PROXY", "http_proxy"); + configureForScheme("https", "HTTPS_PROXY", "https_proxy"); + + final String noProxy = firstNonEmpty(getenv("NO_PROXY"), getenv("no_proxy")); + if (noProxy != null && System.getProperty("http.nonProxyHosts") == null) { + final String list = noProxy.replace(',', '|'); + setProperty("http.nonProxyHosts", list); + + // only write HTTPS when it is still unset + boolean httpsWritten = false; + if (System.getProperty("https.nonProxyHosts") == null) { + setProperty("https.nonProxyHosts", list); + httpsWritten = true; + } + + if (LOG.isWarnEnabled()) { + LOG.warn("Applied NO_PROXY → " + list + + (httpsWritten ? " (http & https)" : " (http only)")); + } + } + } + + /* -------------------------------------------------------------- */ + + private static void configureForScheme(final String scheme, + final String upperEnv, + final String lowerEnv) { + + if (System.getProperty(scheme + ".proxyHost") != null) { + return; // already configured via -D + } + String val = firstNonEmpty(getenv(upperEnv), getenv(lowerEnv)); + if (val == null || val.isEmpty()) { + return; + } + if (val.indexOf("://") < 0) { + val = scheme + "://" + val; + } + + final URI uri = URI.create(val); + + if (uri.getHost() != null) { + setProperty(scheme + ".proxyHost", uri.getHost()); + } + if (uri.getPort() > 0) { + setProperty(scheme + ".proxyPort", Integer.toString(uri.getPort())); + } + + final String ui = uri.getUserInfo(); // user:pass + if (ui != null && !ui.isEmpty()) { + final String[] parts = ui.split(":", 2); + setProperty(scheme + ".proxyUser", parts[0]); + if (parts.length == 2) { + setProperty(scheme + ".proxyPassword", parts[1]); + } + } + } + + private static String firstNonEmpty(final String a, final String b) { + return (a != null && !a.isEmpty()) ? a + : (b != null && !b.isEmpty()) ? b + : null; + } + + private static String getenv(final String key) { + return AccessController.doPrivileged( + (PrivilegedAction) () -> System.getenv(key)); + } + + private static void setProperty(final String key, final String value) { + AccessController.doPrivileged( + (PrivilegedAction) () -> { + System.setProperty(key, value); + return null; + }); + } +} \ No newline at end of file diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java index a9483d6132..5e8d869775 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java @@ -51,6 +51,7 @@ import org.apache.hc.client5.http.classic.BackoffManager; import org.apache.hc.client5.http.classic.ConnectionBackoffStrategy; import org.apache.hc.client5.http.classic.ExecChainHandler; +import org.apache.hc.client5.http.config.EnvironmentProxyConfigurer; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.cookie.BasicCookieStore; import org.apache.hc.client5.http.cookie.CookieSpecFactory; @@ -237,6 +238,9 @@ private ExecInterceptorEntry( private List closeables; + private boolean applyEnvProxy; + + public static HttpClientBuilder create() { return new HttpClientBuilder(); } @@ -791,6 +795,33 @@ public final HttpClientBuilder disableDefaultUserAgent() { return this; } + /** + * Enables transparent transfer of proxy-related environment variables + * to the standard JDK system-properties for this client only. + *

+ * When this flag is set, {@link EnvironmentProxyConfigurer#apply()} + * will be invoked during {@link #build()}, copying + * {@code HTTP_PROXY}, {@code HTTPS_PROXY} and {@code NO_PROXY} + * (plus lower-case aliases) to their {@code http.*}/{@code https.*} + * counterparts, provided those properties are not already defined. + *

+ * + *

Opt-in behaviour: if you do not call + * {@code useEnvironmentProxy()}, the builder leaves JVM system + * properties untouched. Combine with + * {@link #useSystemProperties()} (enabled by default via + * {@code HttpClientBuilder.create()}) so the newly populated + * properties are actually honoured by the client.

+ * + * @return this builder, for method chaining + * @since 5.6 + */ + public HttpClientBuilder useEnvironmentProxy() { + this.applyEnvProxy = true; + return this; + } + + /** * Sets the {@link ProxySelector} that will be used to select the proxies * to be used for establishing HTTP connections. If a non-null proxy selector is set, @@ -838,6 +869,11 @@ protected Function contextAdaptor() { } public CloseableHttpClient build() { + + if (applyEnvProxy) { + EnvironmentProxyConfigurer.apply(); + } + // Create main request executor // We copy the instance fields to avoid changing them, and rename to avoid accidental use of the wrong version HttpRequestExecutor requestExecCopy = this.requestExec; diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurerTest.java b/httpclient5/src/test/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurerTest.java new file mode 100644 index 0000000000..821a45421a --- /dev/null +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurerTest.java @@ -0,0 +1,124 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ + +package org.apache.hc.client5.http.config; + +import static com.github.stefanbirkner.systemlambda.SystemLambda.withEnvironmentVariable; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.util.HashMap; +import java.util.Map; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledForJreRange; +import org.junit.jupiter.api.condition.JRE; + +/** + * Verifies EnvironmentProxyConfigurer on JDKs that allow environment + * mutation without --add-opens. Disabled automatically on 16+. + */ +@EnabledForJreRange(max = JRE.JAVA_15) // ⬅️ key line +class EnvironmentProxyConfigurerTest { + + /** + * keep original values (null allowed) + */ + private final Map backup = new HashMap<>(); + + private void backup(final String... keys) { + for (final String k : keys) { + backup.put(k, System.getProperty(k)); // value may be null + } + } + + @AfterEach + void restore() { + backup.forEach((k, v) -> { + if (v == null) { + System.clearProperty(k); + } else { + System.setProperty(k, v); + } + }); + backup.clear(); + } + + @Test + void sets_http_system_properties_from_uppercase_env() throws Exception { + backup("http.proxyHost", "http.proxyPort", "http.proxyUser", "http.proxyPassword"); + + withEnvironmentVariable("HTTP_PROXY", "http://user:pass@proxy.acme.com:8080") + .execute(() -> { + EnvironmentProxyConfigurer.apply(); + assertEquals("proxy.acme.com", System.getProperty("http.proxyHost")); + assertEquals("8080", System.getProperty("http.proxyPort")); + assertEquals("user", System.getProperty("http.proxyUser")); + assertEquals("pass", System.getProperty("http.proxyPassword")); + }); + } + + @Test + void does_not_overwrite_already_set_properties() throws Exception { + backup("http.proxyHost"); + System.setProperty("http.proxyHost", "preset"); + + withEnvironmentVariable("HTTP_PROXY", "http://other:1111") + .execute(() -> { + EnvironmentProxyConfigurer.apply(); + assertEquals("preset", System.getProperty("http.proxyHost")); + }); + } + + @Test + void translates_no_proxy_to_pipe_delimited_hosts() throws Exception { + backup("http.nonProxyHosts", "https.nonProxyHosts"); + + // ensure both props are null before we invoke the bridge + System.clearProperty("http.nonProxyHosts"); + System.clearProperty("https.nonProxyHosts"); + + withEnvironmentVariable("NO_PROXY", "localhost,127.0.0.1") + .execute(() -> { + EnvironmentProxyConfigurer.apply(); + assertEquals("localhost|127.0.0.1", + System.getProperty("http.nonProxyHosts")); + assertEquals("localhost|127.0.0.1", + System.getProperty("https.nonProxyHosts")); + }); + } + + @Test + void noop_when_no_relevant_env_vars() { + backup("http.proxyHost"); + System.clearProperty("http.proxyHost"); + + EnvironmentProxyConfigurer.apply(); + assertNull(System.getProperty("http.proxyHost")); + } +} diff --git a/pom.xml b/pom.xml index f748421420..cd3453f76e 100644 --- a/pom.xml +++ b/pom.xml @@ -80,6 +80,7 @@ javax.net.ssl.SSLEngine,javax.net.ssl.SSLParameters,java.nio.ByteBuffer,java.nio.CharBuffer 1.27.1 1.5.7-4 + 1.2.1 @@ -216,6 +217,11 @@ zstd-jni ${zstd.jni.version} + + com.github.stefanbirkner + system-lambda + ${stefanbirkner.version} + From b38a7b3ac38e713d04b665be4f86f281f406717a Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Mon, 21 Jul 2025 07:39:20 +0200 Subject: [PATCH 2/3] move env-proxy mapping into SystemDefaultRoutePlanner so routes honour HTTP(S)_PROXY / NO_PROXY directly without altering JVM system properties --- .../config/EnvironmentProxyConfigurer.java | 194 ------------------ .../http/impl/classic/HttpClientBuilder.java | 35 ---- .../routing/SystemDefaultRoutePlanner.java | 77 ++++++- .../EnvironmentProxyConfigurerTest.java | 124 ----------- .../TestSystemDefaultRoutePlanner.java | 58 +++++- 5 files changed, 132 insertions(+), 356 deletions(-) delete mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurer.java delete mode 100644 httpclient5/src/test/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurerTest.java diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurer.java b/httpclient5/src/main/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurer.java deleted file mode 100644 index fefa023eda..0000000000 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurer.java +++ /dev/null @@ -1,194 +0,0 @@ -/* - * ==================================================================== - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - * ==================================================================== - * - * This software consists of voluntary contributions made by many - * individuals on behalf of the Apache Software Foundation. For more - * information on the Apache Software Foundation, please see - * . - * - */ -package org.apache.hc.client5.http.config; - -import java.net.URI; -import java.security.AccessController; -import java.security.PrivilegedAction; - -import org.apache.hc.core5.annotation.Contract; -import org.apache.hc.core5.annotation.ThreadingBehavior; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - *

EnvironmentProxyConfigurer

- * - *

- * Many *nix shells, container runtimes, and CI systems expose an outbound - * proxy exclusively via the environment variables {@code HTTP_PROXY}, - * {@code HTTPS_PROXY}, and {@code NO_PROXY}. The JDK, however, expects the - * equivalent system properties - * ({@code http.proxyHost}, {@code http.proxyPort}, &c.) when it resolves a - * proxy through {@link java.net.ProxySelector} or performs authentication via - * {@link java.net.Authenticator}. - *

- * - *

- * EnvironmentProxyConfigurer is a small, opt-in - * helper that copies those variables to the standard properties once, at - * application start-up. It is not invoked automatically by - * HttpClient. Call it explicitly if you want the mapping: - *

- * - * 
{@code
- * EnvironmentProxyConfigurer.apply();   // one-liner
- * CloseableHttpClient client = HttpClientBuilder.create()
- *         .useSystemProperties()        // default behaviour
- *         .build();
- * }
- * - *

Mapping rules

- *
    - *
  • {@code HTTP_PROXY} → {@code http.proxyHost}, - * {@code http.proxyPort}, {@code http.proxyUser}, - * {@code http.proxyPassword}
    • - *
  • {@code HTTPS_PROXY} → {@code https.proxyHost}, - * {@code https.proxyPort}, {@code https.proxyUser}, - * {@code https.proxyPassword}
    • - *
  • {@code NO_PROXY} → {@code http.nonProxyHosts}, - * {@code https.nonProxyHosts}  (commas are converted to the - * ‘|’ separator required by the JDK)
    • - *
  • Lower-case aliases ({@code http_proxy}, {@code https_proxy}, - * {@code no_proxy}) are recognised as well.
    • - *
- * - *

Design notes

- *
    - *
  • Idempotent: if a target property is already set - * (e.g. via {@code -Dhttp.proxyHost=…}) it is left untouched.
    • - *
  • Thread-safe: all reads and writes are wrapped in - * {@code AccessController.doPrivileged} and synchronise only on the - * global {@link System} properties map.
    • - *
- * - *

Warning

- *

- * Calling {@link #apply()} changes JVM-wide system properties. The new proxy - * settings therefore apply to all libraries and threads in the same - * process. Invoke this method only if your application really needs to - * inherit proxy configuration from the environment and you are aware that - * other components may be affected. - *

- * - *

- * The class is {@linkplain org.apache.hc.core5.annotation.Contract stateless} - * and safe to call multiple times; subsequent invocations are no-ops once the - * copy has succeeded. - *

- * - * @since 5.6 - */ -@Contract(threading = ThreadingBehavior.STATELESS) -public final class EnvironmentProxyConfigurer { - - /** - * Logger associated to this class. - */ - private static final Logger LOG = LoggerFactory.getLogger(EnvironmentProxyConfigurer.class); - - private EnvironmentProxyConfigurer() { - } - - public static void apply() { - configureForScheme("http", "HTTP_PROXY", "http_proxy"); - configureForScheme("https", "HTTPS_PROXY", "https_proxy"); - - final String noProxy = firstNonEmpty(getenv("NO_PROXY"), getenv("no_proxy")); - if (noProxy != null && System.getProperty("http.nonProxyHosts") == null) { - final String list = noProxy.replace(',', '|'); - setProperty("http.nonProxyHosts", list); - - // only write HTTPS when it is still unset - boolean httpsWritten = false; - if (System.getProperty("https.nonProxyHosts") == null) { - setProperty("https.nonProxyHosts", list); - httpsWritten = true; - } - - if (LOG.isWarnEnabled()) { - LOG.warn("Applied NO_PROXY → " + list - + (httpsWritten ? " (http & https)" : " (http only)")); - } - } - } - - /* -------------------------------------------------------------- */ - - private static void configureForScheme(final String scheme, - final String upperEnv, - final String lowerEnv) { - - if (System.getProperty(scheme + ".proxyHost") != null) { - return; // already configured via -D - } - String val = firstNonEmpty(getenv(upperEnv), getenv(lowerEnv)); - if (val == null || val.isEmpty()) { - return; - } - if (val.indexOf("://") < 0) { - val = scheme + "://" + val; - } - - final URI uri = URI.create(val); - - if (uri.getHost() != null) { - setProperty(scheme + ".proxyHost", uri.getHost()); - } - if (uri.getPort() > 0) { - setProperty(scheme + ".proxyPort", Integer.toString(uri.getPort())); - } - - final String ui = uri.getUserInfo(); // user:pass - if (ui != null && !ui.isEmpty()) { - final String[] parts = ui.split(":", 2); - setProperty(scheme + ".proxyUser", parts[0]); - if (parts.length == 2) { - setProperty(scheme + ".proxyPassword", parts[1]); - } - } - } - - private static String firstNonEmpty(final String a, final String b) { - return (a != null && !a.isEmpty()) ? a - : (b != null && !b.isEmpty()) ? b - : null; - } - - private static String getenv(final String key) { - return AccessController.doPrivileged( - (PrivilegedAction) () -> System.getenv(key)); - } - - private static void setProperty(final String key, final String value) { - AccessController.doPrivileged( - (PrivilegedAction) () -> { - System.setProperty(key, value); - return null; - }); - } -} \ No newline at end of file diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java index 5e8d869775..8a53b0129e 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java @@ -51,7 +51,6 @@ import org.apache.hc.client5.http.classic.BackoffManager; import org.apache.hc.client5.http.classic.ConnectionBackoffStrategy; import org.apache.hc.client5.http.classic.ExecChainHandler; -import org.apache.hc.client5.http.config.EnvironmentProxyConfigurer; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.cookie.BasicCookieStore; import org.apache.hc.client5.http.cookie.CookieSpecFactory; @@ -238,9 +237,6 @@ private ExecInterceptorEntry( private List closeables; - private boolean applyEnvProxy; - - public static HttpClientBuilder create() { return new HttpClientBuilder(); } @@ -795,33 +791,6 @@ public final HttpClientBuilder disableDefaultUserAgent() { return this; } - /** - * Enables transparent transfer of proxy-related environment variables - * to the standard JDK system-properties for this client only. - *

- * When this flag is set, {@link EnvironmentProxyConfigurer#apply()} - * will be invoked during {@link #build()}, copying - * {@code HTTP_PROXY}, {@code HTTPS_PROXY} and {@code NO_PROXY} - * (plus lower-case aliases) to their {@code http.*}/{@code https.*} - * counterparts, provided those properties are not already defined. - *

- * - *

Opt-in behaviour: if you do not call - * {@code useEnvironmentProxy()}, the builder leaves JVM system - * properties untouched. Combine with - * {@link #useSystemProperties()} (enabled by default via - * {@code HttpClientBuilder.create()}) so the newly populated - * properties are actually honoured by the client.

- * - * @return this builder, for method chaining - * @since 5.6 - */ - public HttpClientBuilder useEnvironmentProxy() { - this.applyEnvProxy = true; - return this; - } - - /** * Sets the {@link ProxySelector} that will be used to select the proxies * to be used for establishing HTTP connections. If a non-null proxy selector is set, @@ -870,10 +839,6 @@ protected Function contextAdaptor() { public CloseableHttpClient build() { - if (applyEnvProxy) { - EnvironmentProxyConfigurer.apply(); - } - // Create main request executor // We copy the instance fields to avoid changing them, and rename to avoid accidental use of the wrong version HttpRequestExecutor requestExecCopy = this.requestExec; diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/routing/SystemDefaultRoutePlanner.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/routing/SystemDefaultRoutePlanner.java index 25566055c1..62bdb9b3be 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/routing/SystemDefaultRoutePlanner.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/routing/SystemDefaultRoutePlanner.java @@ -27,12 +27,16 @@ package org.apache.hc.client5.http.impl.routing; +import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.Proxy; import java.net.ProxySelector; import java.net.URI; import java.net.URISyntaxException; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.List; +import java.util.Locale; import org.apache.hc.client5.http.SchemePortResolver; import org.apache.hc.core5.annotation.Contract; @@ -85,7 +89,7 @@ protected HttpHost determineProxy(final HttpHost target, final HttpContext conte } if (proxySelectorInstance == null) { //The proxy selector can be "unset", so we must be able to deal with a null selector - return null; + return determineEnvProxy(target); // === env-fallback === } final List proxies = proxySelectorInstance.select(targetURI); final Proxy p = chooseProxy(proxies); @@ -100,8 +104,79 @@ protected HttpHost determineProxy(final HttpHost target, final HttpContext conte result = new HttpHost(null, isa.getAddress(), isa.getHostString(), isa.getPort()); } + if (result == null) { + result = determineEnvProxy(target); + } + return result; } + private static HttpHost determineEnvProxy(final HttpHost target) { + final boolean secure = "https".equalsIgnoreCase(target.getSchemeName()); + HttpHost proxy = proxyFromEnv(secure ? "HTTPS_PROXY" : "HTTP_PROXY"); + if (proxy == null && !secure) { + proxy = proxyFromEnv("HTTPS_PROXY"); // reuse HTTPS proxy for HTTP if only that exists + } + if (proxy != null && !isNoProxy(target)) { + return proxy; + } + return null; + } + + private static HttpHost proxyFromEnv(final String var) { + String val = getenv(var); + if (val == null || val.isEmpty()) { + val = getenv(var.toLowerCase(Locale.ROOT)); + } + if (val == null || val.isEmpty()) { + return null; + } + if (!val.contains("://")) { + val = "http://" + val; + } + try { + final URI uri = new URI(val); + final String host = uri.getHost(); + final int port = uri.getPort() != -1 + ? uri.getPort() + : ("https".equalsIgnoreCase(uri.getScheme()) ? 443 : 80); + return new HttpHost(uri.getScheme(), InetAddress.getByName(host), port); + } catch (final Exception ignore) { + return null; + } + } + + private static boolean isNoProxy(final HttpHost target) { + String list = getenv("NO_PROXY"); + if (list == null || list.isEmpty()) { + list = getenv("no_proxy"); + } + if (list == null || list.isEmpty()) { + return false; + } + final String host = target.getHostName().toLowerCase(Locale.ROOT); + final String hostPort = host + (target.getPort() != -1 ? ":" + target.getPort() : ""); + for (String rule : list.split(",")) { + rule = rule.trim().toLowerCase(Locale.ROOT); + if (rule.isEmpty()) { + continue; + } + if (rule.equals(host) || rule.equals(hostPort)) { + return true; // exact + } + if (rule.startsWith("*.") && host.endsWith(rule.substring(1))) { + return true; // *.example.com + } + if (rule.endsWith("/16") && host.startsWith(rule.substring(0, rule.length() - 3))) { + return true; // cidr /16 + } + } + return false; + } + + private static String getenv(final String key) { + return AccessController.doPrivileged( + (PrivilegedAction) () -> System.getenv(key)); + } private Proxy chooseProxy(final List proxies) { Proxy result = null; diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurerTest.java b/httpclient5/src/test/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurerTest.java deleted file mode 100644 index 821a45421a..0000000000 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurerTest.java +++ /dev/null @@ -1,124 +0,0 @@ -/* - * ==================================================================== - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - * ==================================================================== - * - * This software consists of voluntary contributions made by many - * individuals on behalf of the Apache Software Foundation. For more - * information on the Apache Software Foundation, please see - * . - * - */ - -package org.apache.hc.client5.http.config; - -import static com.github.stefanbirkner.systemlambda.SystemLambda.withEnvironmentVariable; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; - -import java.util.HashMap; -import java.util.Map; - -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.EnabledForJreRange; -import org.junit.jupiter.api.condition.JRE; - -/** - * Verifies EnvironmentProxyConfigurer on JDKs that allow environment - * mutation without --add-opens. Disabled automatically on 16+. - */ -@EnabledForJreRange(max = JRE.JAVA_15) // ⬅️ key line -class EnvironmentProxyConfigurerTest { - - /** - * keep original values (null allowed) - */ - private final Map backup = new HashMap<>(); - - private void backup(final String... keys) { - for (final String k : keys) { - backup.put(k, System.getProperty(k)); // value may be null - } - } - - @AfterEach - void restore() { - backup.forEach((k, v) -> { - if (v == null) { - System.clearProperty(k); - } else { - System.setProperty(k, v); - } - }); - backup.clear(); - } - - @Test - void sets_http_system_properties_from_uppercase_env() throws Exception { - backup("http.proxyHost", "http.proxyPort", "http.proxyUser", "http.proxyPassword"); - - withEnvironmentVariable("HTTP_PROXY", "http://user:pass@proxy.acme.com:8080") - .execute(() -> { - EnvironmentProxyConfigurer.apply(); - assertEquals("proxy.acme.com", System.getProperty("http.proxyHost")); - assertEquals("8080", System.getProperty("http.proxyPort")); - assertEquals("user", System.getProperty("http.proxyUser")); - assertEquals("pass", System.getProperty("http.proxyPassword")); - }); - } - - @Test - void does_not_overwrite_already_set_properties() throws Exception { - backup("http.proxyHost"); - System.setProperty("http.proxyHost", "preset"); - - withEnvironmentVariable("HTTP_PROXY", "http://other:1111") - .execute(() -> { - EnvironmentProxyConfigurer.apply(); - assertEquals("preset", System.getProperty("http.proxyHost")); - }); - } - - @Test - void translates_no_proxy_to_pipe_delimited_hosts() throws Exception { - backup("http.nonProxyHosts", "https.nonProxyHosts"); - - // ensure both props are null before we invoke the bridge - System.clearProperty("http.nonProxyHosts"); - System.clearProperty("https.nonProxyHosts"); - - withEnvironmentVariable("NO_PROXY", "localhost,127.0.0.1") - .execute(() -> { - EnvironmentProxyConfigurer.apply(); - assertEquals("localhost|127.0.0.1", - System.getProperty("http.nonProxyHosts")); - assertEquals("localhost|127.0.0.1", - System.getProperty("https.nonProxyHosts")); - }); - } - - @Test - void noop_when_no_relevant_env_vars() { - backup("http.proxyHost"); - System.clearProperty("http.proxyHost"); - - EnvironmentProxyConfigurer.apply(); - assertNull(System.getProperty("http.proxyHost")); - } -} diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/routing/TestSystemDefaultRoutePlanner.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/routing/TestSystemDefaultRoutePlanner.java index 29260551f5..82cd504254 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/routing/TestSystemDefaultRoutePlanner.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/routing/TestSystemDefaultRoutePlanner.java @@ -27,12 +27,15 @@ package org.apache.hc.client5.http.impl.routing; +import static com.github.stefanbirkner.systemlambda.SystemLambda.withEnvironmentVariable; + import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.Proxy; import java.net.ProxySelector; import java.net.URI; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.apache.hc.client5.http.HttpRoute; @@ -42,9 +45,12 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledForJreRange; +import org.junit.jupiter.api.condition.JRE; import org.mockito.ArgumentMatchers; import org.mockito.Mockito; + /** * Tests for {@link SystemDefaultRoutePlanner}. */ @@ -91,8 +97,8 @@ void testDirectDefaultPort() throws Exception { @Test void testProxy() throws Exception { - final InetAddress ia = InetAddress.getByAddress(new byte[] { - (byte)127, (byte)0, (byte)0, (byte)1 + final InetAddress ia = InetAddress.getByAddress(new byte[]{ + (byte) 127, (byte) 0, (byte) 0, (byte) 1 }); final InetSocketAddress isa1 = new InetSocketAddress(ia, 11111); final InetSocketAddress isa2 = new InetSocketAddress(ia, 22222); @@ -113,4 +119,52 @@ void testProxy() throws Exception { Assertions.assertEquals(isa1.getPort(), route.getProxyHost().getPort()); } + @EnabledForJreRange(max = JRE.JAVA_15) + @Test + void testEnvHttpProxy() throws Exception { + withEnvironmentVariable("HTTP_PROXY", "http://proxy.acme.local:8080") + .execute(() -> { + Mockito.when(proxySelector.select(ArgumentMatchers.any())) + .thenReturn(Collections.singletonList(Proxy.NO_PROXY)); + + final HttpHost target = new HttpHost("http", "example.com", 80); + final HttpRoute route = routePlanner.determineRoute( + target, HttpClientContext.create()); + + Assertions.assertNull(route.getProxyHost()); + }); + } + @EnabledForJreRange(max = JRE.JAVA_15) + @Test + void testEnvHttpsProxy() throws Exception { + withEnvironmentVariable("HTTPS_PROXY", "http://secure.proxy:8443") + .execute(() -> { + Mockito.when(proxySelector.select(ArgumentMatchers.any())) + .thenReturn(Collections.singletonList(Proxy.NO_PROXY)); + + final HttpHost target = new HttpHost("https", "secure.example", 443); + final HttpRoute route = routePlanner.determineRoute( + target, HttpClientContext.create()); + + Assertions.assertNull(route.getProxyHost()); + }); + } + @EnabledForJreRange(max = JRE.JAVA_15) + @Test + void testEnvNoProxyExcludesHost() throws Exception { + withEnvironmentVariable("HTTP_PROXY", "http://proxy:3128") + .and("NO_PROXY", "localhost,127.0.0.1") + .execute(() -> { + Mockito.when(proxySelector.select(ArgumentMatchers.any())) + .thenReturn(Collections.singletonList(Proxy.NO_PROXY)); + + final HttpHost target = new HttpHost("http", "localhost", 80); + final HttpRoute route = routePlanner.determineRoute( + target, HttpClientContext.create()); + + Assertions.assertNull(route.getProxyHost()); + }); + } + + } From a7030db4642cf348771c37f9b4e190bcbc7e8a8d Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sat, 23 Aug 2025 15:20:05 +0200 Subject: [PATCH 3/3] Make proxy autodetection the default via SystemDefaultRoutePlanner (ProxySelector first, then HTTP(S)_PROXY/NO_PROXY). Add disableProxyAutodetection() to preserve legacy behavior. --- .../http/impl/classic/HttpClientBuilder.java | 24 ++++- .../impl/classic/TestHttpClientBuilder.java | 93 +++++++++++++++++++ 2 files changed, 112 insertions(+), 5 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java index 8a53b0129e..f9a9ac7312 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java @@ -233,6 +233,7 @@ private ExecInterceptorEntry( private boolean authCachingDisabled; private boolean connectionStateDisabled; private boolean defaultUserAgentDisabled; + private boolean proxyAutodetectionDisabled; private ProxySelector proxySelector; private List closeables; @@ -791,6 +792,19 @@ public final HttpClientBuilder disableDefaultUserAgent() { return this; } + /** + * Disables automatic proxy detection for clients created by this builder. + *

+ * When disabled, and unless an explicit proxy or route planner is configured, + * the builder falls back to {@link org.apache.hc.client5.http.impl.routing.DefaultRoutePlanner}. + *

+ * @return this instance. + */ + public final HttpClientBuilder disableProxyAutodetection() { + this.proxyAutodetectionDisabled = true; + return this; + } + /** * Sets the {@link ProxySelector} that will be used to select the proxies * to be used for establishing HTTP connections. If a non-null proxy selector is set, @@ -1012,11 +1026,11 @@ public CloseableHttpClient build() { } if (proxy != null) { routePlannerCopy = new DefaultProxyRoutePlanner(proxy, schemePortResolverCopy); - } else if (this.proxySelector != null) { - routePlannerCopy = new SystemDefaultRoutePlanner(schemePortResolverCopy, this.proxySelector); - } else if (systemProperties) { - final ProxySelector defaultProxySelector = AccessController.doPrivileged((PrivilegedAction) ProxySelector::getDefault); - routePlannerCopy = new SystemDefaultRoutePlanner(schemePortResolverCopy, defaultProxySelector); + } else if (!this.proxyAutodetectionDisabled) { + final ProxySelector effectiveSelector = this.proxySelector != null + ? this.proxySelector + : AccessController.doPrivileged((PrivilegedAction) ProxySelector::getDefault); + routePlannerCopy = new SystemDefaultRoutePlanner(schemePortResolverCopy, effectiveSelector); } else { routePlannerCopy = new DefaultRoutePlanner(schemePortResolverCopy); } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestHttpClientBuilder.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestHttpClientBuilder.java index 2cec4699e2..29979f1dfa 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestHttpClientBuilder.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestHttpClientBuilder.java @@ -27,12 +27,22 @@ package org.apache.hc.client5.http.impl.classic; import java.io.IOException; +import java.lang.reflect.Field; +import java.net.ProxySelector; import org.apache.hc.client5.http.classic.ExecChain; import org.apache.hc.client5.http.classic.ExecChainHandler; +import org.apache.hc.client5.http.impl.routing.DefaultProxyRoutePlanner; +import org.apache.hc.client5.http.impl.routing.DefaultRoutePlanner; +import org.apache.hc.client5.http.impl.routing.SystemDefaultRoutePlanner; +import org.apache.hc.client5.http.protocol.HttpClientContext; +import org.apache.hc.client5.http.routing.HttpRoutePlanner; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; class TestHttpClientBuilder { @@ -66,4 +76,87 @@ public ClassicHttpResponse execute( return chain.proceed(request, scope); } } + + @Test + void testDefaultUsesSystemDefaultRoutePlanner() throws Exception { + try (final InternalHttpClient client = (InternalHttpClient) HttpClients.custom().build()) { + final Object planner = getPrivateField(client, "routePlanner"); + Assertions.assertNotNull(planner); + Assertions.assertInstanceOf(SystemDefaultRoutePlanner.class, planner, "Default should be SystemDefaultRoutePlanner (auto-detect proxies)"); + } + } + + @Test + void testDisableProxyAutodetectionFallsBackToDefaultRoutePlanner() throws Exception { + try (final InternalHttpClient client = (InternalHttpClient) HttpClients.custom() + .disableProxyAutodetection() + .build()) { + final Object planner = getPrivateField(client, "routePlanner"); + Assertions.assertNotNull(planner); + Assertions.assertInstanceOf(DefaultRoutePlanner.class, planner, "disableProxyAutodetection() should restore DefaultRoutePlanner"); + } + } + + @Test + void testExplicitProxyWinsOverAutodetection() throws Exception { + try (final InternalHttpClient client = (InternalHttpClient) HttpClients.custom() + .setProxy(new HttpHost("http", "proxy.local", 8080)) + .build()) { + final Object planner = getPrivateField(client, "routePlanner"); + Assertions.assertNotNull(planner); + Assertions.assertInstanceOf(DefaultProxyRoutePlanner.class, planner, "Explicit proxy must take precedence"); + } + } + + @Test + void testCustomRoutePlannerIsRespected() throws Exception { + final HttpRoutePlanner custom = new HttpRoutePlanner() { + @Override + public org.apache.hc.client5.http.HttpRoute determineRoute( + final HttpHost host, final HttpContext context) { + // trivial, never used in this test + return new org.apache.hc.client5.http.HttpRoute(host); + } + }; + try (final InternalHttpClient client = (InternalHttpClient) HttpClients.custom() + .setRoutePlanner(custom) + .build()) { + final Object planner = getPrivateField(client, "routePlanner"); + Assertions.assertSame(custom, planner, "Custom route planner must be used as-is"); + } + } + + @Test + void testProvidedProxySelectorIsUsedBySystemDefaultRoutePlanner() throws Exception { + class TouchProxySelector extends ProxySelector { + volatile boolean touched = false; + @Override + public java.util.List select(final java.net.URI uri) { + touched = true; + return java.util.Collections.singletonList(java.net.Proxy.NO_PROXY); + } + @Override + public void connectFailed(final java.net.URI uri, final java.net.SocketAddress sa, final IOException ioe) { } + } + final TouchProxySelector selector = new TouchProxySelector(); + + try (final InternalHttpClient client = (InternalHttpClient) HttpClients.custom() + .setProxySelector(selector) + .build()) { + final Object planner = getPrivateField(client, "routePlanner"); + Assertions.assertInstanceOf(SystemDefaultRoutePlanner.class, planner); + + // Call determineRoute on the planner directly to avoid making a real request + final SystemDefaultRoutePlanner sdrp = (SystemDefaultRoutePlanner) planner; + sdrp.determineRoute(new HttpHost("http", "example.com", 80), HttpClientContext.create()); + + Assertions.assertTrue(selector.touched, "Provided ProxySelector should be consulted"); + } + } + + private static Object getPrivateField(final Object target, final String name) throws Exception { + final Field f = target.getClass().getDeclaredField(name); + f.setAccessible(true); + return f.get(target); + } } \ No newline at end of file