Skip to content

Commit f6bccd6

Browse files
author
Sheridan C Rawlins
committed
Fixes #15: JsonJacksonFormat: unsigned int and long aren't output/input as unsigned.
1 parent 7f82645 commit f6bccd6

File tree

5 files changed

+88
-55
lines changed

5 files changed

+88
-55
lines changed

‎src/main/java/com/googlecode/protobuf/format/JsonJacksonFormat.java‎

Lines changed: 44 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
3333
import java.io.IOException;
3434
import java.io.InputStream;
3535
import java.io.OutputStream;
36+
import java.math.BigInteger;
3637
import java.nio.charset.Charset;
3738
import java.util.Iterator;
3839
import java.util.List;
@@ -67,8 +68,10 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
6768
*/
6869
public class JsonJacksonFormat extends ProtobufFormatter {
6970
private static JsonFactory jsonFactory = new JsonFactory();
70-
71-
71+
private static final long MAX_UINT_VALUE = (((long) Integer.MAX_VALUE) << 1) + 1;
72+
private static final BigInteger MAX_ULONG_VALUE =
73+
BigInteger.valueOf(Long.MAX_VALUE).shiftLeft(1).add(BigInteger.ONE);
74+
7275
/**
7376
* Outputs a Smile representation of the Protocol Message supplied into the parameter output.
7477
* (This representation is the new version of the classic "ProtocolPrinter" output from the
@@ -79,7 +82,7 @@ public void print(final Message message, OutputStream output, Charset cs) throws
7982
print(message, generator);
8083
generator.close();
8184
}
82-
85+
8386
/**
8487
* Outputs a Smile representation of the Protocol Message supplied into the parameter output.
8588
* (This representation is the new version of the classic "ProtocolPrinter" output from the
@@ -102,29 +105,29 @@ public void print(final UnknownFieldSet fields, OutputStream output, Charset cs)
102105
generator.writeEndObject();
103106
generator.close();
104107
}
105-
106-
108+
109+
107110
/**
108111
* Parse a text-format message from {@code input} and merge the contents into {@code builder}.
109112
* Extensions will be recognized if they are registered in {@code extensionRegistry}.
110-
* @throws IOException
113+
* @throws IOException
111114
*/
112115
public void merge(InputStream input, Charset cs,
113116
ExtensionRegistry extensionRegistry, Message.Builder builder) throws IOException {
114-
117+
115118
JsonParser parser = jsonFactory.createJsonParser(input);
116119
merge(parser, extensionRegistry, builder);
117120
}
118-
121+
119122
/**
120123
* Parse a text-format message from {@code input} and merge the contents into {@code builder}.
121124
* Extensions will be recognized if they are registered in {@code extensionRegistry}.
122-
* @throws IOException
125+
* @throws IOException
123126
*/
124-
public void merge(JsonParser parser,
127+
public void merge(JsonParser parser,
125128
ExtensionRegistry extensionRegistry,
126129
Message.Builder builder) throws IOException {
127-
130+
128131
JsonToken token = parser.nextToken();
129132
if (token.equals(JsonToken.START_OBJECT)) {
130133
token = parser.nextToken();
@@ -133,22 +136,22 @@ public void merge(JsonParser parser,
133136
mergeField(parser, extensionRegistry, builder);
134137
token = parser.nextToken();
135138
}
136-
139+
137140
// Test to make sure the tokenizer has reached the end of the stream.
138141
if (parser.nextToken() != null) {
139142
throw new RuntimeException("Expecting the end of the stream, but there seems to be more data! Check the input for a valid JSON format.");
140143
}
141144
}
142-
143-
144-
145+
146+
147+
145148
protected JsonGenerator createGenerator(OutputStream output) throws IOException {
146149
JsonGenerator generator = jsonFactory.createJsonGenerator(output, JsonEncoding.UTF8);
147150
generator.disable(JsonGenerator.Feature.AUTO_CLOSE_TARGET);
148151
return generator;
149152
}
150153

151-
154+
152155
protected void printMessage(Message message, JsonGenerator generator) throws IOException {
153156

154157
for (Iterator<Map.Entry<FieldDescriptor, Object>> iter = message.getAllFields().entrySet().iterator(); iter.hasNext();) {
@@ -207,29 +210,29 @@ private void printFieldValue(FieldDescriptor field, Object value, JsonGenerator
207210
case SFIXED32:
208211
generator.writeNumber((Integer)value);
209212
break;
210-
213+
211214
case INT64:
212215
case SINT64:
213216
case SFIXED64:
214217
generator.writeNumber((Long)value);
215218
break;
216-
219+
217220
case FLOAT:
218221
generator.writeNumber((Float)value);
219222
break;
220-
223+
221224
case DOUBLE:
222225
generator.writeNumber((Double)value);
223226
break;
224-
227+
225228
case BOOL:
226229
// Good old toString() does what we want for these types.
227230
generator.writeBoolean((Boolean)value);
228231
break;
229232

230233
case UINT32:
231234
case FIXED32:
232-
generator.writeNumber(unsignedInt((Integer) value));
235+
generator.writeNumber(Integer.toUnsignedLong((Integer)value));
233236
break;
234237

235238
case UINT64:
@@ -264,7 +267,7 @@ private void printFieldValue(FieldDescriptor field, Object value, JsonGenerator
264267
protected void printUnknownFields(UnknownFieldSet unknownFields, JsonGenerator generator) throws IOException {
265268
for (Map.Entry<Integer, UnknownFieldSet.Field> entry : unknownFields.asMap().entrySet()) {
266269
UnknownFieldSet.Field field = entry.getValue();
267-
270+
268271
generator.writeArrayFieldStart(entry.getKey().toString());
269272
for (long value : field.getVarintList()) {
270273
generator.writeNumber(value);
@@ -292,13 +295,13 @@ protected void printUnknownFields(UnknownFieldSet unknownFields, JsonGenerator g
292295

293296
// =================================================================
294297
// Parsing
295-
298+
296299

297300
/**
298301
* Parse a single field from {@code parser} and merge it into {@code builder}. If a ',' is
299302
* detected after the field ends, the next field will be parsed automatically
300-
* @throws IOException
301-
* @throws JsonParseException
303+
* @throws IOException
304+
* @throws JsonParseException
302305
*/
303306
protected void mergeField(JsonParser parser,
304307
ExtensionRegistry extensionRegistry,
@@ -311,7 +314,7 @@ protected void mergeField(JsonParser parser,
311314

312315
if (token != null) {
313316
String name = parser.getCurrentName();
314-
317+
315318
if (name.contains(".")) {
316319
// should be an extension
317320
extension = extensionRegistry.findExtensionByName(name);
@@ -366,7 +369,7 @@ protected void mergeField(JsonParser parser,
366369

367370
if (field != null) {
368371
token = parser.nextToken();
369-
372+
370373
boolean array = token.equals(JsonToken.START_ARRAY);
371374

372375
if (array) {
@@ -384,7 +387,7 @@ protected void mergeField(JsonParser parser,
384387
private void handleMissingField(String fieldName, JsonParser parser,
385388
ExtensionRegistry extensionRegistry,
386389
UnknownFieldSet.Builder builder) throws IOException {
387-
390+
388391
JsonToken token = parser.nextToken();
389392
if (token.equals(JsonToken.START_OBJECT)) {
390393
// Message structure
@@ -430,13 +433,13 @@ private void handleValue(JsonParser parser,
430433

431434
private Object handlePrimitive(JsonParser parser, FieldDescriptor field) throws IOException {
432435
Object value = null;
433-
436+
434437
JsonToken token = parser.getCurrentToken();
435-
438+
436439
if (token.equals(JsonToken.VALUE_NULL)) {
437440
return value;
438441
}
439-
442+
440443
switch (field.getType()) {
441444
case INT32:
442445
case SINT32:
@@ -452,20 +455,21 @@ private Object handlePrimitive(JsonParser parser, FieldDescriptor field) throws
452455

453456
case UINT32:
454457
case FIXED32:
455-
int valueInt = parser.getIntValue();
456-
if (valueInt < 0) {
457-
throw new NumberFormatException("Number must be positive: " + valueInt);
458+
long valueLong = parser.getLongValue();
459+
if (valueLong < 0 || valueLong > MAX_UINT_VALUE) {
460+
throw new NumberFormatException("Number must be positive: " + valueLong);
458461
}
459-
value = valueInt;
462+
value = (int) valueLong;
460463
break;
461464

462465
case UINT64:
463466
case FIXED64:
464-
long valueLong = parser.getLongValue();
465-
if (valueLong < 0) {
466-
throw new NumberFormatException("Number must be positive: " + valueLong);
467+
BigInteger valueBigInt = parser.getBigIntegerValue();
468+
// valueBigInt < 0 || valueBigInt > MAX_ULONG_VALUE
469+
if (valueBigInt.compareTo(BigInteger.ZERO) == -1 || valueBigInt.compareTo(MAX_ULONG_VALUE) == 1) {
470+
throw new NumberFormatException("Number must be positive: " + valueBigInt);
467471
}
468-
value = valueLong;
472+
value = valueBigInt.longValue();
469473
break;
470474

471475
case FLOAT:
@@ -518,7 +522,7 @@ private Object handlePrimitive(JsonParser parser, FieldDescriptor field) throws
518522
}
519523
return value;
520524
}
521-
525+
522526

523527
private Object handleObject(JsonParser parser,
524528
ExtensionRegistry extensionRegistry,

‎src/main/java/com/googlecode/protobuf/format/util/TextUtils.java‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,27 +55,17 @@ public static String unsignedToString(final int value) {
5555
return Long.toString((value) & 0x00000000FFFFFFFFL);
5656
}
5757
}
58-
59-
/**
60-
* Convert an unsigned 32-bit integer to a string.
61-
*/
62-
public static Integer unsignedInt(int value) {
63-
if (value < 0) {
64-
return (int) ((value) & 0x00000000FFFFFFFFL);
65-
}
66-
return value;
67-
}
6858

6959
/**
70-
* Convert an unsigned 64-bit integer to a string.
60+
* Convert an unsigned 64-bit integer to a {@link BigInteger}.
7161
*/
72-
public static Long unsignedLong(long value) {
62+
public static BigInteger unsignedLong(long value) {
7363
if (value < 0) {
7464
// Pull off the most-significant bit so that BigInteger doesn't think
7565
// the number is negative, then set it again using setBit().
76-
return BigInteger.valueOf(value & 0x7FFFFFFFFFFFFFFFL).setBit(63).longValue();
66+
return BigInteger.valueOf(value & 0x7FFFFFFFFFFFFFFFL).setBit(63);
7767
}
78-
return value;
68+
return BigInteger.valueOf(value);
7969
}
8070

8171
/**

‎src/test/java/com/googlecode/protobuf/format/JsonJacksonFormatTest.java‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,16 @@
22

33
import com.fasterxml.jackson.core.JsonFactory;
44
import com.fasterxml.jackson.core.JsonGenerator;
5+
import com.fasterxml.jackson.core.JsonParser;
56
import com.fasterxml.jackson.core.util.DefaultPrettyPrinter;
7+
import com.google.protobuf.ExtensionRegistry;
8+
import org.testng.annotations.DataProvider;
69
import org.testng.annotations.Test;
710
import org.testng.reporters.Files;
811
import protobuf_unittest.UnittestProto;
912

1013
import java.io.StringWriter;
14+
import java.math.BigInteger;
1115

1216
import static org.hamcrest.CoreMatchers.is;
1317
import static org.hamcrest.MatcherAssert.assertThat;
@@ -18,17 +22,50 @@
1822
*/
1923
@Test
2024
public class JsonJacksonFormatTest {
25+
private static final JsonFactory JSON_FACTORY = new JsonFactory();
26+
2127
@Test
2228
public void testPrettyPrint() throws Exception {
2329
StringWriter writer = new StringWriter();
24-
JsonGenerator jsonGenerator = new JsonFactory().createGenerator(writer)
30+
JsonGenerator jsonGenerator = JSON_FACTORY.createGenerator(writer)
2531
.setPrettyPrinter(new DefaultPrettyPrinter());
2632
JsonJacksonFormat jsonJacksonFormat =
2733
(JsonJacksonFormat) new FormatFactory().createFormatter(FormatFactory.Formatter.JSON_JACKSON);
2834
jsonJacksonFormat.print(UnittestProto.TestAllTypes.newBuilder()
2935
.setOptionalForeignEnum(UnittestProto.ForeignEnum.FOREIGN_BAR)
36+
.setOptionalFloat(0)
3037
.build(), jsonGenerator);
38+
jsonGenerator.close();
3139
assertThat(writer.toString(), is(Files.readFile(JsonJacksonFormatTest.class.getResourceAsStream(
3240
"/expectations/JsonJacksonFormatTest/prettyprint.json")).trim()));
3341
}
42+
43+
@Test
44+
public void testUnsignedTypes() throws Exception {
45+
StringWriter writer = new StringWriter();
46+
JsonGenerator jsonGenerator = JSON_FACTORY.createGenerator(writer);
47+
JsonJacksonFormat jsonJacksonFormat =
48+
(JsonJacksonFormat) new FormatFactory().createFormatter(FormatFactory.Formatter.JSON_JACKSON);
49+
final long maxIntAsLong = ((long) Integer.MAX_VALUE) << 1;
50+
final BigInteger maxLongAsBigInt = BigInteger.valueOf(Long.MAX_VALUE).shiftLeft(1);
51+
jsonJacksonFormat.print(UnittestProto.TestAllTypes.newBuilder()
52+
.setOptionalUint32((int) maxIntAsLong)
53+
.setOptionalFixed32((int) maxIntAsLong)
54+
.setOptionalUint64(maxLongAsBigInt.longValue())
55+
.setOptionalFixed64(maxLongAsBigInt.longValue())
56+
.build(), jsonGenerator);
57+
jsonGenerator.close();
58+
assertThat(writer.toString(), is(Files.readFile(JsonJacksonFormatTest.class.getResourceAsStream(
59+
"/expectations/JsonJacksonFormatTest/unsigneds.json")).trim()));
60+
JsonParser jsonParser = JSON_FACTORY.createParser(writer.toString());
61+
62+
UnittestProto.TestAllTypes.Builder testAllTypesBuilder = UnittestProto.TestAllTypes.newBuilder();
63+
ExtensionRegistry extensionRegistry = ExtensionRegistry.newInstance();
64+
UnittestProto.registerAllExtensions(extensionRegistry);
65+
jsonJacksonFormat.merge(jsonParser, extensionRegistry, testAllTypesBuilder);
66+
assertThat(testAllTypesBuilder.getOptionalUint32(), is((int) maxIntAsLong));
67+
assertThat(testAllTypesBuilder.getOptionalFixed32(), is((int) maxIntAsLong));
68+
assertThat(testAllTypesBuilder.getOptionalUint64(), is(maxLongAsBigInt.longValue()));
69+
assertThat(testAllTypesBuilder.getOptionalFixed64(), is(maxLongAsBigInt.longValue()));
70+
}
3471
}

‎src/test/resources/expectations/JsonJacksonFormatTest/prettyprint.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
{
2+
"optional_float" : 0.0,
23
"optional_foreign_enum" : "FOREIGN_BAR"
34
}

‎src/test/resources/expectations/JsonJacksonFormatTest/unsigneds.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"optional_uint32":4294967294,"optional_uint64":18446744073709551614,"optional_fixed32":4294967294,"optional_fixed64":18446744073709551614}

0 commit comments

Comments
 (0)