SONAR-8351 Configure HTTP proxy URL from bootstrap process

Proxy is configured in conf/sonar.properties. The related properties
are propagated to system properties by the first call  to DefaultHttpDownloader.
The correct approach is to directly configure the JVM properties when
the bootstrap process creates web server and CE processes.

2 files changed, 12 insertions, 94 deletions

diff --git a/sonar-core/src/main/java/org/sonar/core/util/DefaultHttpDownloader.java b/sonar-core/src/main/java/org/sonar/core/util/DefaultHttpDownloader.java
index ed174aff4c9..fe8e60475fc 100644
--- a/sonar-core/src/main/java/org/sonar/core/util/DefaultHttpDownloader.java
+++ b/sonar-core/src/main/java/org/sonar/core/util/DefaultHttpDownloader.java
@@ -22,7 +22,6 @@ package org.sonar.core.util;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.common.io.ByteStreams;
 import java.io.File;
@@ -75,7 +74,7 @@ public class DefaultHttpDownloader extends HttpDownloader {
   public DefaultHttpDownloader(Server server, Settings settings, @Nullable Integer connectTimeout, @Nullable Integer readTimeout) {
     this.readTimeout = readTimeout;
     this.connectTimeout = connectTimeout;
-    downloader = new BaseHttpDownloader(new SystemFacade(), settings, server.getVersion());
+    downloader = new BaseHttpDownloader(new AuthenticatorFacade(), settings, server.getVersion());
   }
 
   public DefaultHttpDownloader(Settings settings) {
@@ -89,7 +88,7 @@ public class DefaultHttpDownloader extends HttpDownloader {
   public DefaultHttpDownloader(Settings settings, @Nullable Integer connectTimeout, @Nullable Integer readTimeout) {
     this.readTimeout = readTimeout;
     this.connectTimeout = connectTimeout;
-    downloader = new BaseHttpDownloader(new SystemFacade(), settings, null);
+    downloader = new BaseHttpDownloader(new AuthenticatorFacade(), settings, null);
   }
 
   @Override
@@ -158,16 +157,10 @@ public class DefaultHttpDownloader extends HttpDownloader {
   }
 
   /**
-   * Facade to allow unit tests to verify calls to {@link System}.
-   * This class could be replaced by {@link org.sonar.api.utils.System2},
-   * but it sounds overkill to define {@link #setDefaultAuthenticator(Authenticator)}.
+   * Facade to allow unit tests to verify calls to {@link Authenticator#setDefault(Authenticator)}.
    */
-  static class SystemFacade {
-    public void setProperty(String key, String value) {
-      System.setProperty(key, value);
-    }
-
-    public void setDefaultAuthenticator(Authenticator authenticator) {
+  static class AuthenticatorFacade {
+    void setDefaultAuthenticator(Authenticator authenticator) {
       Authenticator.setDefault(authenticator);
     }
   }
@@ -177,34 +170,15 @@ public class DefaultHttpDownloader extends HttpDownloader {
     private static final String GET = "GET";
     private static final String HTTP_PROXY_USER = "http.proxyUser";
     private static final String HTTP_PROXY_PASSWORD = "http.proxyPassword";
-    private static final String HTTP_PROXY_HOST = "http.proxyHost";
-    private static final String HTTPS_PROXY_HOST = "https.proxyHost";
-    private static final String HTTP_PROXY_PORT = "http.proxyPort";
-    private static final String HTTPS_PROXY_PORT = "https.proxyPort";
-
-    private static final List<String> PROXY_SETTINGS = ImmutableList.of(
-      HTTP_PROXY_HOST, HTTP_PROXY_PORT, "http.nonProxyHosts",
-      HTTPS_PROXY_HOST, HTTPS_PROXY_PORT,
-      "http.auth.ntlm.domain", "socksProxyHost", "socksProxyPort");
 
     private String userAgent;
 
-    BaseHttpDownloader(SystemFacade system, Settings settings, @Nullable String userAgent) {
+    BaseHttpDownloader(AuthenticatorFacade system, Settings settings, @Nullable String userAgent) {
       initProxy(system, settings);
       initUserAgent(userAgent, settings);
     }
 
-    private void initProxy(SystemFacade system, Settings settings) {
-      // propagate system properties
-      for (String key : PROXY_SETTINGS) {
-        if (settings.hasKey(key)) {
-          system.setProperty(key, settings.getString(key));
-        }
-      }
-      // defaults of HTTPS properties are the values of HTTP properties
-      setSystemPropertyToDefaultIfNotSet(system, settings, HTTPS_PROXY_HOST, HTTP_PROXY_HOST);
-      setSystemPropertyToDefaultIfNotSet(system, settings, HTTPS_PROXY_PORT, HTTP_PROXY_PORT);
-
+    private void initProxy(AuthenticatorFacade system, Settings settings) {
       // register credentials
       String login = settings.getString(HTTP_PROXY_USER);
       if (isNotEmpty(login)) {
@@ -212,12 +186,6 @@ public class DefaultHttpDownloader extends HttpDownloader {
       }
     }
 
-    private static void setSystemPropertyToDefaultIfNotSet(SystemFacade system, Settings settings, String httpsProperty, String httpProperty) {
-      if (!settings.hasKey(httpsProperty) && settings.hasKey(httpProperty)) {
-        system.setProperty(httpsProperty, settings.getString(httpProperty));
-      }
-    }
-
     private void initUserAgent(@Nullable String sonarVersion, Settings settings) {
       String serverId = settings.getString(CoreProperties.SERVER_ID);
       userAgent = sonarVersion == null ? "SonarQube" : String.format("SonarQube %s # %s", sonarVersion, Optional.ofNullable(serverId).orElse(""));
diff --git a/sonar-core/src/test/java/org/sonar/core/util/DefaultHttpDownloaderTest.java b/sonar-core/src/test/java/org/sonar/core/util/DefaultHttpDownloaderTest.java
index fd90f8c6768..872094de53e 100644
--- a/sonar-core/src/test/java/org/sonar/core/util/DefaultHttpDownloaderTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/util/DefaultHttpDownloaderTest.java
@@ -61,11 +61,8 @@ import org.sonar.api.utils.SonarException;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.internal.matchers.ThrowableCauseMatcher.hasCause;
 import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.argThat;
-import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -289,41 +286,8 @@ public class DefaultHttpDownloaderTest {
   }
 
   @Test
-  public void configure_http_and_https_proxies() {
-    DefaultHttpDownloader.SystemFacade system = mock(DefaultHttpDownloader.SystemFacade.class);
-    Settings settings = new MapSettings();
-    settings.setProperty("http.proxyHost", "1.2.3.4");
-    settings.setProperty("http.proxyPort", "80");
-    settings.setProperty("https.proxyHost", "5.6.7.8");
-    settings.setProperty("https.proxyPort", "443");
-
-    new DefaultHttpDownloader.BaseHttpDownloader(system, settings, null);
-
-    verify(system).setProperty("http.proxyHost", "1.2.3.4");
-    verify(system).setProperty("http.proxyPort", "80");
-    verify(system).setProperty("https.proxyHost", "5.6.7.8");
-    verify(system).setProperty("https.proxyPort", "443");
-    verify(system, never()).setDefaultAuthenticator(any(Authenticator.class));
-  }
-
-  @Test
-  public void https_defaults_are_http_properties() {
-    DefaultHttpDownloader.SystemFacade system = mock(DefaultHttpDownloader.SystemFacade.class);
-    Settings settings = new MapSettings();
-    settings.setProperty("http.proxyHost", "1.2.3.4");
-    settings.setProperty("http.proxyPort", "80");
-
-    new DefaultHttpDownloader.BaseHttpDownloader(system, settings, null);
-
-    verify(system).setProperty("http.proxyHost", "1.2.3.4");
-    verify(system).setProperty("http.proxyPort", "80");
-    verify(system).setProperty("https.proxyHost", "1.2.3.4");
-    verify(system).setProperty("https.proxyPort", "80");
-  }
-
-  @Test
   public void configure_http_proxy_credentials() {
-    DefaultHttpDownloader.SystemFacade system = mock(DefaultHttpDownloader.SystemFacade.class);
+    DefaultHttpDownloader.AuthenticatorFacade system = mock(DefaultHttpDownloader.AuthenticatorFacade.class);
     Settings settings = new MapSettings();
     settings.setProperty("https.proxyHost", "1.2.3.4");
     settings.setProperty("http.proxyUser", "the_login");
@@ -346,23 +310,9 @@ public class DefaultHttpDownloaderTest {
     }));
   }
 
-  @Test
-  public void no_http_proxy_settings_by_default() {
-    DefaultHttpDownloader.SystemFacade system = mock(DefaultHttpDownloader.SystemFacade.class);
-    Settings settings = new MapSettings();
-    new DefaultHttpDownloader.BaseHttpDownloader(system, settings, null);
-
-    verify(system, never()).setProperty(eq("http.proxyHost"), anyString());
-    verify(system, never()).setProperty(eq("https.proxyHost"), anyString());
-    verify(system, never()).setProperty(eq("http.proxyPort"), anyString());
-    verify(system, never()).setProperty(eq("https.proxyPort"), anyString());
-    verify(system, never()).setDefaultAuthenticator(any(Authenticator.class));
-  }
-
-}
-
-class FakeProxy extends Proxy {
-  public FakeProxy() {
-    super(Type.HTTP, new InetSocketAddress("123.45.67.89", 4040));
+  private static class FakeProxy extends Proxy {
+    FakeProxy() {
+      super(Type.HTTP, new InetSocketAddress("123.45.67.89", 4040));
+    }
   }
 }