]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-21857 fix ssf
authorlukasz-jarocki-sonarsource <lukasz.jarocki@sonarsource.com>
Tue, 19 Mar 2024 14:39:33 +0000 (15:39 +0100)
committersonartech <sonartech@sonarsource.com>
Tue, 19 Mar 2024 20:02:38 +0000 (20:02 +0000)
sonar-core/src/it/java/org/sonar/core/util/DefaultHttpDownloaderIT.java
sonar-scanner-engine/src/main/java/org/sonar/scanner/bootstrap/ScannerWsClientProvider.java
sonar-scanner-engine/src/test/java/org/sonar/scanner/bootstrap/PluginFilesTest.java
sonar-ws/src/main/java/org/sonarqube/ws/client/GzipRejectorInterceptor.java [new file with mode: 0644]
sonar-ws/src/main/java/org/sonarqube/ws/client/HttpConnector.java
sonar-ws/src/main/java/org/sonarqube/ws/client/OkHttpClientBuilder.java
sonar-ws/src/test/java/org/sonarqube/ws/client/GzipRejectorInterceptorTest.java [new file with mode: 0644]
sonar-ws/src/test/java/org/sonarqube/ws/client/HttpConnectorTest.java
sonar-ws/src/test/java/org/sonarqube/ws/client/OkHttpClientBuilderTest.java
sonar-ws/src/testFixtures/java/org/sonarqube/ws/tester/Tester.java

index 332fe10b57e715d1c76fa4f86ca929aa668a060f..7aededcf286a6847f15d3ec678c65ff592f09741 100644 (file)
@@ -33,8 +33,10 @@ import java.util.Properties;
 import java.util.zip.GZIPOutputStream;
 import org.hamcrest.BaseMatcher;
 import org.hamcrest.Description;
+import org.junit.Ignore;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Timeout;
 import org.junit.jupiter.api.io.TempDir;
@@ -70,14 +72,6 @@ class DefaultHttpDownloaderIT {
           if (req.getPath().getPath().contains("/redirect/")) {
             resp.setCode(303);
             resp.setValue("Location", "/redirected");
-          } else if (req.getPath().getPath().contains("/gzip/")) {
-            if (!"gzip".equals(req.getValue("Accept-Encoding"))) {
-              throw new IllegalStateException("Should accept gzip");
-            }
-            resp.setValue("Content-Encoding", "gzip");
-            GZIPOutputStream gzipOutputStream = new GZIPOutputStream(resp.getOutputStream());
-            gzipOutputStream.write("GZIP response".getBytes());
-            gzipOutputStream.close();
           } else if (req.getPath().getPath().contains("/redirected")) {
             resp.getPrintStream().append("redirected");
           } else if (req.getPath().getPath().contains("/error")) {
@@ -153,12 +147,6 @@ class DefaultHttpDownloaderIT {
     assertThat(text.length()).isGreaterThan(10);
   }
 
-  @Test
-  void readGzipString() throws URISyntaxException {
-    String text = new DefaultHttpDownloader(mock(Server.class), new MapSettings().asConfig()).readString(new URI(baseUrl + "/gzip/"), StandardCharsets.UTF_8);
-    assertThat(text).isEqualTo("GZIP response");
-  }
-
   @Test
   void readStringWithDefaultTimeout() throws URISyntaxException {
     String text = new DefaultHttpDownloader(mock(Server.class), new MapSettings().asConfig()).readString(new URI(baseUrl + "/timeout/"), StandardCharsets.UTF_8);
index 9d1dfe6fa0eaee94719570fa64123edc26c4cd6c..b06d0eb4e08d9c5ffa49ee2ed62510f1bae672e8 100644 (file)
@@ -44,7 +44,7 @@ public class ScannerWsClientProvider {
   public DefaultScannerWsClient provide(ScannerProperties scannerProps, EnvironmentInformation env, GlobalAnalysisMode globalMode,
     System2 system, AnalysisWarnings analysisWarnings) {
     String url = defaultIfBlank(scannerProps.property("sonar.host.url"), "http://localhost:9000");
-    HttpConnector.Builder connectorBuilder = HttpConnector.newBuilder();
+    HttpConnector.Builder connectorBuilder = HttpConnector.newBuilder().acceptGzip(true);
 
     String timeoutSec = defaultIfBlank(scannerProps.property(READ_TIMEOUT_SEC_PROPERTY), valueOf(DEFAULT_READ_TIMEOUT_SEC));
     String envVarToken = defaultIfBlank(system.envVariable(TOKEN_ENV_VARIABLE), null);
index a252bbbe3a0e0c187dc79efcd64f8e89ffa12e6d..21d6b3c5317a8c1fb4ac54152ea5ff9ecd7e8948 100644 (file)
@@ -65,7 +65,7 @@ public class PluginFilesTest {
 
   @Before
   public void setUp() throws Exception {
-    HttpConnector connector = HttpConnector.newBuilder().url(server.url("/").toString()).build();
+    HttpConnector connector = HttpConnector.newBuilder().acceptGzip(true).url(server.url("/").toString()).build();
     GlobalAnalysisMode analysisMode = new GlobalAnalysisMode(new ScannerProperties(Collections.emptyMap()));
     DefaultScannerWsClient wsClient = new DefaultScannerWsClient(WsClientFactories.getDefault().newClient(connector), false,
       analysisMode, analysisWarnings);
diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/GzipRejectorInterceptor.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/GzipRejectorInterceptor.java
new file mode 100644 (file)
index 0000000..bee32f5
--- /dev/null
@@ -0,0 +1,43 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2024 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonarqube.ws.client;
+
+import java.io.IOException;
+import okhttp3.Interceptor;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.jetbrains.annotations.NotNull;
+
+public class GzipRejectorInterceptor implements Interceptor {
+
+  @NotNull
+  @Override
+  public Response intercept(@NotNull Chain chain) throws IOException {
+    Request request = chain.request().newBuilder().removeHeader("Accept-Encoding").build();
+
+    Response response = chain.proceed(request);
+
+    if (response.headers("Content-Encoding").contains("gzip")) {
+      response.close();
+    }
+    return response;
+  }
+
+}
index fcf4cd991d4df4293da6591987cbea8734b5a580..2af46da667d6095f016fa6dc85f9eefc43e6d1b2 100644 (file)
@@ -88,6 +88,7 @@ public class HttpConnector implements WsConnector {
     okHttpClientBuilder.setReadTimeoutMs(builder.readTimeoutMs);
     okHttpClientBuilder.setSSLSocketFactory(builder.sslSocketFactory);
     okHttpClientBuilder.setTrustManager(builder.sslTrustManager);
+    okHttpClientBuilder.acceptGzip(builder.acceptGzip);
     this.okHttpClient = okHttpClientBuilder.build();
     this.noRedirectOkHttpClient = newClientWithoutRedirect(this.okHttpClient);
   }
@@ -265,6 +266,7 @@ public class HttpConnector implements WsConnector {
     private int readTimeoutMs = DEFAULT_READ_TIMEOUT_MILLISECONDS;
     private SSLSocketFactory sslSocketFactory = null;
     private X509TrustManager sslTrustManager = null;
+    private boolean acceptGzip = false;
 
     /**
      * Private since 5.5.
@@ -308,6 +310,14 @@ public class HttpConnector implements WsConnector {
       return this;
     }
 
+    /**
+     * This flag decides whether the client should accept GZIP encoding. Default is false.
+     */
+    public Builder acceptGzip(boolean acceptGzip) {
+      this.acceptGzip = acceptGzip;
+      return this;
+    }
+
     /**
      * Sets a specified timeout value, in milliseconds, to be used when opening HTTP connection.
      * A timeout of zero is interpreted as an infinite timeout. Default value is {@link #DEFAULT_CONNECT_TIMEOUT_MILLISECONDS}
index a26dc7e2fb9f0a5859fc829b773e2c8b901f812a..92be8643433fc4fb9d77aa55e90ce67400568d6f 100644 (file)
@@ -72,6 +72,7 @@ public class OkHttpClientBuilder {
   private long readTimeoutMs = -1;
   private SSLSocketFactory sslSocketFactory = null;
   private X509TrustManager sslTrustManager = null;
+  private boolean acceptGzip = false;
 
   /**
    * Optional User-Agent. If set, then all the requests sent by the
@@ -118,6 +119,14 @@ public class OkHttpClientBuilder {
     return this;
   }
 
+  /**
+   * This flag decides whether the client should accept GZIP encoding. Default is false.
+   */
+  public OkHttpClientBuilder acceptGzip(boolean acceptGzip) {
+    this.acceptGzip = acceptGzip;
+    return this;
+  }
+
   /**
    * Password used for proxy authentication. It is ignored if
    * proxy login is not defined (see {@link #setProxyLogin(String)}).
@@ -179,6 +188,9 @@ public class OkHttpClientBuilder {
       builder.readTimeout(readTimeoutMs, TimeUnit.MILLISECONDS);
     }
     builder.addNetworkInterceptor(this::addHeaders);
+    if(!acceptGzip) {
+      builder.addNetworkInterceptor(new GzipRejectorInterceptor());
+    }
     if (proxyLogin != null) {
       builder.proxyAuthenticator((route, response) -> {
         if (response.request().header(PROXY_AUTHORIZATION) != null) {
diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/client/GzipRejectorInterceptorTest.java b/sonar-ws/src/test/java/org/sonarqube/ws/client/GzipRejectorInterceptorTest.java
new file mode 100644 (file)
index 0000000..9d49546
--- /dev/null
@@ -0,0 +1,79 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2024 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonarqube.ws.client;
+
+import java.io.IOException;
+import java.util.List;
+import okhttp3.Headers;
+import okhttp3.Interceptor;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class GzipRejectorInterceptorTest {
+
+  private final GzipRejectorInterceptor underTest = new GzipRejectorInterceptor();
+
+  private Interceptor.Chain chain = mock();
+  private Response response = mock(Response.class);
+  private Request request = mock(Request.class);
+  private Request.Builder builderThatRemovesHeaders = mock(Request.Builder.class);
+
+  @Before
+  public void before() throws IOException {
+    when(builderThatRemovesHeaders.removeHeader(any())).thenReturn(builderThatRemovesHeaders);
+    when(builderThatRemovesHeaders.build()).thenReturn(request);
+    when(request.newBuilder()).thenReturn(builderThatRemovesHeaders);
+    when(chain.request()).thenReturn(request);
+    when(chain.proceed(any())).thenReturn(response);
+  }
+
+  @Test
+  public void intercept_shouldAlwaysRemoveAcceptEncoding() throws IOException {
+    underTest.intercept(chain);
+
+    verify(builderThatRemovesHeaders, times(1)).removeHeader("Accept-Encoding");
+  }
+
+  @Test
+  public void intercept_whenGzipContentEncodingIncluded_shouldCloseTheResponse() throws IOException {
+    when(response.headers("Content-Encoding")).thenReturn(List.of("gzip"));
+
+    underTest.intercept(chain);
+
+    verify(response, times(1)).close();
+  }
+
+  @Test
+  public void intercept_whenGzipContentEncodingNotIncluded_shouldNotCloseTheResponse() throws IOException {
+    when(response.headers()).thenReturn(Headers.of("Custom-header", "not-gzip"));
+
+    underTest.intercept(chain);
+
+    verify(response, times(0)).close();
+  }
+}
\ No newline at end of file
index 75986e3617d9b9b99962499498561bdf1f5ce9d4..bbc9c0e127d3e5b06a422d710c397ef89b78b79e 100644 (file)
@@ -28,12 +28,14 @@ import java.util.Base64;
 import java.util.List;
 import java.util.Random;
 import java.util.concurrent.TimeUnit;
+import java.util.zip.GZIPOutputStream;
 import javax.net.ssl.SSLSocketFactory;
 import okhttp3.ConnectionSpec;
 import okhttp3.OkHttpClient;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
 import okhttp3.mockwebserver.RecordedRequest;
+import okio.Buffer;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.RandomStringUtils;
@@ -49,6 +51,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 import static okhttp3.Credentials.basic;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 import static org.sonarqube.ws.client.HttpConnector.newBuilder;
@@ -99,6 +102,42 @@ public class HttpConnectorTest {
     assertThat(response.code()).isEqualTo(200);
   }
 
+  @Test
+  public void call_whenGzipNotAcceptedInResponse_shouldNotUseGzip() throws Exception {
+    server.enqueue(new MockResponse().setResponseCode(200).addHeader("Content-Encoding", "gzip")
+      .setBody(gzip("potentially a body with 100 GB of data normally encoded in gzip")));
+
+    //by default we dont accept gzip
+    underTest = HttpConnector.newBuilder().url(serverUrl).build();
+    GetRequest request = new GetRequest("rest/api/1.0/repos");
+
+    WsResponse call = underTest.call(request);
+    assertThrows(Throwable.class, () -> call.content());
+  }
+
+  @Test
+  public void call_whenGzipIsAcceptedInResponse_shouldResponseContainContent() throws Exception {
+    server.enqueue(new MockResponse().setResponseCode(200).addHeader("Content-Encoding", "gzip")
+      .setBody(gzip("example")));
+
+    underTest = HttpConnector.newBuilder().acceptGzip(true).url(serverUrl).build();
+    GetRequest request = new GetRequest("rest/api/1.0/repos").setHeader("Accept-Encoding", "gzip");
+
+    WsResponse call = underTest.call(request);
+    RecordedRequest recordedRequest = server.takeRequest();
+
+    assertThat(recordedRequest.getHeader("Accept-Encoding")).isEqualTo("gzip");
+    assertThat(call.content()).isNotEmpty();
+  }
+
+  private Buffer gzip(String content) throws IOException {
+    Buffer buffer = new Buffer();
+    GZIPOutputStream gzip = new GZIPOutputStream(buffer.outputStream());
+    gzip.write(content.getBytes(UTF_8));
+    gzip.close();
+    return buffer;
+  }
+
   @Test
   public void test_default_settings() throws Exception {
     answerHelloWorld();
@@ -122,8 +161,7 @@ public class HttpConnectorTest {
     assertThat(recordedRequest.getHeader("Accept")).isEqualTo(MediaTypes.PROTOBUF);
     assertThat(recordedRequest.getHeader("Accept-Charset")).isEqualTo("UTF-8");
     assertThat(recordedRequest.getHeader("User-Agent")).startsWith("okhttp/");
-    // compression is handled by OkHttp
-    assertThat(recordedRequest.getHeader("Accept-Encoding")).isEqualTo("gzip");
+    assertThat(recordedRequest.getHeader("Accept-Encoding")).isNull();
   }
 
   @Test
index a588a5bcb37e508dfd04c34874c74036953ef26f..7881f554f92098b1681db1856c30896de14d009a 100644 (file)
@@ -37,7 +37,7 @@ public class OkHttpClientBuilderTest {
     OkHttpClient okHttpClient = underTest.build();
 
     assertThat(okHttpClient.proxy()).isNull();
-    assertThat(okHttpClient.networkInterceptors()).hasSize(1);
+    assertThat(okHttpClient.networkInterceptors()).hasSize(2);
     assertThat(okHttpClient.sslSocketFactory()).isNotNull();
     assertThat(okHttpClient.followRedirects()).isTrue();
     assertThat(okHttpClient.followSslRedirects()).isTrue();
index ba5dba55f23372eb59cef6679bf216befef76b0d..f33b3bea89fea0bcd30651ecfbe5995a529c9cc3 100644 (file)
@@ -364,6 +364,7 @@ public class Tester extends ExternalResource implements TesterSession, BeforeEac
     private TesterSessionImpl(Orchestrator orchestrator, @Nullable String login, @Nullable String password) {
       Server server = orchestrator.getServer();
       this.client = WsClientFactories.getDefault().newClient(HttpConnector.newBuilder()
+        .acceptGzip(true)
         .url(server.getUrl())
         .credentials(login, password)
         .build());
@@ -372,6 +373,7 @@ public class Tester extends ExternalResource implements TesterSession, BeforeEac
     private TesterSessionImpl(Orchestrator orchestrator, @Nullable String systemPassCode) {
       Server server = orchestrator.getServer();
       this.client = WsClientFactories.getDefault().newClient(HttpConnector.newBuilder()
+        .acceptGzip(true)
         .systemPassCode(systemPassCode)
         .url(server.getUrl())
         .build());
@@ -380,6 +382,7 @@ public class Tester extends ExternalResource implements TesterSession, BeforeEac
     private TesterSessionImpl(Orchestrator orchestrator, Consumer<HttpConnector.Builder>... httpConnectorPopulators) {
       Server server = orchestrator.getServer();
       HttpConnector.Builder httpConnectorBuilder = HttpConnector.newBuilder()
+        .acceptGzip(true)
         .url(server.getUrl());
       stream(httpConnectorPopulators).forEach(populator -> populator.accept(httpConnectorBuilder));
       this.client = WsClientFactories.getDefault().newClient(httpConnectorBuilder.build());