]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-14213 Do not follow redirects when interacting with gitlab.com API
authorWouter Admiraal <45544358+wouter-admiraal-sonarsource@users.noreply.github.com>
Tue, 20 Apr 2021 14:39:10 +0000 (16:39 +0200)
committersonartech <sonartech@sonarsource.com>
Tue, 20 Apr 2021 20:03:47 +0000 (20:03 +0000)
server/sonar-alm-client/src/main/java/org/sonar/alm/client/gitlab/GitlabHttpClient.java
server/sonar-alm-client/src/test/java/org/sonar/alm/client/gitlab/GitlabHttpClientTest.java
sonar-ws/src/main/java/org/sonarqube/ws/client/OkHttpClientBuilder.java
sonar-ws/src/test/java/org/sonarqube/ws/client/OkHttpClientBuilderTest.java

index 61dc5a7c0bc0ac931b4699b9cd301b403c806d7a..36011e4e96eba1f9554bf6f75a3e636fb48fc664 100644 (file)
@@ -55,6 +55,7 @@ public class GitlabHttpClient {
     client = new OkHttpClientBuilder()
       .setConnectTimeoutMs(timeoutConfiguration.getConnectTimeout())
       .setReadTimeoutMs(timeoutConfiguration.getReadTimeout())
+      .setFollowRedirects(false)
       .build();
   }
 
@@ -167,6 +168,8 @@ public class GitlabHttpClient {
         throw new IllegalArgumentException("Your GitLab token has insufficient scope");
       } else if (response.code() == HTTP_UNAUTHORIZED) {
         throw new IllegalArgumentException("Invalid personal access token");
+      } else if (response.isRedirect()) {
+        throw new IllegalArgumentException("Request was redirected, please provide the correct URL");
       } else {
         throw new IllegalArgumentException(errorMessage);
       }
index 1a67af5818b5ab74cead169df7792cc0693ee6d9..228d21a2349051231ebaae50e77cde834e0f6ecf 100644 (file)
@@ -99,6 +99,17 @@ public class GitlabHttpClientTest {
       .hasMessage("Invalid personal access token");
   }
 
+  @Test
+  public void should_throw_IllegalArgumentException_when_redirected() {
+    MockResponse response = new MockResponse()
+      .setResponseCode(308);
+    server.enqueue(response);
+
+    assertThatThrownBy(() -> underTest.searchProjects(gitlabUrl, "pat", "example", 1, 2))
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Request was redirected, please provide the correct URL");
+  }
+
   @Test
   public void get_project() {
     MockResponse response = new MockResponse()
index ede86381037999bf15913a9f9adb1eca1c430e14..aa3a736eb4cb554e64b4ee443eb27acf6b0c1f85 100644 (file)
@@ -67,6 +67,7 @@ public class OkHttpClientBuilder {
   private String credentials;
   private String proxyLogin;
   private String proxyPassword;
+  private Boolean followRedirects;
   private long connectTimeoutMs = -1;
   private long readTimeoutMs = -1;
   private SSLSocketFactory sslSocketFactory = null;
@@ -159,6 +160,15 @@ public class OkHttpClientBuilder {
     return this;
   }
 
+  /**
+   * Set if redirects should be followed or not.
+   * Default is defined by OkHttp (true, follow redirects).
+   */
+  public OkHttpClientBuilder setFollowRedirects(Boolean followRedirects) {
+    this.followRedirects = followRedirects;
+    return this;
+  }
+
   public OkHttpClient build() {
     OkHttpClient.Builder builder = new OkHttpClient.Builder();
     builder.proxy(proxy);
@@ -182,6 +192,10 @@ public class OkHttpClientBuilder {
         return null;
       });
     }
+    if (followRedirects != null) {
+      builder.followRedirects(followRedirects);
+      builder.followSslRedirects(followRedirects);
+    }
 
     ConnectionSpec tls = new ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS)
       .allEnabledTlsVersions()
index 26c66de6af5adccb587f25e6de8c9bf069eb8807..047cfbfc205ebd416ab621c92cd769bc2281b1aa 100644 (file)
@@ -42,6 +42,8 @@ public class OkHttpClientBuilderTest {
     assertThat(okHttpClient.proxy()).isNull();
     assertThat(okHttpClient.networkInterceptors()).hasSize(1);
     assertThat(okHttpClient.sslSocketFactory()).isNotNull();
+    assertThat(okHttpClient.followRedirects()).isTrue();
+    assertThat(okHttpClient.followSslRedirects()).isTrue();
   }
 
   @Test
@@ -54,6 +56,23 @@ public class OkHttpClientBuilderTest {
     assertThat(okHttpClient.sslSocketFactory()).isEqualTo(sslSocketFactory);
   }
 
+  @Test
+  public void build_follow_redirects() {
+    OkHttpClient okHttpClientWithRedirect = underTest
+      .setFollowRedirects(true)
+      .build();
+
+    assertThat(okHttpClientWithRedirect.followRedirects()).isTrue();
+    assertThat(okHttpClientWithRedirect.followSslRedirects()).isTrue();
+
+    OkHttpClient okHttpClientWithoutRedirect = underTest
+      .setFollowRedirects(false)
+      .build();
+
+    assertThat(okHttpClientWithoutRedirect.followRedirects()).isFalse();
+    assertThat(okHttpClientWithoutRedirect.followSslRedirects()).isFalse();
+  }
+
   @Test
   public void build_throws_IAE_if_connect_timeout_is_negative() {
     expectedException.expect(IllegalArgumentException.class);