Skip to content

Commit

Permalink
Fixes #462: ProtobufParser.currentName() returns wrong value (#463)
Browse files Browse the repository at this point in the history
  • Loading branch information
@cowtowncoder
cowtowncoder authored Jan 18, 2024
1 parent a2b521f commit 07de9d1
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 27 deletions.
13 changes: 9 additions & 4 deletions protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,11 @@ public void close() throws IOException
_releaseBuffers();
}
_ioContext.close();
// 17-Jan-2024, tatu: Most code paths won't update context so:
if (!_parsingContext.inRoot()) {
_parsingContext = _parsingContext.getParent();
}
_parsingContext.setCurrentName(null);
}
}

Expand Down Expand Up @@ -598,7 +603,7 @@ public JsonToken nextToken() throws IOException
return t;
}
case STATE_NESTED_KEY:
if (_checkEnd()) {
if (_checkEnd()) { // will update _parsingContext
return (_currToken = JsonToken.END_OBJECT);
}
return _handleNestedKey(_decodeVInt());
Expand Down Expand Up @@ -967,7 +972,7 @@ private JsonToken _skipUnknownField(int tag, int wireType) throws IOException
_skipUnknownValue(wireType);
// 05-Dec-2017, tatu: as per [#126] seems like we need to check this not just for
// STATE_NESTED_KEY but for arrays too at least?
if (_checkEnd()) {
if (_checkEnd()) { // updates _parsingContext
return (_currToken = JsonToken.END_OBJECT);
}
if (_state == STATE_NESTED_KEY) {
Expand Down Expand Up @@ -1069,7 +1074,7 @@ public boolean nextFieldName(SerializableString sstr) throws IOException
return name.equals(sstr.getValue());
}
if (_state == STATE_NESTED_KEY) {
if (_checkEnd()) {
if (_checkEnd()) { // updates _parsingContext
_currToken = JsonToken.END_OBJECT;
return false;
}
Expand Down Expand Up @@ -1153,7 +1158,7 @@ public String nextFieldName() throws IOException
return name;
}
if (_state == STATE_NESTED_KEY) {
if (_checkEnd()) {
if (_checkEnd()) { // updates _parsingContext
_currToken = JsonToken.END_OBJECT;
return null;
}
Expand Down
98 changes: 78 additions & 20 deletions protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/ReadSimpleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,13 @@ public NamedStrings(String n, String... v) {

private final ObjectMapper MAPPER = newObjectMapper();

public void testReadPointInt() throws Exception
public void testReadPointIntAsPOJO() throws Exception
{
ProtobufSchema schema = ProtobufSchemaLoader.std.parse(PROTOC_BOX, "Point");
final ObjectWriter w = MAPPER.writerFor(Point.class)
.with(schema);
Point input = new Point(151, -444);
byte[] bytes = w.writeValueAsBytes(input);
assertNotNull(bytes);

// 6 bytes: 1 byte tags, 2 byte values
assertEquals(6, bytes.length);
Expand All @@ -79,6 +78,16 @@ public void testReadPointInt() throws Exception
assertNotNull(result);
assertEquals(input.x, result.x);
assertEquals(input.y, result.y);
}

public void testReadPointIntStreaming() throws Exception
{
ProtobufSchema schema = ProtobufSchemaLoader.std.parse(PROTOC_BOX, "Point");
final ObjectWriter w = MAPPER.writerFor(Point.class)
.with(schema);
Point input = new Point(151, -444);
byte[] bytes = w.writeValueAsBytes(input);
assertEquals(6, bytes.length);

// actually let's also try via streaming parser
JsonParser p = MAPPER.getFactory().createParser(bytes);
Expand All @@ -98,9 +107,9 @@ public void testReadPointInt() throws Exception
assertEquals("y", p.currentName());
assertEquals(input.y, p.getIntValue());
assertToken(JsonToken.END_OBJECT, p.nextToken());
// 17-Jan-2024, tatu: This should return `null` but somehow return "y"?
// assertNull(p.currentName());
assertNull(p.currentName());
p.close();
assertNull(p.currentName());
}

public void testReadPointLong() throws Exception
Expand All @@ -121,21 +130,24 @@ public void testReadPointLong() throws Exception
assertEquals(input.y, result.y);

// actually let's also try via streaming parser
JsonParser p = MAPPER.getFactory().createParser(bytes);
p.setSchema(schema);
assertToken(JsonToken.START_OBJECT, p.nextToken());
assertToken(JsonToken.FIELD_NAME, p.nextToken());
assertEquals("x", p.currentName());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals(NumberType.LONG, p.getNumberType());
assertEquals(NumberTypeFP.UNKNOWN, p.getNumberTypeFP());
assertEquals(input.x, p.getIntValue());
assertToken(JsonToken.FIELD_NAME, p.nextToken());
assertEquals("y", p.currentName());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals(input.y, p.getIntValue());
assertToken(JsonToken.END_OBJECT, p.nextToken());
p.close();
try (JsonParser p = MAPPER.getFactory().createParser(bytes)) {
p.setSchema(schema);
assertNull(p.currentName());
assertToken(JsonToken.START_OBJECT, p.nextToken());
assertNull(p.currentName());
assertToken(JsonToken.FIELD_NAME, p.nextToken());
assertEquals("x", p.currentName());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals(NumberType.LONG, p.getNumberType());
assertEquals(NumberTypeFP.UNKNOWN, p.getNumberTypeFP());
assertEquals(input.x, p.getIntValue());
assertToken(JsonToken.FIELD_NAME, p.nextToken());
assertEquals("y", p.currentName());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals(input.y, p.getIntValue());
assertToken(JsonToken.END_OBJECT, p.nextToken());
assertNull(p.currentName());
}
}

public void testReadName() throws Exception
Expand All @@ -159,7 +171,7 @@ public void testReadName() throws Exception

public void testReadBox() throws Exception
{
ProtobufSchema schema = ProtobufSchemaLoader.std.parse(PROTOC_BOX);
final ProtobufSchema schema = ProtobufSchemaLoader.std.parse(PROTOC_BOX);
final ObjectWriter w = MAPPER.writerFor(Box.class)
.with(schema);
Point topLeft = new Point(100, 150);
Expand All @@ -177,6 +189,52 @@ public void testReadBox() throws Exception
assertNotNull(result.bottomRight);
assertEquals(input.topLeft, result.topLeft);
assertEquals(input.bottomRight, result.bottomRight);

// But let's try streaming too:
// actually let's also try via streaming parser
try (JsonParser p = MAPPER.getFactory().createParser(bytes)) {
p.setSchema(schema);
assertNull(p.currentName());
assertToken(JsonToken.START_OBJECT, p.nextToken());
assertNull(p.currentName());

assertToken(JsonToken.FIELD_NAME, p.nextToken());
assertEquals("topLeft", p.currentName());
assertToken(JsonToken.START_OBJECT, p.nextToken());
assertEquals("topLeft", p.currentName());
assertToken(JsonToken.FIELD_NAME, p.nextToken());
assertEquals("x", p.currentName());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals("x", p.currentName());
assertEquals(input.topLeft.x, p.getIntValue());
assertToken(JsonToken.FIELD_NAME, p.nextToken());
assertEquals("y", p.currentName());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals("y", p.currentName());
assertEquals(input.topLeft.y, p.getIntValue());
assertToken(JsonToken.END_OBJECT, p.nextToken());
assertEquals("topLeft", p.currentName());

assertToken(JsonToken.FIELD_NAME, p.nextToken());
assertEquals("bottomRight", p.currentName());
assertToken(JsonToken.START_OBJECT, p.nextToken());
assertEquals("bottomRight", p.currentName());
assertToken(JsonToken.FIELD_NAME, p.nextToken());
assertEquals("x", p.currentName());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals("x", p.currentName());
assertEquals(input.bottomRight.x, p.getIntValue());
assertToken(JsonToken.FIELD_NAME, p.nextToken());
assertEquals("y", p.currentName());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals("y", p.currentName());
assertEquals(input.bottomRight.y, p.getIntValue());
assertToken(JsonToken.END_OBJECT, p.nextToken());
assertEquals("bottomRight", p.currentName());

assertToken(JsonToken.END_OBJECT, p.nextToken());
assertNull(p.currentName());
}
}

public void testStringArraySimple() throws Exception
Expand Down
7 changes: 4 additions & 3 deletions .../test/java/com/fasterxml/jackson/dataformat/protobuf/fuzz/ProtobufFuzz_65674_NPETest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import org.junit.Test;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.dataformat.protobuf.*;

Expand All @@ -10,13 +11,13 @@

public class ProtobufFuzz_65674_NPETest
{
private final ProtobufMapper mapper = new ProtobufMapper();
private final ProtobufMapper MAPPER = new ProtobufMapper();

@Test
public void testFuzz65674NPE() throws Exception {
final byte[] doc = new byte[0];
try (ProtobufParser p = (ProtobufParser) mapper.getFactory().createParser(doc)) {
p.setSchema(mapper.generateSchemaFor(getClass()));
try (JsonParser p = MAPPER.createParser(doc)) {
p.setSchema(MAPPER.generateSchemaFor(getClass()));
assertEquals(JsonToken.START_OBJECT, p.nextToken());
assertNull(p.currentName());
assertEquals(JsonToken.END_OBJECT, p.nextToken());
Expand Down
1 change: 1 addition & 0 deletions release-notes/VERSION-2.x
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Active maintainers:
(fix contributed by Arthur C)
#460: (protobuf) Unexpected `NullPointerException` in `ProtobufParser.currentName()`
(fix contributed by Arthur C)
#462: (protobuf) `ProtobufParser.currentName()` returns wrong value at root level
- (ion) Update `com.amazon.ion:ion-java` to 1.11.0 (from 1.10.5)

2.16.1 (24-Dec-2023)
Expand Down

0 comments on commit 07de9d1

Please sign in to comment.