Fix disabled local proxy configuration being used over global config (#1780)

The original proxy configuration implementation only used the global configuration if the local proxy configuration was not provided (i.e. null). This PR adds the corner case where a local configuration is provided, but disabled. In this case, the global proxy configuration will be used as a fallback as well.

gradle/changelog/disabled_local_proxy_config.yaml

@@ -0,0 +1,2 @@
+- type: Fixed
+  description: Fix disabled local proxy configuration being used over global config ([#1780](https://github.com/scm-manager/scm-manager/pull/1780))

scm-core/src/main/java/sonia/scm/net/HttpURLConnectionFactory.java

@@ -129,7 +129,7 @@ public final class HttpURLConnectionFactory {
       // because we are not able to remove the authentication from thread local
       ThreadLocalAuthenticator.clear();
 
-      ProxyConfiguration proxyConfiguration = options.getProxyConfiguration().orElse(globalProxyConfiguration);
+      ProxyConfiguration proxyConfiguration = options.getProxyConfiguration().filter(ProxyConfiguration::isEnabled).orElse(globalProxyConfiguration);
       if (isProxyEnabled(proxyConfiguration, url)) {
         return openProxyConnection(proxyConfiguration, url);
       }

scm-core/src/test/java/sonia/scm/net/HttpURLConnectionFactoryTest.java

@@ -180,6 +180,22 @@ class HttpURLConnectionFactoryTest {
       verify(connection).setReadTimeout(3000);
     }
 
+    @Test
+    void shouldUseGlobalProxyConfigurationIfLocalOneIsDisabled() throws IOException {
+      configuration.setProxyServer("proxy.hitchhiker.com");
+      configuration.setProxyPort(3128);
+      configuration.setEnableProxy(true);
+
+      ScmConfiguration localProxyConf = new ScmConfiguration();
+      localProxyConf.setEnableProxy(false);
+      localProxyConf.setProxyServer("prox.hitchhiker.net");
+      localProxyConf.setProxyPort(3127);
+
+      connectionFactory.create(new URL("https://hitchhiker.org"), new HttpConnectionOptions().withProxyConfiguration(new GlobalProxyConfiguration(localProxyConf)));
+
+      assertUsedProxy("proxy.hitchhiker.com", 3128);
+    }
+
     @Test
     void shouldCreateProxyConnection() throws IOException {
       configuration.setEnableProxy(true);
@@ -341,7 +357,6 @@ class HttpURLConnectionFactoryTest {
       return captor.getValue();
     }
 
-
     private void assertUsedProxy(String host, int port) {
       assertThat(usedProxy).isNotNull();
       assertThat(usedProxy.address()).isInstanceOfSatisfying(InetSocketAddress.class, inet -> {

scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnMirrorCommand.java

@@ -147,7 +147,7 @@ public class SvnMirrorCommand extends AbstractSvnCommand implements MirrorComman
       }
     };
     checkAndApplyProxyConfiguration(
-      authManager, mirrorCommandRequest.getProxyConfiguration().orElse(globalProxyConfiguration), url
+      authManager, mirrorCommandRequest.getProxyConfiguration().filter(ProxyConfiguration::isEnabled).orElse(globalProxyConfiguration), url
     );
     return authManager;
   }

scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnMirrorCommandTest.java

@@ -53,6 +53,7 @@ import java.util.function.Consumer;
 import static java.util.Collections.singleton;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static sonia.scm.repository.api.MirrorCommandResult.ResultType.OK;
@@ -197,6 +198,18 @@ public class SvnMirrorCommandTest extends AbstractSvnCommandTestBase {
     assertThat(authenticationManager.getProxyPasswordValue()).isNull();
   }
 
+  @Test
+  public void shouldNotApplyDisabledLocalProxySettings() throws SVNException {
+    MirrorCommandRequest request = new MirrorCommandRequest();
+    request.setProxyConfiguration(createDisabledProxyConfiguration());
+
+    BasicAuthenticationManager authenticationManager = createAuthenticationManager(request);
+    assertThat(authenticationManager.getProxyHost()).isNull();
+    assertThat(authenticationManager.getProxyPort()).isZero();
+    assertThat(authenticationManager.getProxyUserName()).isNull();
+    assertThat(authenticationManager.getProxyPasswordValue()).isNull();
+  }
+
   private ProxyConfiguration createProxyConfiguration() {
     ProxyConfiguration configuration = mock(ProxyConfiguration.class);
     when(configuration.isEnabled()).thenReturn(true);
@@ -205,6 +218,14 @@ public class SvnMirrorCommandTest extends AbstractSvnCommandTestBase {
     return configuration;
   }
 
+  private ProxyConfiguration createDisabledProxyConfiguration() {
+    ProxyConfiguration configuration = mock(ProxyConfiguration.class);
+    when(configuration.isEnabled()).thenReturn(false);
+    lenient().when(configuration.getHost()).thenReturn("proxy.hitchhiker.com");
+    lenient().when(configuration.getPort()).thenReturn(3128);
+    return configuration;
+  }
+
   private BasicAuthenticationManager createAuthenticationManager() throws SVNException {
     return createAuthenticationManager(new MirrorCommandRequest());
   }