NEXUS-8718: More fixes for HTTP Proxy handling

Changes:
- removed "default" non proxy hosts
- removed sync-to-system-props
- update of label

components/nexus-httpclient/src/main/java/org/sonatype/nexus/httpclient/config/ConfigurationCustomizer.java

@@ -25,6 +25,7 @@
 
 import javax.annotation.Nullable;
 
+import org.sonatype.nexus.common.text.Strings2;
 import org.sonatype.nexus.httpclient.HttpClientPlan;
 import org.sonatype.nexus.httpclient.SSLContextSelector;
 import org.sonatype.nexus.httpclient.internal.NexusHttpRoutePlanner;
@@ -61,13 +62,6 @@ public class ConfigurationCustomizer
     extends ComponentSupport
     implements HttpClientPlan.Customizer
 {
-  /**
-   * Default non-proxyHosts value, used by Java7 too. Always appended to the user given list, if any.
-   */
-  private static final List<String> DEFAULT_NO_PROXY_HOSTS_PATTERN_STRINGS = ImmutableList.of(
-      "localhost", "127.*", "[::1]", "0.0.0.0", "[::0]"
-  );
-
   /**
    * Simple reusable function that converts "glob-like" expressions to regexp.
    */
@@ -81,16 +75,6 @@ public String apply(final String input) {
     }
   };
 
-  /**
-   * Regexp pattern used as default or as fallback when user pattern cannot be converted to regexp. Built from
-   * {@link #DEFAULT_NO_PROXY_HOSTS_PATTERN_STRINGS}.
-   */
-  private static final Pattern DEFAULT_NO_PROXY_HOSTS_PATTERN =
-      Pattern.compile(Joiner.on("|").join(
-              Iterables.transform(DEFAULT_NO_PROXY_HOSTS_PATTERN_STRINGS, GLOB_STRING_TO_REGEXP_STRING)
-          )
-      );
-
   static {
     /**
      * Install custom {@link Authenticator} for proxy.
@@ -215,86 +199,17 @@ NexusHttpRoutePlanner createRoutePlanner(final ProxyConfiguration proxy) {
     if (proxy.getNonProxyHosts() != null) {
       patterns.addAll(Arrays.asList(proxy.getNonProxyHosts()));
     }
-    patterns.addAll(DEFAULT_NO_PROXY_HOSTS_PATTERN_STRINGS);
     String nonProxyPatternString = Joiner.on("|").join(Iterables.transform(patterns, GLOB_STRING_TO_REGEXP_STRING));
-    Pattern nonProxyPattern;
-    try {
-      nonProxyPattern = Pattern.compile(nonProxyPatternString, Pattern.CASE_INSENSITIVE);
-    }
-    catch (PatternSyntaxException e) {
-      log.warn("Invalid non-proxy host regex: {}, using defaults", nonProxyPatternString, e);
-      nonProxyPattern = DEFAULT_NO_PROXY_HOSTS_PATTERN;
-    }
-    syncHttpSystemProperties(proxy);
-    return new NexusHttpRoutePlanner(proxies, nonProxyPattern);
-  }
-
-  private void syncHttpSystemProperties(final ProxyConfiguration proxy) {
-    // HTTP proxy
-    ProxyServerConfiguration http = proxy.getHttp();
-    if (http != null && http.isEnabled()) {
-      System.setProperty("http.proxyHost", http.getHost());
-      System.setProperty("http.proxyPort", Integer.toString(http.getPort()));
-      if (http.getAuthentication() != null) {
-        if (http.getAuthentication() instanceof UsernameAuthenticationConfiguration) {
-          UsernameAuthenticationConfiguration usernamePassword = (UsernameAuthenticationConfiguration) http
-              .getAuthentication();
-          System.setProperty("http.proxyUser", usernamePassword.getUsername());
-          System.setProperty("http.proxyPassword", usernamePassword.getPassword());
-        }
-        else {
-          log.warn("Authentication {} not supported for Java Networking, system properties not set",
-              http.getAuthentication().getClass().getSimpleName());
-        }
+    Pattern nonProxyPattern = null;
+    if (!Strings2.isBlank(nonProxyPatternString)) {
+      try {
+        nonProxyPattern = Pattern.compile(nonProxyPatternString, Pattern.CASE_INSENSITIVE);
       }
-      else {
-        System.clearProperty("http.proxyUser");
-        System.clearProperty("http.proxyPassword");
+      catch (PatternSyntaxException e) {
+        log.warn("Invalid non-proxy host regex: {}, using defaults", nonProxyPatternString, e);
       }
     }
-    else {
-      System.clearProperty("http.proxyHost");
-      System.clearProperty("http.proxyPort");
-      System.clearProperty("http.proxyUser");
-      System.clearProperty("http.proxyPassword");
-    }
-
-    // HTTPS proxy
-    ProxyServerConfiguration https = proxy.getHttps();
-    if (https != null && https.isEnabled()) {
-      System.setProperty("https.proxyHost", https.getHost());
-      System.setProperty("https.proxyPort", Integer.toString(https.getPort()));
-      if (https.getAuthentication() != null) {
-        if (https.getAuthentication() instanceof UsernameAuthenticationConfiguration) {
-          UsernameAuthenticationConfiguration usernamePassword = (UsernameAuthenticationConfiguration) https
-              .getAuthentication();
-          System.setProperty("https.proxyUser", usernamePassword.getUsername());
-          System.setProperty("https.proxyPassword", usernamePassword.getPassword());
-        }
-        else {
-          log.warn("Authentication {} not supported for Java Networking, system properties not set",
-              https.getAuthentication().getClass().getSimpleName());
-        }
-      }
-      else {
-        System.clearProperty("https.proxyUser");
-        System.clearProperty("https.proxyPassword");
-      }
-    }
-    else {
-      System.clearProperty("https.proxyHost");
-      System.clearProperty("https.proxyPort");
-      System.clearProperty("https.proxyUser");
-      System.clearProperty("https.proxyPassword");
-    }
-
-    // nonProxyHosts
-    if (proxy.getNonProxyHosts() != null) {
-      System.setProperty("http.nonProxyHosts", Joiner.on("|").join(proxy.getNonProxyHosts()));
-    }
-    else {
-      System.clearProperty("http.nonProxyHosts");
-    }
+    return new NexusHttpRoutePlanner(proxies, nonProxyPattern);
   }
 
   /**

components/nexus-httpclient/src/main/java/org/sonatype/nexus/httpclient/config/NonProxyHostsValidator.java

@@ -12,12 +12,13 @@
  */
 package org.sonatype.nexus.httpclient.config;
 
+import java.util.regex.Pattern;
+
 import javax.validation.ConstraintValidatorContext;
 
+import org.sonatype.nexus.common.text.Strings2;
 import org.sonatype.nexus.validation.ConstraintValidatorSupport;
 
-import com.google.common.base.Strings;
-
 /**
  * {@link NonProxyHosts} validator.
  *
@@ -26,6 +27,22 @@
 public class NonProxyHostsValidator
     extends ConstraintValidatorSupport<NonProxyHosts, String[]>
 {
+  /**
+   * Pattern checking for allowed characters, best we can do, as input may be:
+   * <ul>
+   * <li>hostname w/o or w/ wildcard (incomplete)</li>
+   * <li>IPv4 address w/o or w/ wildcard (incomplete)</li>
+   * <li>IPv6 non-compressed address w/o or w/ wildcard (incomplete)</li>
+   * <li>IPv6 compressed address w/o or w/ wildcard (incomplete)</li>
+   * </ul>
+   * Due to the "incomplete" case (hostname may be missing domain, IPv4 might have one, two, three or four segments,
+   * IPv6 might have three or more segments) the validation we perform here is basically just enforcing allowed
+   * characters, and simply not treating these as hostname or IP address, but merely as an opaque pattern.
+   * If wildcard present, wildcard in a nonProxyHost element may be only on it's beginning or end, nowhere else.
+   */
+  private static final Pattern CONTENT_PATTERN = Pattern
+      .compile("\\*?[\\p{IsAlphabetic}|\\d|\\-|\\_|\\.|\\:|\\[|\\]]+\\*?");
+
   @Override
   public boolean isValid(final String[] values, final ConstraintValidatorContext context) {
     for (String value : values) {
@@ -40,20 +57,17 @@ public boolean isValid(final String[] values, final ConstraintValidatorContext c
    * Returns {@code true} if value is considered as valid nonProxyHosts expression. This is NOT validating the
    * single-string used to set system property (where expressions are delimited with "|")!
    */
-  private boolean isValid(final String value) {
+  private boolean isValid(String value) {
     // A value should be a non-empty string optionally prefixed or suffixed with an asterisk
-    if (Strings.isNullOrEmpty(value)) {
-      // must be non-empty
-      return false;
-    }
-    if (value.contains("|")) {
-      // must not contain | separator (used to separate multiple values in system properties)
+    // must be non-empty, non-blank
+    if (Strings2.isBlank(value)) {
       return false;
     }
-    if (value.contains("*") && !(value.startsWith("*") || value.endsWith("*"))) {
-      // if contains asterisk, it must be at beginning or end only
+    // must not contain | separator (used to separate multiple values in system properties)
+    if (value.indexOf('|') > -1) {
       return false;
     }
-    return true;
+    // asterisk '*' can be only on beginning or end
+    return CONTENT_PATTERN.matcher(value).matches();
   }
 }

components/nexus-httpclient/src/main/java/org/sonatype/nexus/httpclient/internal/NexusHttpRoutePlanner.java

@@ -15,6 +15,8 @@
 import java.util.Map;
 import java.util.regex.Pattern;
 
+import javax.annotation.Nullable;
+
 import org.apache.http.HttpException;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpRequest;
@@ -50,11 +52,11 @@ public class NexusHttpRoutePlanner
    * @since 2.5
    */
   public NexusHttpRoutePlanner(final Map<String, HttpHost> proxies,
-                               final Pattern nonProxyHostsPattern)
+                               @Nullable final Pattern nonProxyHostsPattern)
   {
     super(DefaultSchemePortResolver.INSTANCE);
     this.proxies = checkNotNull(proxies);
-    this.nonProxyHostPattern = checkNotNull(nonProxyHostsPattern);
+    this.nonProxyHostPattern = nonProxyHostsPattern;
   }
 
   @Override
@@ -77,6 +79,6 @@ protected HttpHost determineProxy(final HttpHost host,
    * @return true if no proxy should be configured.
    */
   private boolean noProxyFor(final String hostName) {
-    return nonProxyHostPattern.matcher(hostName).matches();
+    return nonProxyHostPattern != null && nonProxyHostPattern.matcher(hostName).matches();
   }
 }

components/nexus-httpclient/src/test/java/org/sonatype/nexus/httpclient/config/ConfigurationCustomizerTest.groovy

@@ -60,45 +60,69 @@ class ConfigurationCustomizerTest
 
     HttpHost localhost = new HttpHost('localhost', 80)
     route = planner.determineRoute(localhost, mock(HttpRequest), mock(HttpContext))
-    assertThat(route.getHopTarget(0), equalTo(localhost))
+    assertThat(route.getHopTarget(0), equalTo(httpProxyHost))
   }
 
   @Test
-  void 'default nonProxyHosts'() {
-    NexusHttpRoutePlanner planner = create(null)
+  void 'custom nonProxyHosts'() {
+    NexusHttpRoutePlanner planner = create(['*.sonatype.*', '*.example.com', 'localhost', '10.*', '[:*', '*:8]'] as String[])
     HttpRoute route
 
-    for (String host : ['localhost', '127.0.0.1', '[::1]', '0.0.0.0', '[::0]']) {
-      HttpHost target = new HttpHost(host, 80, HTTP)
+    // must not have proxy used
+    for (String host : ['www.sonatype.org', 'www.sonatype.com', 'smtp.example.com', 'localhost', '10.0.0.1', '[::8]', '[::9]']) {
+      HttpHost target = new HttpHost(host, 80)
       route = planner.determineRoute(target, mock(HttpRequest), mock(HttpContext))
       assertThat(route.getHopTarget(0), equalTo(target))
     }
+
+    // must have HTTP proxy used
+    for (String host : ['www.thesonatype.com', 'www.google.com', 'example.com', 'example.org']) {
+      HttpHost target = new HttpHost(host, 80, HTTP)
+      route = planner.determineRoute(target, mock(HttpRequest), mock(HttpContext))
+      assertThat(route.getHopTarget(0), equalTo(httpProxyHost))
+    }
+
+    // must have HTTPS proxy used
+    for (String host : ['www.google.com', 'example.com', 'example.org']) {
+      HttpHost target = new HttpHost(host, 8443, HTTPS)
+      route = planner.determineRoute(target, mock(HttpRequest), mock(HttpContext))
+      assertThat(route.getHopTarget(0), equalTo(httpsProxyHost))
+    }
   }
 
   @Test
-  void 'custom nonProxyHosts'() {
-    NexusHttpRoutePlanner planner = create(['*.sonatype.*', '*.example.com'] as String[])
+  void 'ipv6 nonProxyHosts'() {
+    NexusHttpRoutePlanner planner
     HttpRoute route
 
+    planner = create(['*:8]'] as String[])
     // must not have proxy used
-    for (String host : ['www.sonatype.org', 'www.sonatype.com', 'smtp.example.com', 'localhost', '127.0.0.1', '[::1]', '0.0.0.0', '[::0]']) {
+    for (String host : ['[::8]']) {
       HttpHost target = new HttpHost(host, 80)
       route = planner.determineRoute(target, mock(HttpRequest), mock(HttpContext))
       assertThat(route.getHopTarget(0), equalTo(target))
     }
 
     // must have HTTP proxy used
-    for (String host : ['www.thesonatype.com', 'www.google.com', 'example.com', 'example.org']) {
+    for (String host : ['[::9]']) {
       HttpHost target = new HttpHost(host, 80, HTTP)
       route = planner.determineRoute(target, mock(HttpRequest), mock(HttpContext))
       assertThat(route.getHopTarget(0), equalTo(httpProxyHost))
     }
 
-    // must have HTTPS proxy used
-    for (String host : ['www.google.com', 'example.com', 'example.org']) {
-      HttpHost target = new HttpHost(host, 8443, HTTPS)
+    planner = create(['[:*'] as String[])
+    // must not have proxy used
+    for (String host : ['[::8]', '[::9]']) {
+      HttpHost target = new HttpHost(host, 80)
       route = planner.determineRoute(target, mock(HttpRequest), mock(HttpContext))
-      assertThat(route.getHopTarget(0), equalTo(httpsProxyHost))
+      assertThat(route.getHopTarget(0), equalTo(target))
+    }
+
+    // must have HTTP proxy used
+    for (String host : ['sonatype.org', '1.2.3.4']) {
+      HttpHost target = new HttpHost(host, 80, HTTP)
+      route = planner.determineRoute(target, mock(HttpRequest), mock(HttpContext))
+      assertThat(route.getHopTarget(0), equalTo(httpProxyHost))
     }
   }
 }

components/nexus-httpclient/src/test/java/org/sonatype/nexus/httpclient/config/NonProxyHostsValidatorTest.groovy

@@ -0,0 +1,69 @@
+/*
+ * Sonatype Nexus (TM) Open Source Version
+ * Copyright (c) 2008-present Sonatype, Inc.
+ * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
+ *
+ * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
+ * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
+ *
+ * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
+ * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
+ * Eclipse Foundation. All other trademarks are the property of their respective owners.
+ */
+package org.sonatype.nexus.httpclient.config
+
+import javax.validation.ConstraintValidatorContext
+
+import org.sonatype.sisu.litmus.testsupport.TestSupport
+
+import org.junit.Test
+import org.mockito.Mock
+
+import static org.hamcrest.MatcherAssert.assertThat
+import static org.hamcrest.Matchers.equalTo
+
+/**
+ * Tests for {@link NonProxyHostsValidator}.
+ */
+class NonProxyHostsValidatorTest
+    extends TestSupport
+{
+  @Mock
+  ConstraintValidatorContext context
+
+  NonProxyHostsValidator validator = new NonProxyHostsValidator()
+
+  private validateAndExpect(String expression, boolean expected) {
+    assertThat(validator.isValid([expression].toArray(new String[0]), context), equalTo(expected))
+  }
+
+  @Test
+  void 'validation positive test'() {
+    validateAndExpect('sonatype.org', true)
+    validateAndExpect('*.sonatype.org', true)
+    validateAndExpect('*.sonatype.*', true)
+    validateAndExpect('1.2.3.4', true)
+    validateAndExpect('*.2.3.4', true)
+    validateAndExpect('1.2.3.*', true)
+    validateAndExpect('*.2.3.*', true)
+    validateAndExpect('10.*', true)
+    validateAndExpect('*.10', true)
+    validateAndExpect('csétamás.hu', true)
+    validateAndExpect('2001:db8:85a3:8d3:1319:8a2e:370:7348', true)
+    validateAndExpect('[2001:db8:85a3:8d3:1319:8a2e:370:7348]', true)
+    validateAndExpect('[::1]', true)
+    validateAndExpect('*:8]', true)
+    validateAndExpect('[:*', true)
+    validateAndExpect('localhost', true)
+  }
+
+  @Test
+  void 'validation negative test'() {
+    // these below are the "best effort" we can rule out
+    validateAndExpect('', false)
+    validateAndExpect('  ', false)
+    validateAndExpect('foo|sonatype.org', false)
+    validateAndExpect('comma,com', false)
+    validateAndExpect('[*:8]', false)
+    }
+}

plugins/rapture/nexus-coreui-plugin/src/main/resources/static/rapture/NX/coreui/app/PluginStrings.js

@@ -742,7 +742,7 @@ Ext.define('NX.coreui.app.PluginStrings', {
     System_HttpSettings_ProxyPort_FieldLabel: 'HTTP proxy port',
     System_HttpSettings_Authentication_Title: 'Authentication',
     System_HttpSettings_ExcludeHosts_FieldLabel: 'Hosts to exclude from HTTP/HTTPS proxy',
-    System_HttpSettings_ExcludeHosts_HelpText: 'Java http.nonProxyHosts glob-like expressions are supported (do not use \'|\' delimiter)',
+    System_HttpSettings_ExcludeHosts_HelpText: 'Accepts Java "http.nonProxyHosts" wildcard patterns (one per line, no \'|\' hostname delimiters)',
     System_HttpSettings_HttpsProxy_Title: 'HTTPS proxy',
     System_HttpSettings_HttpsProxyHost_FieldLabel: 'HTTPS proxy host',
     System_HttpSettings_HttpsProxyHost_HelpText: 'No https:// required (e.g. "proxy-host" or "192.168.1.101")',