Skip to content

Commit 4669b5d

Browse files
carterkozakcarterkozak
authored
Fix string-to-long coercion when afterburner is enabled (#1832)
Fix string-to-long coercion when afterburner is enabled
1 parent 946bd0a commit 4669b5d

File tree

6 files changed

+207
-16
lines changed

6 files changed

+207
-16
lines changed

‎changelog/@unreleased/pr-1832.v2.yml

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
type: fix
2+
fix:
3+
description: Fix string-to-long coercion when afterburner is enabled
4+
links:
5+
- https://github.com/palantir/conjure-java-runtime/pull/1832

‎conjure-java-client-verifier/src/test/resources/ignored-test-cases.jersey.yml

-2
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,6 @@ client:
9999
- 'null'
100100
receiveMapEnumExampleAlias:
101101
- 'null'
102-
receiveSafeLongExample: #allowed long coercion applies to safelong
103-
- '{"value":"12"}'
104102

105103
singleHeaderService: {}
106104

‎conjure-java-client-verifier/src/test/resources/ignored-test-cases.retrofit.yml

-2
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,6 @@ client:
136136
- 'null'
137137
receiveMapEnumExampleAlias:
138138
- 'null'
139-
receiveSafeLongExample: #allowed long coercion applies to safelong
140-
- '{"value":"12"}'
141139

142140
singleHeaderService: {}
143141

‎conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/LenientLongModule.java

+94
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/*
2+
* (c) Copyright 2019 Palantir Technologies Inc. All rights reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.palantir.conjure.java.serialization;
18+
19+
import com.fasterxml.jackson.core.JsonParser;
20+
import com.fasterxml.jackson.databind.DeserializationContext;
21+
import com.fasterxml.jackson.databind.cfg.CoercionAction;
22+
import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
23+
import com.fasterxml.jackson.databind.exc.InvalidFormatException;
24+
import com.fasterxml.jackson.databind.module.SimpleModule;
25+
import com.fasterxml.jackson.datatype.jdk8.OptionalLongDeserializer;
26+
import com.palantir.logsafe.exceptions.SafeIoException;
27+
import java.io.IOException;
28+
import java.util.OptionalLong;
29+
30+
/**
31+
* Provides support for the {@link Long} deserialization from JSON string and numeric values regardless of
32+
*
33+
* <pre>MapperFeature.ALLOW_COERCION_OF_SCALARS</pre>
34+
*
35+
* configuration.
36+
*/
37+
final class LenientLongModule extends SimpleModule {
38+
39+
LenientLongModule() {
40+
super("lenient long");
41+
// Register to both Long.TYPE and Long.class
42+
this.addDeserializer(long.class, new LongAsStringDeserializer())
43+
.addDeserializer(Long.class, new LongAsStringDeserializer())
44+
.addDeserializer(OptionalLong.class, new OptionalLongAsStringDeserializer());
45+
}
46+
47+
private static final class LongAsStringDeserializer extends StdDeserializer<Long> {
48+
49+
private LongAsStringDeserializer() {
50+
super(Long.TYPE);
51+
}
52+
53+
@Override
54+
public Long deserialize(JsonParser jsonParser, DeserializationContext _ctxt) throws IOException {
55+
switch (jsonParser.currentToken()) {
56+
case VALUE_NUMBER_INT:
57+
return jsonParser.getLongValue();
58+
case VALUE_STRING:
59+
return parseLong(jsonParser);
60+
case VALUE_NULL:
61+
return null;
62+
default:
63+
throw new SafeIoException("Expected a long value");
64+
}
65+
}
66+
67+
@Override
68+
public boolean isCachable() {
69+
return true;
70+
}
71+
72+
private static Long parseLong(JsonParser jsonParser) throws IOException {
73+
String value = jsonParser.getValueAsString();
74+
try {
75+
return Long.valueOf(value);
76+
} catch (NumberFormatException e) {
77+
InvalidFormatException failure =
78+
new InvalidFormatException(jsonParser, "not a valid long value", value, long.class);
79+
failure.initCause(e);
80+
throw failure;
81+
}
82+
}
83+
}
84+
85+
private static final class OptionalLongAsStringDeserializer extends OptionalLongDeserializer {
86+
87+
private OptionalLongAsStringDeserializer() {}
88+
89+
@Override
90+
protected CoercionAction _checkFromStringCoercion(DeserializationContext _ctxt, String _value) {
91+
return CoercionAction.TryConvert;
92+
}
93+
}
94+
}

‎conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java

+1-12
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,12 @@
2020
import com.fasterxml.jackson.databind.MapperFeature;
2121
import com.fasterxml.jackson.databind.ObjectMapper;
2222
import com.fasterxml.jackson.databind.SerializationFeature;
23-
import com.fasterxml.jackson.databind.cfg.CoercionAction;
24-
import com.fasterxml.jackson.databind.cfg.CoercionInputShape;
2523
import com.fasterxml.jackson.dataformat.cbor.CBORFactory;
2624
import com.fasterxml.jackson.dataformat.smile.SmileFactory;
2725
import com.fasterxml.jackson.datatype.guava.GuavaModule;
2826
import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
2927
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
3028
import com.fasterxml.jackson.module.afterburner.AfterburnerModule;
31-
import java.util.OptionalLong;
3229

3330
public final class ObjectMappers {
3431

@@ -131,14 +128,12 @@ public static ObjectMapper newSmileServerObjectMapper() {
131128
* </ul>
132129
*/
133130
public static ObjectMapper withDefaultModules(ObjectMapper mapper) {
134-
allowStringCoercion(mapper, long.class);
135-
allowStringCoercion(mapper, Long.class);
136-
allowStringCoercion(mapper, OptionalLong.class);
137131
return mapper.registerModule(new GuavaModule())
138132
.registerModule(new ShimJdk7Module())
139133
.registerModule(new Jdk8Module().configureAbsentsAsNulls(true))
140134
.registerModule(new AfterburnerModule())
141135
.registerModule(new JavaTimeModule())
136+
.registerModule(new LenientLongModule())
142137
.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
143138
.disable(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS)
144139
.disable(DeserializationFeature.ADJUST_DATES_TO_CONTEXT_TIME_ZONE)
@@ -148,10 +143,4 @@ public static ObjectMapper withDefaultModules(ObjectMapper mapper) {
148143
.disable(MapperFeature.ALLOW_COERCION_OF_SCALARS)
149144
.disable(DeserializationFeature.ACCEPT_FLOAT_AS_INT);
150145
}
151-
152-
private static void allowStringCoercion(ObjectMapper mapper, Class<?> clazz) {
153-
mapper.coercionConfigFor(clazz)
154-
.setAcceptBlankAsEmpty(false)
155-
.setCoercion(CoercionInputShape.String, CoercionAction.TryConvert);
156-
}
157146
}

‎conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java

+107
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@
1919
import static org.assertj.core.api.Assertions.assertThat;
2020
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2121

22+
import com.fasterxml.jackson.annotation.JsonProperty;
2223
import com.fasterxml.jackson.core.JsonParseException;
2324
import com.fasterxml.jackson.core.JsonProcessingException;
2425
import com.fasterxml.jackson.core.exc.InputCoercionException;
2526
import com.fasterxml.jackson.core.type.TypeReference;
2627
import com.fasterxml.jackson.databind.ObjectMapper;
2728
import com.fasterxml.jackson.databind.exc.InvalidFormatException;
29+
import com.palantir.logsafe.Preconditions;
2830
import java.io.File;
2931
import java.io.IOException;
3032
import java.math.BigInteger;
@@ -38,6 +40,7 @@
3840
import java.time.ZonedDateTime;
3941
import java.util.Collections;
4042
import java.util.Map;
43+
import java.util.Objects;
4144
import java.util.Optional;
4245
import java.util.OptionalInt;
4346
import java.util.OptionalLong;
@@ -128,11 +131,115 @@ public void testLongTypeDeserializationFromString() throws IOException {
128131
assertThat(MAPPER.readValue("\"1\"", Long.TYPE)).isEqualTo(1L);
129132
}
130133

134+
@Test
135+
public void testLongBeanTypeDeserializationFromString() throws IOException {
136+
assertThat(MAPPER.readValue("{\"value\":\"1\"}", LongBean.class)).isEqualTo(new LongBean(1L));
137+
}
138+
139+
@Test
140+
public void testLongBeanTypeDeserializationFromNumber() throws IOException {
141+
assertThat(MAPPER.readValue("{\"value\":\"1\"}", LongBean.class)).isEqualTo(new LongBean(1L));
142+
}
143+
144+
static final class LongBean {
145+
@JsonProperty
146+
private long value;
147+
148+
LongBean() {}
149+
150+
LongBean(long value) {
151+
setValue(value);
152+
}
153+
154+
public long getValue() {
155+
return value;
156+
}
157+
158+
public void setValue(long value) {
159+
this.value = value;
160+
}
161+
162+
@Override
163+
public boolean equals(Object other) {
164+
if (this == other) {
165+
return true;
166+
}
167+
if (other == null || getClass() != other.getClass()) {
168+
return false;
169+
}
170+
LongBean that = (LongBean) other;
171+
return value == that.value;
172+
}
173+
174+
@Override
175+
public int hashCode() {
176+
return Objects.hashCode(value);
177+
}
178+
179+
@Override
180+
public String toString() {
181+
return "LongBean{value=" + value + '}';
182+
}
183+
}
184+
131185
@Test
132186
public void testOptionalLongTypeDeserializationFromString() throws IOException {
133187
assertThat(MAPPER.readValue("\"1\"", OptionalLong.class)).hasValue(1L);
134188
}
135189

190+
@Test
191+
public void testOptionalLongBeanTypeDeserializationFromString() throws IOException {
192+
assertThat(MAPPER.readValue("{\"value\":\"1\"}", OptionalLongBean.class))
193+
.isEqualTo(new OptionalLongBean(OptionalLong.of(1L)));
194+
}
195+
196+
@Test
197+
public void testOptionalLongBeanTypeDeserializationFromNumber() throws IOException {
198+
assertThat(MAPPER.readValue("{\"value\":1}", OptionalLongBean.class))
199+
.isEqualTo(new OptionalLongBean(OptionalLong.of(1L)));
200+
}
201+
202+
static final class OptionalLongBean {
203+
@JsonProperty
204+
private OptionalLong value = OptionalLong.empty();
205+
206+
OptionalLongBean() {}
207+
208+
OptionalLongBean(OptionalLong value) {
209+
setValue(value);
210+
}
211+
212+
public OptionalLong getValue() {
213+
return value;
214+
}
215+
216+
public void setValue(OptionalLong value) {
217+
this.value = Preconditions.checkNotNull(value, "value");
218+
}
219+
220+
@Override
221+
public boolean equals(Object other) {
222+
if (this == other) {
223+
return true;
224+
}
225+
if (other == null || getClass() != other.getClass()) {
226+
return false;
227+
}
228+
OptionalLongBean that = (OptionalLongBean) other;
229+
return value.equals(that.value);
230+
}
231+
232+
@Override
233+
public int hashCode() {
234+
return Objects.hashCode(value);
235+
}
236+
237+
@Override
238+
public String toString() {
239+
return "OptionalLongBean{value=" + value + '}';
240+
}
241+
}
242+
136243
@Test
137244
public void testLongDeserializationFromJsonNumber() throws IOException {
138245
assertThat(MAPPER.readValue("1", Long.class)).isEqualTo(1L);

0 commit comments

Comments
 (0)