From 917a1f424307557ab9f6fbc2dcb1a8320774e511 Mon Sep 17 00:00:00 2001 From: lukasz-jarocki-sonarsource Date: Tue, 19 Mar 2024 15:39:33 +0100 Subject: [PATCH] SONAR-21857 fix ssf --- .../core/util/DefaultHttpDownloaderIT.java | 16 +--- .../bootstrap/ScannerWsClientProvider.java | 2 +- .../scanner/bootstrap/PluginFilesTest.java | 2 +- .../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 +- .../java/org/sonarqube/ws/tester/Tester.java | 3 + 10 files changed, 192 insertions(+), 19 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/it/java/org/sonar/core/util/DefaultHttpDownloaderIT.java b/sonar-core/src/it/java/org/sonar/core/util/DefaultHttpDownloaderIT.java index 332fe10b57e..7aededcf286 100644 --- a/sonar-core/src/it/java/org/sonar/core/util/DefaultHttpDownloaderIT.java +++ b/sonar-core/src/it/java/org/sonar/core/util/DefaultHttpDownloaderIT.java @@ -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); 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 9d1dfe6fa0e..b06d0eb4e08 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 @@ -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); 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 a252bbbe3a0..21d6b3c5317 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 @@ -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 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 fcf4cd991d4..2af46da667d 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 @@ -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} 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 75986e3617d..bbc9c0e127d 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.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 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(); diff --git a/sonar-ws/src/testFixtures/java/org/sonarqube/ws/tester/Tester.java b/sonar-ws/src/testFixtures/java/org/sonarqube/ws/tester/Tester.java index ba5dba55f23..f33b3bea89f 100644 --- a/sonar-ws/src/testFixtures/java/org/sonarqube/ws/tester/Tester.java +++ b/sonar-ws/src/testFixtures/java/org/sonarqube/ws/tester/Tester.java @@ -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... 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()); -- 2.39.5