Skip to content

Commit 9e7518e

Browse files
opensearch-trigger-bot[bot]github-actions[bot]
opensearch-trigger-bot[bot]
and
github-actions[bot]
authored
[1887] Fix NPE in transport option conversion (#1888) (#1912)
(cherry picked from commit 16b7747) Signed-off-by: Baekgyu <baekgyukim.dev@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 656433b commit 9e7518e

5 files changed

Lines changed: 234 additions & 12 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
1818
### Removed
1919

2020
### Fixed
21+
- Fix NPEs while converting transport options when null options, headers, or query parameters are provided ([#1887](https://github.com/opensearch-project/opensearch-java/pull/1888))
2122

2223
### Security
2324

‎java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Options.java‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -220,16 +220,25 @@ static ApacheHttpClient5Options initialOptions() {
220220
}
221221

222222
static ApacheHttpClient5Options of(TransportOptions options) {
223+
if (options == null) return DEFAULT;
224+
223225
if (options instanceof ApacheHttpClient5Options) {
224226
return (ApacheHttpClient5Options) options;
227+
}
225228

226-
} else {
227-
final Builder builder = new Builder(DEFAULT.toBuilder());
228-
options.headers().forEach(h -> builder.addHeader(h.getKey(), h.getValue()));
229-
options.queryParameters().forEach(builder::setParameter);
230-
builder.onWarnings(options.onWarnings());
231-
return builder.build();
229+
final Builder builder = new Builder(DEFAULT.toBuilder());
230+
final Collection<Entry<String, String>> headers = options.headers();
231+
if (headers != null && !headers.isEmpty()) {
232+
headers.forEach(h -> builder.addHeader(h.getKey(), h.getValue()));
232233
}
234+
235+
final Map<String, String> parameters = options.queryParameters();
236+
if (parameters != null && !parameters.isEmpty()) {
237+
parameters.forEach(builder::setParameter);
238+
}
239+
240+
builder.onWarnings(options.onWarnings());
241+
return builder.build();
233242
}
234243

235244
/**

‎java-client/src/main/java/org/opensearch/client/transport/rest_client/RestClientOptions.java‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,16 +57,28 @@ public class RestClientOptions implements TransportOptions {
5757
private final RequestOptions options;
5858

5959
static RestClientOptions of(TransportOptions options) {
60+
if (options == null) {
61+
return new Builder(RequestOptions.DEFAULT.toBuilder()).build();
62+
}
63+
6064
if (options instanceof RestClientOptions) {
6165
return (RestClientOptions) options;
66+
}
67+
68+
final Builder builder = new Builder(RequestOptions.DEFAULT.toBuilder());
6269

63-
} else {
64-
final Builder builder = new Builder(RequestOptions.DEFAULT.toBuilder());
65-
options.headers().forEach(h -> builder.addHeader(h.getKey(), h.getValue()));
66-
options.queryParameters().forEach(builder::setParameter);
67-
builder.onWarnings(options.onWarnings());
68-
return builder.build();
70+
final Collection<Map.Entry<String, String>> headers = options.headers();
71+
if (headers != null && !headers.isEmpty()) {
72+
headers.forEach(h -> builder.addHeader(h.getKey(), h.getValue()));
6973
}
74+
75+
final Map<String, String> parameters = options.queryParameters();
76+
if (parameters != null && !parameters.isEmpty()) {
77+
parameters.forEach(builder::setParameter);
78+
}
79+
80+
builder.onWarnings(options.onWarnings());
81+
return builder.build();
7082
}
7183

7284
public RestClientOptions(RequestOptions options) {

‎java-client/src/test/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5OptionsTest.java‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.client.transport.httpclient5;
10+
11+
import java.util.AbstractMap;
12+
import java.util.Collection;
13+
import java.util.Collections;
14+
import java.util.List;
15+
import java.util.Map;
16+
import java.util.Map.Entry;
17+
import java.util.function.Function;
18+
import org.junit.Assert;
19+
import org.junit.Test;
20+
import org.opensearch.client.transport.TransportOptions;
21+
22+
public class ApacheHttpClient5OptionsTest extends Assert {
23+
@Test
24+
public void testOfWithNullOptions() {
25+
// Null input should fall back to defaults instead of throwing.
26+
ApacheHttpClient5Options options = ApacheHttpClient5Options.of(null);
27+
28+
assertNotNull(options);
29+
assertNotNull(options.headers());
30+
}
31+
32+
@Test
33+
public void testOfWithNullHeaders() {
34+
// Missing headers from source options should be treated as empty.
35+
ApacheHttpClient5Options options = ApacheHttpClient5Options.of(
36+
transportOptions(null, Collections.emptyMap(), warnings -> warnings != null && !warnings.isEmpty())
37+
);
38+
39+
assertNotNull(options);
40+
assertNotNull(options.headers());
41+
assertTrue(options.headers().isEmpty());
42+
assertNotNull(options.onWarnings());
43+
assertTrue(options.onWarnings().apply(Collections.singletonList("warning")));
44+
}
45+
46+
@Test
47+
public void testOfWithNullQueryParameters() {
48+
// Null query parameters should not drop other fields during conversion.
49+
ApacheHttpClient5Options options = ApacheHttpClient5Options.of(
50+
transportOptions(
51+
Collections.singletonList(new AbstractMap.SimpleImmutableEntry<>("x-test-header", "value")),
52+
null,
53+
warnings -> warnings != null && !warnings.isEmpty()
54+
)
55+
);
56+
57+
assertNotNull(options);
58+
assertNotNull(options.onWarnings());
59+
assertTrue(options.onWarnings().apply(Collections.singletonList("warning")));
60+
assertTrue(containsHeader(options.headers(), "x-test-header", "value"));
61+
}
62+
63+
private static boolean containsHeader(Collection<Entry<String, String>> headers, String name, String value) {
64+
for (Entry<String, String> header : headers) {
65+
if (name.equals(header.getKey()) && value.equals(header.getValue())) {
66+
return true;
67+
}
68+
}
69+
70+
return false;
71+
}
72+
73+
private static TransportOptions transportOptions(
74+
Collection<Entry<String, String>> headers,
75+
Map<String, String> queryParameters,
76+
Function<List<String>, Boolean> warningsHandler
77+
) {
78+
return new TransportOptions() {
79+
@Override
80+
public Collection<Entry<String, String>> headers() {
81+
return headers;
82+
}
83+
84+
@Override
85+
public Map<String, String> queryParameters() {
86+
return queryParameters;
87+
}
88+
89+
@Override
90+
public Function<List<String>, Boolean> onWarnings() {
91+
return warningsHandler;
92+
}
93+
94+
@Override
95+
public Builder toBuilder() {
96+
return new BuilderImpl(this);
97+
}
98+
};
99+
}
100+
}

‎java-client/src/test/java/org/opensearch/client/transport/rest_client/RestClientOptionsTest.java‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.client.transport.rest_client;
10+
11+
import java.util.AbstractMap;
12+
import java.util.Collection;
13+
import java.util.Collections;
14+
import java.util.List;
15+
import java.util.Map;
16+
import java.util.Map.Entry;
17+
import java.util.function.Function;
18+
import org.junit.Assert;
19+
import org.junit.Test;
20+
import org.opensearch.client.transport.TransportOptions;
21+
22+
public class RestClientOptionsTest extends Assert {
23+
@Test
24+
public void testOfWithNullOptions() {
25+
// Null input should fall back to defaults instead of throwing.
26+
RestClientOptions options = RestClientOptions.of(null);
27+
28+
assertNotNull(options);
29+
assertNotNull(options.headers());
30+
}
31+
32+
@Test
33+
public void testOfWithNullHeaders() {
34+
// Missing headers from source options should be treated as empty.
35+
RestClientOptions options = RestClientOptions.of(
36+
transportOptions(null, Collections.emptyMap(), warnings -> warnings != null && !warnings.isEmpty())
37+
);
38+
39+
assertNotNull(options);
40+
assertNotNull(options.headers());
41+
assertTrue(options.headers().isEmpty());
42+
assertNotNull(options.onWarnings());
43+
assertTrue(options.onWarnings().apply(Collections.singletonList("warning")));
44+
}
45+
46+
@Test
47+
public void testOfWithNullQueryParameters() {
48+
// Null query parameters should not drop other fields during conversion.
49+
RestClientOptions options = RestClientOptions.of(
50+
transportOptions(
51+
Collections.singletonList(new AbstractMap.SimpleImmutableEntry<>("x-test-header", "value")),
52+
null,
53+
warnings -> warnings != null && !warnings.isEmpty()
54+
)
55+
);
56+
57+
assertNotNull(options);
58+
assertNotNull(options.onWarnings());
59+
assertTrue(options.onWarnings().apply(Collections.singletonList("warning")));
60+
assertTrue(containsHeader(options.headers(), "x-test-header", "value"));
61+
}
62+
63+
private static boolean containsHeader(Collection<Entry<String, String>> headers, String name, String value) {
64+
for (Entry<String, String> header : headers) {
65+
if (name.equals(header.getKey()) && value.equals(header.getValue())) {
66+
return true;
67+
}
68+
}
69+
70+
return false;
71+
}
72+
73+
private static TransportOptions transportOptions(
74+
Collection<Entry<String, String>> headers,
75+
Map<String, String> queryParameters,
76+
Function<List<String>, Boolean> warningsHandler
77+
) {
78+
return new TransportOptions() {
79+
@Override
80+
public Collection<Entry<String, String>> headers() {
81+
return headers;
82+
}
83+
84+
@Override
85+
public Map<String, String> queryParameters() {
86+
return queryParameters;
87+
}
88+
89+
@Override
90+
public Function<List<String>, Boolean> onWarnings() {
91+
return warningsHandler;
92+
}
93+
94+
@Override
95+
public Builder toBuilder() {
96+
return new BuilderImpl(this);
97+
}
98+
};
99+
}
100+
}

0 commit comments

Comments
 (0)