Skip to content

Commit aaca502

Browse files
swallezgithub-actions[bot]
authored andcommitted
Fix NPEs and race condition in deserializers (#405)
1 parent 1ff9b01 commit aaca502

File tree

7 files changed

+126
-10
lines changed

7 files changed

+126
-10
lines changed

java-client/src/main/java/co/elastic/clients/json/BuildFunctionDeserializer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,12 @@ protected JsonpDeserializer<B> unwrap() {
4545
@Override
4646
public T deserialize(JsonParser parser, JsonpMapper mapper) {
4747
B builder = builderDeserializer.deserialize(parser, mapper);
48-
return build.apply(builder);
48+
return builder == null ? null : build.apply(builder);
4949
}
5050

5151
@Override
5252
public T deserialize(JsonParser parser, JsonpMapper mapper, JsonParser.Event event) {
5353
B builder = builderDeserializer.deserialize(parser, mapper, event);
54-
return build.apply(builder);
54+
return builder == null ? null : build.apply(builder);
5555
}
5656
}

java-client/src/main/java/co/elastic/clients/json/JsonpDeserializerBase.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -295,11 +295,13 @@ public EnumSet<Event> nativeEvents() {
295295
public EnumSet<Event> acceptedEvents() {
296296
// Accepted events is computed lazily
297297
// no need for double-checked lock, we don't care about computing it several times
298-
if (acceptedEvents == null) {
299-
acceptedEvents = EnumSet.of(Event.START_ARRAY);
300-
acceptedEvents.addAll(itemDeserializer.acceptedEvents());
298+
EnumSet<Event> events = acceptedEvents;
299+
if (events == null) {
300+
events = EnumSet.of(Event.START_ARRAY);
301+
events.addAll(itemDeserializer.acceptedEvents());
302+
acceptedEvents = events;
301303
}
302-
return acceptedEvents;
304+
return events;
303305
}
304306

305307
@Override

java-client/src/main/java/co/elastic/clients/json/LazyDeserializer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,12 @@
2929
* @see JsonpDeserializable
3030
* @see <a href="https://docs.oracle.com/javase/specs/jls/se8/html/jls-12.html#jls-12.4">Initialization of Classes and Interfaces</a>
3131
*/
32-
class LazyDeserializer<T> extends DelegatingDeserializer.SameType<T> {
32+
public class LazyDeserializer<T> extends DelegatingDeserializer.SameType<T> {
3333

3434
private final Supplier<JsonpDeserializer<T>> ctor;
3535
private volatile JsonpDeserializer<T> deserializer = null;
3636

37-
LazyDeserializer(Supplier<JsonpDeserializer<T>> ctor) {
37+
public LazyDeserializer(Supplier<JsonpDeserializer<T>> ctor) {
3838
this.ctor = ctor;
3939
}
4040

java-client/src/main/java/co/elastic/clients/json/ObjectBuilderDeserializer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,12 @@ protected JsonpDeserializer<B> unwrap() {
7777
@Override
7878
public T deserialize(JsonParser parser, JsonpMapper mapper) {
7979
ObjectBuilder<T> builder = builderDeserializer.deserialize(parser, mapper);
80-
return builder.build();
80+
return builder == null ? null : builder.build();
8181
}
8282

8383
@Override
8484
public T deserialize(JsonParser parser, JsonpMapper mapper, JsonParser.Event event) {
8585
ObjectBuilder<T> builder = builderDeserializer.deserialize(parser, mapper, event);
86-
return builder.build();
86+
return builder == null ? null : builder.build();
8787
}
8888
}

java-client/src/main/java/co/elastic/clients/json/ObjectDeserializer.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ public FieldDeserializer(String name) {
4444
this.name = name;
4545
}
4646

47+
public abstract EnumSet<Event> acceptedEvents();
48+
4749
public abstract void deserialize(JsonParser parser, JsonpMapper mapper, String fieldName, ObjectType object);
4850

4951
public abstract void deserialize(JsonParser parser, JsonpMapper mapper, String fieldName, ObjectType object, Event event);
@@ -67,6 +69,11 @@ public String name() {
6769
return this.name;
6870
}
6971

72+
@Override
73+
public EnumSet<Event> acceptedEvents() {
74+
return deserializer.acceptedEvents();
75+
}
76+
7077
public void deserialize(JsonParser parser, JsonpMapper mapper, String fieldName, ObjectType object) {
7178
FieldType fieldValue = deserializer.deserialize(parser, mapper);
7279
setter.accept(object, fieldValue);
@@ -90,6 +97,11 @@ public void deserialize(JsonParser parser, JsonpMapper mapper, String fieldName,
9097
public void deserialize(JsonParser parser, JsonpMapper mapper, String fieldName, Object object, Event event) {
9198
JsonpUtils.skipValue(parser, event);
9299
}
100+
101+
@Override
102+
public EnumSet<Event> acceptedEvents() {
103+
return EnumSet.allOf(Event.class);
104+
}
93105
};
94106

95107
//---------------------------------------------------------------------------------------------
@@ -242,6 +254,8 @@ public void shortcutProperty(String name) {
242254
throw new NoSuchElementException("No deserializer was setup for '" + name + "'");
243255
}
244256

257+
//acceptedEvents = EnumSet.copyOf(acceptedEvents);
258+
//acceptedEvents.addAll(shortcutProperty.acceptedEvents());
245259
acceptedEvents = EventSetObjectAndString;
246260
}
247261

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package co.elastic.clients.json;
21+
22+
import co.elastic.clients.elasticsearch.model.ModelTestCase;
23+
import jakarta.json.stream.JsonParser;
24+
import org.junit.jupiter.api.Test;
25+
26+
import java.util.List;
27+
28+
public class JsonpDeserializerBaseTest extends ModelTestCase {
29+
30+
@Test
31+
public void testArrayDeserializer() {
32+
33+
JsonpDeserializer<List<Integer>> deser =
34+
JsonpDeserializer.arrayDeserializer(JsonpDeserializer.integerDeserializer());
35+
36+
assertFalse(deser.nativeEvents().contains(JsonParser.Event.VALUE_NUMBER));
37+
assertTrue(deser.acceptedEvents().contains(JsonParser.Event.VALUE_NUMBER));
38+
39+
List<Integer> values = fromJson("[ 42, 43 ]", deser);
40+
assertEquals(2, values.size());
41+
assertEquals(42, values.get(0));
42+
assertEquals(43, values.get(1));
43+
44+
// Single value representation
45+
values = fromJson("42", deser);
46+
assertEquals(1, values.size());
47+
assertEquals(42, values.get(0));
48+
49+
}
50+
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package co.elastic.clients.json;
21+
22+
import co.elastic.clients.elasticsearch._types.mapping.TextProperty;
23+
import co.elastic.clients.elasticsearch.model.ModelTestCase;
24+
import co.elastic.clients.elasticsearch.transform.UpdateTransformRequest;
25+
import org.junit.jupiter.api.Test;
26+
27+
import java.io.StringReader;
28+
29+
public class ObjectBuilderDeserializerTest extends ModelTestCase {
30+
31+
@Test
32+
public void testNullObjectValue() {
33+
// Should also accept null for optional values
34+
String json = "{ \"index_prefixes\": null }";
35+
fromJson(json, TextProperty.class);
36+
}
37+
38+
@Test
39+
public void testNullObjectValueInFunctionBuilder() {
40+
String json = "{\n" +
41+
" \"retention_policy\": null\n" +
42+
" }";
43+
44+
UpdateTransformRequest.Builder builder = new UpdateTransformRequest.Builder();
45+
builder.transformId("foo");
46+
builder.withJson(new StringReader(json));
47+
builder.build();
48+
}
49+
50+
}

0 commit comments

Comments
 (0)