From b2d86314802a630ce4007d82ffc3afcf8aa31dfc Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Fri, 30 Jun 2017 15:39:20 +0200 Subject: [PATCH] SONAR-9336 Fix infinite proxy authentication loop --- .../ws/client/OkHttpClientBuilder.java | 7 +- .../ws/client/HttpConnectorTest.java | 65 ++++++++++++++----- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/OkHttpClientBuilder.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/OkHttpClientBuilder.java index 77ad1552be4..a9d4985fff5 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/OkHttpClientBuilder.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/OkHttpClientBuilder.java @@ -59,6 +59,7 @@ public class OkHttpClientBuilder { private static final String NONE = "NONE"; private static final String P11KEYSTORE = "PKCS11"; + private static final String PROXY_AUTHORIZATION = "Proxy-Authorization"; private String userAgent; private Proxy proxy; @@ -160,9 +161,13 @@ public class OkHttpClientBuilder { builder.addNetworkInterceptor(this::addUserAgent); if (proxyLogin != null) { builder.proxyAuthenticator((route, response) -> { + if (response.request().header(PROXY_AUTHORIZATION) != null) { + // Give up, we've already attempted to authenticate. + return null; + } if (HttpURLConnection.HTTP_PROXY_AUTH == response.code()) { String credential = Credentials.basic(proxyLogin, nullToEmpty(proxyPassword)); - return response.request().newBuilder().header("Proxy-Authorization", credential).build(); + return response.request().newBuilder().header(PROXY_AUTHORIZATION, credential).build(); } return null; }); diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/client/HttpConnectorTest.java b/sonar-ws/src/test/java/org/sonarqube/ws/client/HttpConnectorTest.java index ba6e9d4dc43..449b4fcebe8 100644 --- a/sonar-ws/src/test/java/org/sonarqube/ws/client/HttpConnectorTest.java +++ b/sonar-ws/src/test/java/org/sonarqube/ws/client/HttpConnectorTest.java @@ -31,6 +31,7 @@ import okhttp3.mockwebserver.RecordedRequest; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -62,6 +63,11 @@ public class HttpConnectorTest { serverUrl = server.url("").url().toString(); } + @After + public void stop() throws Exception { + server.close(); + } + @Test public void test_default_settings() throws Exception { answerHelloWorld(); @@ -140,23 +146,50 @@ public class HttpConnectorTest { @Test public void use_proxy_authentication() throws Exception { - MockWebServer proxy = new MockWebServer(); - proxy.start(); - underTest = HttpConnector.newBuilder() - .url(serverUrl) - .proxy(new Proxy(Proxy.Type.HTTP, new InetSocketAddress(proxy.getHostName(), proxy.getPort()))) - .proxyCredentials("theProxyLogin", "theProxyPassword") - .build(); - - GetRequest request = new GetRequest("api/issues/search"); - proxy.enqueue(new MockResponse().setResponseCode(407)); - proxy.enqueue(new MockResponse().setBody("OK!")); - underTest.call(request); + try (MockWebServer proxy = new MockWebServer()) { + proxy.start(); + + underTest = HttpConnector.newBuilder() + .url(serverUrl) + .proxy(new Proxy(Proxy.Type.HTTP, new InetSocketAddress(proxy.getHostName(), proxy.getPort()))) + .proxyCredentials("theProxyLogin", "theProxyPassword") + .build(); + + GetRequest request = new GetRequest("api/issues/search"); + proxy.enqueue(new MockResponse().setResponseCode(407)); + proxy.enqueue(new MockResponse().setBody("OK!")); + underTest.call(request); + + RecordedRequest recordedRequest = proxy.takeRequest(); + assertThat(recordedRequest.getHeader("Proxy-Authorization")).isNull(); + recordedRequest = proxy.takeRequest(); + assertThat(recordedRequest.getHeader("Proxy-Authorization")).isEqualTo(basic("theProxyLogin", "theProxyPassword")); + } + } - RecordedRequest recordedRequest = proxy.takeRequest(); - assertThat(recordedRequest.getHeader("Proxy-Authorization")).isNull(); - recordedRequest = proxy.takeRequest(); - assertThat(recordedRequest.getHeader("Proxy-Authorization")).isEqualTo(basic("theProxyLogin", "theProxyPassword")); + @Test + public void use_proxy_authentication_wrong_crendentials() throws Exception { + try (MockWebServer proxy = new MockWebServer()) { + proxy.start(); + + underTest = HttpConnector.newBuilder() + .url(serverUrl) + .proxy(new Proxy(Proxy.Type.HTTP, new InetSocketAddress(proxy.getHostName(), proxy.getPort()))) + .proxyCredentials("theProxyLogin", "wrongPassword") + .build(); + + GetRequest request = new GetRequest("api/issues/search"); + proxy.enqueue(new MockResponse().setResponseCode(407)); + proxy.enqueue(new MockResponse().setResponseCode(407)); + proxy.enqueue(new MockResponse().setResponseCode(407)); + underTest.call(request); + + RecordedRequest recordedRequest = proxy.takeRequest(); + assertThat(recordedRequest.getHeader("Proxy-Authorization")).isNull(); + recordedRequest = proxy.takeRequest(); + assertThat(recordedRequest.getHeader("Proxy-Authorization")).isEqualTo(basic("theProxyLogin", "wrongPassword")); + assertThat(proxy.getRequestCount()).isEqualTo(2); + } } @Test -- 2.39.5