Skip to content

Commit d7c6715

Browse files
thllclaude
andauthored
Bump apache-httpclient5.version to 5.6.2, apache-httpcore5.version to 5.4.3 (#26613)
* Centrally manage httpclient5/httpcore5 dependencies * Add gzip decompression guard test for the OS3 client A gzip-encoded response must be decompressed by exactly one layer: opensearch-java < 3.6.0 decompresses itself, so HttpClient's transparent decompression (default since HttpClient 5.6) must be disabled; from opensearch-java 3.6.0 on it's the other way around. This test fails in either misconfiguration, in particular when disableContentCompression() outlives a future opensearch-java upgrade. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Disable content compression by http client --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent bb6fa03 commit d7c6715

4 files changed

Lines changed: 168 additions & 1 deletion

File tree

graylog-storage-opensearch3/pom.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,11 @@
135135
<artifactId>mockito-junit-jupiter</artifactId>
136136
<scope>test</scope>
137137
</dependency>
138+
<dependency>
139+
<groupId>com.squareup.okhttp3</groupId>
140+
<artifactId>mockwebserver</artifactId>
141+
<scope>test</scope>
142+
</dependency>
138143
</dependencies>
139144

140145
<build>

graylog-storage-opensearch3/src/main/java/org/graylog/storage/opensearch3/OfficialOpensearchClientProvider.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,14 @@ public OpenSearchTransport buildTransport(List<URI> uris, TransportConfig config
164164
httpClientBuilder.addResponseInterceptorFirst(new OpenSearchFilterDeprecationWarningsInterceptor());
165165
}
166166

167+
// Workaround to disable transparent content compression and decompression by the http client.
168+
// In opensearch-java < 3.6.0, the OpenSearch client handles decompression of the responses itself, so
169+
// leaving this enabled would cause double decompression and fail with "Not in GZIP format".
170+
// Remove this when upgrading opensearch-java to >= 3.6.0, which relies on the http client for
171+
// decompression instead (https://github.com/opensearch-project/opensearch-java/pull/1844) —
172+
// OpensearchClientGzipHandlingTest will fail if this line outlives the upgrade.
173+
httpClientBuilder.disableContentCompression();
174+
167175
return httpClientBuilder;
168176
});
169177

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/*
2+
* Copyright (C) 2020 Graylog, Inc.
3+
*
4+
* This program is free software: you can redistribute it and/or modify
5+
* it under the terms of the Server Side Public License, version 1,
6+
* as published by MongoDB, Inc.
7+
*
8+
* This program is distributed in the hope that it will be useful,
9+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
10+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11+
* Server Side Public License for more details.
12+
*
13+
* You should have received a copy of the Server Side Public License
14+
* along with this program. If not, see
15+
* <http://www.mongodb.com/licensing/server-side-public-license>.
16+
*/
17+
package org.graylog.storage.opensearch3;
18+
19+
import com.github.joschi.jadconfig.JadConfig;
20+
import com.github.joschi.jadconfig.repositories.InMemoryRepository;
21+
import com.google.common.collect.ImmutableList;
22+
import mockwebserver3.MockResponse;
23+
import mockwebserver3.MockWebServer;
24+
import okio.Buffer;
25+
import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider;
26+
import org.graylog2.configuration.ElasticsearchClientConfiguration;
27+
import org.graylog2.security.jwt.IndexerJwtAuthToken;
28+
import org.graylog2.shared.bindings.providers.ObjectMapperProvider;
29+
import org.junit.jupiter.api.AfterEach;
30+
import org.junit.jupiter.api.BeforeEach;
31+
import org.junit.jupiter.api.Test;
32+
import org.opensearch.client.opensearch.core.InfoResponse;
33+
34+
import java.io.ByteArrayOutputStream;
35+
import java.net.URI;
36+
import java.nio.charset.StandardCharsets;
37+
import java.util.Map;
38+
import java.util.zip.GZIPOutputStream;
39+
40+
import static org.assertj.core.api.Assertions.assertThat;
41+
42+
/**
43+
* Guards the division of labor for response gzip handling between opensearch-java and Apache HttpClient.
44+
*
45+
* <p>A gzip-encoded response must be decompressed by <em>exactly one</em> layer:
46+
* <ul>
47+
* <li>opensearch-java &lt; 3.6.0 decompresses unconditionally itself, so HttpClient's transparent
48+
* decompression (on by default since HttpClient 5.6) must be switched off via
49+
* {@code disableContentCompression()} in {@link OfficialOpensearchClientProvider} — otherwise the
50+
* body is inflated twice ("Not in GZIP format").</li>
51+
* <li>opensearch-java &gt;= 3.6.0 no longer decompresses
52+
* (see https://github.com/opensearch-project/opensearch-java/pull/1844) and relies on HttpClient,
53+
* so {@code disableContentCompression()} must be removed — otherwise nobody inflates the body.</li>
54+
* </ul>
55+
*
56+
* <p>This test fails in either misconfiguration. If it starts failing after an opensearch-java upgrade,
57+
* remove {@code disableContentCompression()} from {@code OfficialOpensearchClientProvider}.
58+
*/
59+
class OpensearchClientGzipHandlingTest {
60+
61+
// A minimal but complete OpenSearch info() response so the client call parses cleanly.
62+
private static final String INFO_JSON = """
63+
{
64+
"name": "test-node",
65+
"cluster_name": "test-cluster",
66+
"cluster_uuid": "test-uuid",
67+
"version": {
68+
"distribution": "opensearch",
69+
"number": "2.11.0",
70+
"build_type": "tar",
71+
"build_hash": "deadbeef",
72+
"build_date": "2023-01-01T00:00:00.000Z",
73+
"build_snapshot": false,
74+
"lucene_version": "9.7.0",
75+
"minimum_wire_compatibility_version": "7.10.0",
76+
"minimum_index_compatibility_version": "7.0.0"
77+
},
78+
"tagline": "The OpenSearch Project"
79+
}""";
80+
81+
private final MockWebServer server = new MockWebServer();
82+
83+
@BeforeEach
84+
void setUp() throws Exception {
85+
server.start();
86+
}
87+
88+
@AfterEach
89+
void tearDown() throws Exception {
90+
server.close();
91+
}
92+
93+
@Test
94+
void gzipEncodedResponsesAreDecompressedExactlyOnce() throws Exception {
95+
try (Buffer body = new Buffer()) {
96+
body.write(gzip(INFO_JSON));
97+
server.enqueue(new MockResponse.Builder()
98+
.code(200)
99+
.addHeader("Content-Type", "application/json")
100+
.addHeader("Content-Encoding", "gzip")
101+
.body(body)
102+
.build());
103+
}
104+
105+
final InfoResponse info = buildClient().sync(client -> client.info(), "Failed to read gzip-encoded info response");
106+
107+
assertThat(info.clusterName())
108+
.as("A gzip response must be inflated by exactly one layer. If this fails after upgrading "
109+
+ "opensearch-java to >= 3.6.0, remove disableContentCompression() from "
110+
+ "OfficialOpensearchClientProvider. If it fails with opensearch-java < 3.6.0, "
111+
+ "that call went missing and the response is inflated twice.")
112+
.isEqualTo("test-cluster");
113+
}
114+
115+
private OfficialOpensearchClient buildClient() throws Exception {
116+
final URI uri = server.url("/").uri();
117+
return new OfficialOpensearchClientProvider(
118+
ImmutableList.of(uri),
119+
IndexerJwtAuthToken.disabled(),
120+
new BasicCredentialsProvider(),
121+
config(),
122+
new ObjectMapperProvider().get(),
123+
null
124+
).get();
125+
}
126+
127+
private static byte[] gzip(String content) throws Exception {
128+
final ByteArrayOutputStream out = new ByteArrayOutputStream();
129+
try (GZIPOutputStream gzipOut = new GZIPOutputStream(out)) {
130+
gzipOut.write(content.getBytes(StandardCharsets.UTF_8));
131+
}
132+
return out.toByteArray();
133+
}
134+
135+
private static ElasticsearchClientConfiguration config() throws Exception {
136+
final ElasticsearchClientConfiguration config = new ElasticsearchClientConfiguration();
137+
new JadConfig(new InMemoryRepository(Map.of(
138+
"elasticsearch_connect_timeout", "10s",
139+
"elasticsearch_socket_timeout", "10s"
140+
)), config).process();
141+
return config;
142+
}
143+
}

pom.xml

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,9 @@
104104
<antlr.version>4.13.2</antlr.version>
105105
<apache-directory-version>2.1.8</apache-directory-version>
106106
<apache-httpclient.version>4.5.14</apache-httpclient.version>
107-
<apache-httpclient5.version>5.5.1</apache-httpclient5.version>
107+
<apache-httpclient5.version>5.6.2</apache-httpclient5.version>
108108
<apache-httpcore.version>4.4.16</apache-httpcore.version>
109+
<apache-httpcore5.version>5.4.3</apache-httpcore5.version>
109110
<asm.version>9.10.1</asm.version>
110111
<auto-service.version>1.1.1</auto-service.version>
111112
<auto-value.version>1.11.1</auto-value.version>
@@ -274,6 +275,16 @@
274275
<artifactId>httpclient5</artifactId>
275276
<version>${apache-httpclient5.version}</version>
276277
</dependency>
278+
<dependency>
279+
<groupId>org.apache.httpcomponents.core5</groupId>
280+
<artifactId>httpcore5</artifactId>
281+
<version>${apache-httpcore5.version}</version>
282+
</dependency>
283+
<dependency>
284+
<groupId>org.apache.httpcomponents.core5</groupId>
285+
<artifactId>httpcore5-h2</artifactId>
286+
<version>${apache-httpcore5.version}</version>
287+
</dependency>
277288

278289
<dependency>
279290
<groupId>org.apache.httpcomponents</groupId>

0 commit comments

Comments
 (0)