From e21e8474d4c2af05aca478fc0ab5163ff5668901 Mon Sep 17 00:00:00 2001 From: lukasz-jarocki-sonarsource Date: Thu, 15 Aug 2024 16:14:37 +0200 Subject: [PATCH] SONAR-21857 fix ssf --- .../core/util/DefaultHttpDownloaderIT.java | 19 +---- .../bootstrap/ScannerWsClientProvider.java | 2 +- .../scanner/bootstrap/PluginFilesTest.java | 2 +- sonar-ws/build.gradle | 1 + .../ws/client/GzipRejectorInterceptor.java | 43 ++++++++++ .../sonarqube/ws/client/HttpConnector.java | 10 +++ .../ws/client/OkHttpClientBuilder.java | 12 +++ .../client/GzipRejectorInterceptorTest.java | 79 +++++++++++++++++++ .../ws/client/HttpConnectorTest.java | 42 +++++++++- .../ws/client/OkHttpClientBuilderTest.java | 2 +- 10 files changed, 190 insertions(+), 22 deletions(-) create mode 100644 sonar-ws/src/main/java/org/sonarqube/ws/client/GzipRejectorInterceptor.java create mode 100644 sonar-ws/src/test/java/org/sonarqube/ws/client/GzipRejectorInterceptorTest.java diff --git a/sonar-core/src/test/java/org/sonar/core/util/DefaultHttpDownloaderIT.java b/sonar-core/src/test/java/org/sonar/core/util/DefaultHttpDownloaderIT.java index 321744b615f..d9bd0922089 100644 --- a/sonar-core/src/test/java/org/sonar/core/util/DefaultHttpDownloaderIT.java +++ b/sonar-core/src/test/java/org/sonar/core/util/DefaultHttpDownloaderIT.java @@ -30,7 +30,6 @@ import java.net.URI; import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.util.Properties; -import java.util.zip.GZIPOutputStream; import org.hamcrest.BaseMatcher; import org.hamcrest.Description; import org.junit.AfterClass; @@ -82,14 +81,6 @@ public class DefaultHttpDownloaderIT { } catch (InterruptedException e) { throw new IllegalStateException(e); } - } 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 { @@ -123,7 +114,7 @@ public class DefaultHttpDownloaderIT { } @Test(timeout = 10000) - public void openStream_network_errors() throws IOException, URISyntaxException { + public void openStream_network_errors() { // non routable address String url = "http://10.255.255.1"; @@ -156,12 +147,6 @@ public class DefaultHttpDownloaderIT { assertThat(text.length()).isGreaterThan(10); } - @Test - public 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 public void readStringWithDefaultTimeout() throws URISyntaxException { String text = new DefaultHttpDownloader(mock(Server.class), new MapSettings().asConfig()).readString(new URI(baseUrl + "/timeout/"), StandardCharsets.UTF_8); @@ -169,7 +154,7 @@ public class DefaultHttpDownloaderIT { } @Test - public void readStringWithTimeout() throws URISyntaxException { + public void readStringWithTimeout() { assertThatThrownBy( () -> new DefaultHttpDownloader(mock(Server.class), new MapSettings().asConfig(), null, 50).readString(new URI(baseUrl + "/timeout/"), StandardCharsets.UTF_8)) .isEqualToComparingFieldByField(new BaseMatcher() { diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/bootstrap/ScannerWsClientProvider.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/bootstrap/ScannerWsClientProvider.java index b4cbad4e365..49284974a1f 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/bootstrap/ScannerWsClientProvider.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/bootstrap/ScannerWsClientProvider.java @@ -40,7 +40,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 token = defaultIfBlank(system.envVariable("SONAR_TOKEN"), null); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/bootstrap/PluginFilesTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/bootstrap/PluginFilesTest.java index febfe453317..65fee066163 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/bootstrap/PluginFilesTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/bootstrap/PluginFilesTest.java @@ -62,7 +62,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/build.gradle b/sonar-ws/build.gradle index d6c59dfce1b..957b4bd1d6a 100644 --- a/sonar-ws/build.gradle +++ b/sonar-ws/build.gradle @@ -29,6 +29,7 @@ dependencies { testImplementation 'org.hamcrest:hamcrest-core' testImplementation 'org.mockito:mockito-core' testImplementation project(':sonar-testing-harness') + testImplementation 'org.mockito:mockito-inline:2.13.0' } artifactoryPublish.skip = false 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 index 00000000000..bee32f569fa --- /dev/null +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/GzipRejectorInterceptor.java @@ -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; + } + +} diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpConnector.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpConnector.java index c76ea1ca30d..46e0b5cff82 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpConnector.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpConnector.java @@ -86,6 +86,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); } @@ -264,6 +265,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. @@ -307,6 +309,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} 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 a26dc7e2fb9..92be8643433 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 @@ -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 index 00000000000..9d49546d9f1 --- /dev/null +++ b/sonar-ws/src/test/java/org/sonarqube/ws/client/GzipRejectorInterceptorTest.java @@ -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 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 c7b9df1ab49..d41adb73469 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 @@ -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.lang.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 diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/client/OkHttpClientBuilderTest.java b/sonar-ws/src/test/java/org/sonarqube/ws/client/OkHttpClientBuilderTest.java index a588a5bcb37..7881f554f92 100644 --- a/sonar-ws/src/test/java/org/sonarqube/ws/client/OkHttpClientBuilderTest.java +++ b/sonar-ws/src/test/java/org/sonarqube/ws/client/OkHttpClientBuilderTest.java @@ -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(); -- 2.39.5