move env-proxy mapping into SystemDefaultRoutePlanner so routes honour HTTP(S)_PROXY / NO_PROXY directly without altering JVM system properties

Author: arturobernalg

httpclient5/src/main/java/org/apache/hc/client5/http/config/EnvironmentProxyConfigurer.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<Closeable> 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 <em>for this client only</em>.
-     * <p>
-     * 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.
-     * </p>
-     *
-     * <p><strong>Opt-in behaviour:</strong> if you do not call
-     * {@code useEnvironmentProxy()}, the builder leaves JVM system
-     * properties untouched.  Combine with
-     * {@link #useSystemProperties()}&nbsp;(enabled by default via
-     * {@code HttpClientBuilder.create()}) so the newly populated
-     * properties are actually honoured by the client.</p>
-     *
-     * @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<HttpContext, HttpClientContext> 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;

httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.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<Proxy> 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<String>) () -> System.getenv(key));
+    }
 
     private Proxy chooseProxy(final List<Proxy> proxies) {
         Proxy result = null;