From 3239a08915bc5c7b48bbed696065717438dea21b Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Wed, 2 Dec 2020 09:16:49 -0500 Subject: [PATCH 1/2] Fix string-to-long coercion when afterburner is enabled Reported upstream: https://github.com/FasterXML/jackson-modules-base/issues/120 The CoercionConfig is fantastic, unfortunately it is not yet supported by the AfterBurner module. Separately we may consider moving to the new Blackbird module on java 11 runtimes. For now I've updated our existing long deserializers for 2.12.0 which has the benefit of rejecting coercion from string to SafeLong. --- .../resources/ignored-test-cases.jersey.yml | 2 - .../resources/ignored-test-cases.retrofit.yml | 2 - .../java/serialization/LenientLongModule.java | 94 +++++++++++++++ .../java/serialization/ObjectMappers.java | 13 +-- .../java/serialization/ObjectMappersTest.java | 107 ++++++++++++++++++ 5 files changed, 202 insertions(+), 16 deletions(-) create mode 100644 conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/LenientLongModule.java diff --git a/conjure-java-client-verifier/src/test/resources/ignored-test-cases.jersey.yml b/conjure-java-client-verifier/src/test/resources/ignored-test-cases.jersey.yml index 032f2e2c6..ceca02cb3 100644 --- a/conjure-java-client-verifier/src/test/resources/ignored-test-cases.jersey.yml +++ b/conjure-java-client-verifier/src/test/resources/ignored-test-cases.jersey.yml @@ -99,8 +99,6 @@ client: - 'null' receiveMapEnumExampleAlias: - 'null' - receiveSafeLongExample: #allowed long coercion applies to safelong - - '{"value":"12"}' singleHeaderService: {} diff --git a/conjure-java-client-verifier/src/test/resources/ignored-test-cases.retrofit.yml b/conjure-java-client-verifier/src/test/resources/ignored-test-cases.retrofit.yml index 75c18394d..9c3574822 100644 --- a/conjure-java-client-verifier/src/test/resources/ignored-test-cases.retrofit.yml +++ b/conjure-java-client-verifier/src/test/resources/ignored-test-cases.retrofit.yml @@ -136,8 +136,6 @@ client: - 'null' receiveMapEnumExampleAlias: - 'null' - receiveSafeLongExample: #allowed long coercion applies to safelong - - '{"value":"12"}' singleHeaderService: {} diff --git a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/LenientLongModule.java b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/LenientLongModule.java new file mode 100644 index 000000000..6c08ea92e --- /dev/null +++ b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/LenientLongModule.java @@ -0,0 +1,94 @@ +/* + * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.conjure.java.serialization; + +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.cfg.CoercionAction; +import com.fasterxml.jackson.databind.deser.std.StdDeserializer; +import com.fasterxml.jackson.databind.exc.InvalidFormatException; +import com.fasterxml.jackson.databind.module.SimpleModule; +import com.fasterxml.jackson.datatype.jdk8.OptionalLongDeserializer; +import com.palantir.logsafe.exceptions.SafeIoException; +import java.io.IOException; +import java.util.OptionalLong; + +/** + * Provides support for the {@link Long} deserialization from JSON string and numeric values regardless of + * + * 
MapperFeature.ALLOW_COERCION_OF_SCALARS
+ * + * configuration. + */ +final class LenientLongModule extends SimpleModule { + + LenientLongModule() { + super("lenient long"); + // Register to both Long.TYPE and Long.class + this.addDeserializer(long.class, new LongAsStringDeserializer()) + .addDeserializer(Long.class, new LongAsStringDeserializer()) + .addDeserializer(OptionalLong.class, new OptionalLongAsStringDeserializer()); + } + + private static final class LongAsStringDeserializer extends StdDeserializer { + + private LongAsStringDeserializer() { + super(Long.TYPE); + } + + @Override + public Long deserialize(JsonParser jsonParser, DeserializationContext _ctxt) throws IOException { + switch (jsonParser.currentToken()) { + case VALUE_NUMBER_INT: + return jsonParser.getLongValue(); + case VALUE_STRING: + return parseLong(jsonParser); + case VALUE_NULL: + return null; + default: + throw new SafeIoException("Expected a long value"); + } + } + + @Override + public boolean isCachable() { + return true; + } + + private static Long parseLong(JsonParser jsonParser) throws IOException { + String value = jsonParser.getValueAsString(); + try { + return Long.valueOf(value); + } catch (NumberFormatException e) { + InvalidFormatException failure = + new InvalidFormatException(jsonParser, "not a valid long value", value, long.class); + failure.initCause(e); + throw failure; + } + } + } + + private static final class OptionalLongAsStringDeserializer extends OptionalLongDeserializer { + + private OptionalLongAsStringDeserializer() {} + + @Override + protected CoercionAction _checkFromStringCoercion(DeserializationContext _ctxt, String _value) { + return CoercionAction.TryConvert; + } + } +} diff --git a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java index 4161030f5..916647ee0 100644 --- a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java +++ b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java @@ -20,15 +20,12 @@ import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.SerializationFeature; -import com.fasterxml.jackson.databind.cfg.CoercionAction; -import com.fasterxml.jackson.databind.cfg.CoercionInputShape; import com.fasterxml.jackson.dataformat.cbor.CBORFactory; import com.fasterxml.jackson.dataformat.smile.SmileFactory; import com.fasterxml.jackson.datatype.guava.GuavaModule; import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import com.fasterxml.jackson.module.afterburner.AfterburnerModule; -import java.util.OptionalLong; public final class ObjectMappers { @@ -131,14 +128,12 @@ public static ObjectMapper newSmileServerObjectMapper() { * */ public static ObjectMapper withDefaultModules(ObjectMapper mapper) { - allowStringCoercion(mapper, long.class); - allowStringCoercion(mapper, Long.class); - allowStringCoercion(mapper, OptionalLong.class); return mapper.registerModule(new GuavaModule()) .registerModule(new ShimJdk7Module()) .registerModule(new Jdk8Module().configureAbsentsAsNulls(true)) .registerModule(new AfterburnerModule()) .registerModule(new JavaTimeModule()) + .registerModule(new LenientLongModule()) .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) .disable(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS) .disable(DeserializationFeature.ADJUST_DATES_TO_CONTEXT_TIME_ZONE) @@ -148,10 +143,4 @@ public static ObjectMapper withDefaultModules(ObjectMapper mapper) { .disable(MapperFeature.ALLOW_COERCION_OF_SCALARS) .disable(DeserializationFeature.ACCEPT_FLOAT_AS_INT); } - - private static void allowStringCoercion(ObjectMapper mapper, Class clazz) { - mapper.coercionConfigFor(clazz) - .setAcceptBlankAsEmpty(false) - .setCoercion(CoercionInputShape.String, CoercionAction.TryConvert); - } } diff --git a/conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java b/conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java index cf01ec487..2a4d8bc0b 100644 --- a/conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java +++ b/conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java @@ -19,12 +19,14 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.exc.InputCoercionException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.exc.InvalidFormatException; +import com.palantir.logsafe.Preconditions; import java.io.File; import java.io.IOException; import java.math.BigInteger; @@ -38,6 +40,7 @@ import java.time.ZonedDateTime; import java.util.Collections; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.OptionalInt; import java.util.OptionalLong; @@ -128,11 +131,115 @@ public void testLongTypeDeserializationFromString() throws IOException { assertThat(MAPPER.readValue("\"1\"", Long.TYPE)).isEqualTo(1L); } + @Test + public void testLongBeanTypeDeserializationFromString() throws IOException { + assertThat(MAPPER.readValue("{\"value\":\"1\"}", LongBean.class)).isEqualTo(new LongBean(1L)); + } + + @Test + public void testLongBeanTypeDeserializationFromNumber() throws IOException { + assertThat(MAPPER.readValue("{\"value\":\"1\"}", LongBean.class)).isEqualTo(new LongBean(1L)); + } + + static final class LongBean { + @JsonProperty + private long value; + + LongBean() {} + + LongBean(long value) { + setValue(value); + } + + public long getValue() { + return value; + } + + public void setValue(long value) { + this.value = value; + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (other == null || getClass() != other.getClass()) { + return false; + } + LongBean that = (LongBean) other; + return value == that.value; + } + + @Override + public int hashCode() { + return Objects.hashCode(value); + } + + @Override + public String toString() { + return "LongBean{value=" + value + '}'; + } + } + @Test public void testOptionalLongTypeDeserializationFromString() throws IOException { assertThat(MAPPER.readValue("\"1\"", OptionalLong.class)).hasValue(1L); } + @Test + public void testOptionalLongBeanTypeDeserializationFromString() throws IOException { + assertThat(MAPPER.readValue("{\"value\":\"1\"}", OptionalLongBean.class)) + .isEqualTo(new OptionalLongBean(OptionalLong.of(1L))); + } + + @Test + public void testOptionalLongBeanTypeDeserializationFromNumber() throws IOException { + assertThat(MAPPER.readValue("{\"value\":1}", OptionalLongBean.class)) + .isEqualTo(new OptionalLongBean(OptionalLong.of(1L))); + } + + static final class OptionalLongBean { + @JsonProperty + private OptionalLong value = OptionalLong.empty(); + + OptionalLongBean() {} + + OptionalLongBean(OptionalLong value) { + setValue(value); + } + + public OptionalLong getValue() { + return value; + } + + public void setValue(OptionalLong value) { + this.value = Preconditions.checkNotNull(value, "value"); + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (other == null || getClass() != other.getClass()) { + return false; + } + OptionalLongBean that = (OptionalLongBean) other; + return value.equals(that.value); + } + + @Override + public int hashCode() { + return Objects.hashCode(value); + } + + @Override + public String toString() { + return "OptionalLongBean{value=" + value + '}'; + } + } + @Test public void testLongDeserializationFromJsonNumber() throws IOException { assertThat(MAPPER.readValue("1", Long.class)).isEqualTo(1L); From 3ef6d0df6b81ee32cec5790c26c9d8a24ae59a71 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Wed, 2 Dec 2020 14:16:49 +0000 Subject: [PATCH 2/2] Add generated changelog entries --- changelog/@unreleased/pr-1832.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-1832.v2.yml diff --git a/changelog/@unreleased/pr-1832.v2.yml b/changelog/@unreleased/pr-1832.v2.yml new file mode 100644 index 000000000..a20c5b88b --- /dev/null +++ b/changelog/@unreleased/pr-1832.v2.yml @@ -0,0 +1,5 @@ +type: fix +fix: + description: Fix string-to-long coercion when afterburner is enabled + links: + - https://github.com/palantir/conjure-java-runtime/pull/1832