diff --git a/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java b/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java index bb628a5c7f..24d76c66dd 100644 --- a/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java +++ b/java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java @@ -21,6 +21,7 @@ import co.elastic.clients.json.jackson.JacksonJsonProvider; import co.elastic.clients.util.AllowForbiddenApis; +import co.elastic.clients.util.ApiTypeHelper; import jakarta.json.JsonException; import jakarta.json.JsonObject; import jakarta.json.JsonString; @@ -498,7 +499,12 @@ public void close() { } }; - try(JsonGenerator generator = mapper.jsonProvider().createGenerator(writer)) { + try(JsonGenerator rawGenerator = mapper.jsonProvider().createGenerator(writer)) { + // When the missing-properties workaround is active, wrap the generator so that null + // reference values are serialized as JSON null instead of throwing NPE. See #557. + JsonGenerator generator = ApiTypeHelper.requiredPropertiesCheckDisabled() + ? new NullSafeJsonGenerator(rawGenerator) + : rawGenerator; value.serialize(generator, mapper); } catch (ToStringTooLongException e) { // Ignore @@ -508,7 +514,12 @@ public void close() { public static String toJsonString(Object value, JsonpMapper mapper) { StringWriter writer = new StringWriter(); - JsonGenerator generator = mapper.jsonProvider().createGenerator(writer); + JsonGenerator rawGenerator = mapper.jsonProvider().createGenerator(writer); + // When the missing-properties workaround is active, wrap the generator so that null + // reference values are serialized as JSON null instead of throwing NPE. See #557. + JsonGenerator generator = ApiTypeHelper.requiredPropertiesCheckDisabled() + ? new NullSafeJsonGenerator(rawGenerator) + : rawGenerator; mapper.serialize(value, generator); generator.close(); return writer.toString(); diff --git a/java-client/src/main/java/co/elastic/clients/json/NullSafeJsonGenerator.java b/java-client/src/main/java/co/elastic/clients/json/NullSafeJsonGenerator.java new file mode 100644 index 0000000000..d62fd3871a --- /dev/null +++ b/java-client/src/main/java/co/elastic/clients/json/NullSafeJsonGenerator.java @@ -0,0 +1,82 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package co.elastic.clients.json; + +import jakarta.json.JsonValue; +import jakarta.json.stream.JsonGenerator; + +import java.math.BigDecimal; +import java.math.BigInteger; + +/** + * A {@link DelegatingJsonGenerator} that translates {@code write(...)} calls with a {@code null} reference value + * into a {@link JsonGenerator#writeNull() writeNull} call. The {@code write(name, value)} overloads in + * {@link DelegatingJsonGenerator} are {@code final} and split into {@code writeKey(name)} + {@code write(value)}, + * so overriding the value-only methods here is enough to also cover the name+value variants via virtual dispatch. + * + *

This is used as a safety net when {@link co.elastic.clients.util.ApiTypeHelper#DANGEROUS_disableRequiredPropertiesCheck(boolean)} + * is active: with the workaround enabled, deserialization can leave required reference fields as {@code null}, + * and the generated {@code serializeInternal} methods would otherwise call e.g. {@code generator.write((String) null)}, + * which throws {@link NullPointerException} in JSON-P providers such as Parsson. + * + *

See #557. + */ +class NullSafeJsonGenerator extends DelegatingJsonGenerator { + + NullSafeJsonGenerator(JsonGenerator delegate) { + super(delegate); + } + + @Override + public JsonGenerator write(String value) { + if (value == null) { + generator.writeNull(); + return this; + } + return super.write(value); + } + + @Override + public JsonGenerator write(BigDecimal value) { + if (value == null) { + generator.writeNull(); + return this; + } + return super.write(value); + } + + @Override + public JsonGenerator write(BigInteger value) { + if (value == null) { + generator.writeNull(); + return this; + } + return super.write(value); + } + + @Override + public JsonGenerator write(JsonValue value) { + if (value == null) { + generator.writeNull(); + return this; + } + return super.write(value); + } +} diff --git a/java-client/src/test/java/co/elastic/clients/elasticsearch/model/BehaviorsTest.java b/java-client/src/test/java/co/elastic/clients/elasticsearch/model/BehaviorsTest.java index dbba3a7141..1b519240cf 100644 --- a/java-client/src/test/java/co/elastic/clients/elasticsearch/model/BehaviorsTest.java +++ b/java-client/src/test/java/co/elastic/clients/elasticsearch/model/BehaviorsTest.java @@ -468,4 +468,38 @@ public void testDangerousDisablePropertyCheckPrimitive(){ assertTrue(healthResponse.toString()!=null); } } + + @Test + public void testDangerousDisablePropertyCheckReferenceSerialization() { + // Repro for #557: with the workaround enabled, a null required reference + // field (here: clusterName, a required String) causes an NPE during + // re-serialization because HealthResponseBody#serializeInternal calls + // generator.write((String) null). + try (ApiTypeHelper.DisabledChecksHandle h = + ApiTypeHelper.DANGEROUS_disableRequiredPropertiesCheck(true)) { + HealthResponse healthResponse = HealthResponse.of(hr -> hr.withJson(new StringReader("{\n" + + // cluster_name (required) intentionally omitted + " \"status\" : \"green\",\n" + + " \"timed_out\" : false,\n" + + " \"number_of_nodes\" : 3,\n" + + " \"number_of_data_nodes\" : 2,\n" + + " \"active_primary_shards\" : 88,\n" + + " \"active_shards\" : 176,\n" + + " \"relocating_shards\" : 0,\n" + + " \"initializing_shards\" : 0,\n" + + " \"unassigned_shards\" : 0,\n" + + " \"delayed_unassigned_shards\" : 0,\n" + + " \"number_of_pending_tasks\" : 0,\n" + + " \"number_of_in_flight_fetch\" : 0,\n" + + " \"task_max_waiting_in_queue_millis\" : 0,\n" + + " \"active_shards_percent_as_number\" : 100.0\n" + + "}"))); + + // deserialization succeeds — clusterName is null because the workaround is active + assertTrue(healthResponse.clusterName() == null); + + // round-trip currently fails with NullPointerException — this is bug #557 + assertTrue(healthResponse.toString() != null); + } + } } diff --git a/java-client/src/test/java/co/elastic/clients/json/NullSafeJsonGeneratorTest.java b/java-client/src/test/java/co/elastic/clients/json/NullSafeJsonGeneratorTest.java new file mode 100644 index 0000000000..20e56fb79a --- /dev/null +++ b/java-client/src/test/java/co/elastic/clients/json/NullSafeJsonGeneratorTest.java @@ -0,0 +1,140 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package co.elastic.clients.json; + +import co.elastic.clients.testkit.ModelTestCase; +import co.elastic.clients.util.ApiTypeHelper; +import jakarta.json.JsonValue; +import jakarta.json.stream.JsonGenerator; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; + +import java.io.StringWriter; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.util.LinkedHashMap; +import java.util.Map; + +public class NullSafeJsonGeneratorTest extends ModelTestCase { + + @Test + public void testNullReferencesAreWrittenAsJsonNull() { + StringWriter writer = new StringWriter(); + try (JsonGenerator generator = new NullSafeJsonGenerator(mapper.jsonProvider().createGenerator(writer))) { + generator.writeStartObject(); + + // writeKey + write(value) overloads with null reference values + generator.writeKey("string_field"); + generator.write((String) null); + generator.writeKey("decimal_field"); + generator.write((BigDecimal) null); + generator.writeKey("integer_field"); + generator.write((BigInteger) null); + generator.writeKey("json_value_field"); + generator.write((JsonValue) null); + + // name+value overloads with null reference values — DelegatingJsonGenerator splits these into + // writeKey(name) + write(value), so they reach the overrides above via virtual dispatch. + generator.write("named_string", (String) null); + generator.write("named_decimal", (BigDecimal) null); + generator.write("named_integer", (BigInteger) null); + generator.write("named_json_value", (JsonValue) null); + + generator.writeEnd(); + } + + assertEquals( + "{" + + "\"string_field\":null," + + "\"decimal_field\":null," + + "\"integer_field\":null," + + "\"json_value_field\":null," + + "\"named_string\":null," + + "\"named_decimal\":null," + + "\"named_integer\":null," + + "\"named_json_value\":null" + + "}", + writer.toString() + ); + } + + @Test + public void testNonNullValuesPassThrough() { + StringWriter writer = new StringWriter(); + try (JsonGenerator generator = new NullSafeJsonGenerator(mapper.jsonProvider().createGenerator(writer))) { + generator.writeStartObject(); + generator.write("string_field", "hello"); + generator.write("int_field", 42); + generator.write("long_field", 10000000000L); + generator.write("double_field", 1.5); + generator.write("bool_field", true); + generator.write("decimal_field", new BigDecimal("3.14")); + generator.write("integer_field", new BigInteger("12345")); + generator.writeEnd(); + } + + assertEquals( + "{" + + "\"string_field\":\"hello\"," + + "\"int_field\":42," + + "\"long_field\":10000000000," + + "\"double_field\":1.5," + + "\"bool_field\":true," + + "\"decimal_field\":3.14," + + "\"integer_field\":12345" + + "}", + writer.toString() + ); + } + + @Test + public void testToJsonStringWithMapWhileWorkaroundActive() { + // SimpleJsonpMapper only serializes types registered in the Java client and doesn't support arbitrary Maps. + Assumptions.assumeFalse(jsonImpl == JsonImpl.Simple, "SimpleJsonpMapper does not support arbitrary Map serialization"); + Map map = new LinkedHashMap<>(); + map.put("a", 1); + map.put("b", "two"); + + try (ApiTypeHelper.DisabledChecksHandle h = + ApiTypeHelper.DANGEROUS_disableRequiredPropertiesCheck(true)) { + assertEquals("{\"a\":1,\"b\":\"two\"}", JsonpUtils.toJsonString(map, mapper)); + } + } + + @Test + public void testToJsonStringWithPojoWhileWorkaroundActive() { + Assumptions.assumeFalse(jsonImpl == JsonImpl.Simple, "SimpleJsonpMapper does not support POJO serialization"); + SomePojo pojo = new SomePojo(); + pojo.name = "test"; + pojo.value = 42; + + try (ApiTypeHelper.DisabledChecksHandle h = + ApiTypeHelper.DANGEROUS_disableRequiredPropertiesCheck(true)) { + String json = JsonpUtils.toJsonString(pojo, mapper); + assertTrue(json.contains("\"name\":\"test\""), "Expected name field, got: " + json); + assertTrue(json.contains("\"value\":42"), "Expected value field, got: " + json); + } + } + + public static class SomePojo { + public String name; + public int value; + } +}