Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,14 @@ public class ProtobufParser extends ParserMinimalBase
// State after either reaching end-of-input, or getting explicitly closed
private final static int STATE_CLOSED = 12;

// State after returning END_OBJECT for root level, before closing
// (added for issue #598 to separate END_OBJECT return from close())
//
// @since 3.1
private final static int STATE_ROOT_END = 13;

private final static int[] UTF8_UNIT_CODES = ProtobufUtil.sUtf8UnitLengths;

// @since 2.14
protected final static JacksonFeatureSet<StreamReadCapability> PROTOBUF_READ_CAPABILITIES
= DEFAULT_READ_CAPABILITIES.with(StreamReadCapability.EXACT_FLOATS);

Expand Down Expand Up @@ -539,7 +544,8 @@ public JsonToken nextToken() throws JacksonException
// end-of-input?
if (_inputPtr >= _inputEnd) {
if (!loadMore()) {
close();
_state = STATE_ROOT_END;
_streamReadContext.setCurrentName(null);
return _updateToken(JsonToken.END_OBJECT);
}
}
Expand Down Expand Up @@ -634,9 +640,14 @@ public JsonToken nextToken() throws JacksonException
return _updateToken(_readNextValue(_currentField.type, STATE_NESTED_KEY));

case STATE_MESSAGE_END: // occurs if we end with array
close(); // sets state to STATE_CLOSED
_state = STATE_ROOT_END;
_streamReadContext.setCurrentName(null);
return _updateToken(JsonToken.END_OBJECT);

case STATE_ROOT_END: // returned END_OBJECT, now close on next call
close(); // sets state to STATE_CLOSED
return null;

case STATE_CLOSED:
return null;

Expand Down Expand Up @@ -913,7 +924,8 @@ private JsonToken _skipUnknownField(int tag, int wireType, boolean nestedField)
}
} else if (_inputPtr >= _inputEnd) {
if (!loadMore()) {
close();
_state = STATE_ROOT_END;
_streamReadContext.setCurrentName(null);
return _updateToken(JsonToken.END_OBJECT);
}
}
Expand Down Expand Up @@ -976,7 +988,8 @@ public String nextName() throws JacksonException
if (_state == STATE_ROOT_KEY) {
if (_inputPtr >= _inputEnd) {
if (!loadMore()) {
close();
_state = STATE_ROOT_END;
_streamReadContext.setCurrentName(null);
_updateToken(JsonToken.END_OBJECT);
return null;
}
Expand Down Expand Up @@ -1051,7 +1064,8 @@ public String nextName() throws JacksonException
return name;
}
if (_state == STATE_MESSAGE_END) {
close(); // sets state to STATE_CLOSED
_state = STATE_ROOT_END;
_streamReadContext.setCurrentName(null);
_updateToken(JsonToken.END_OBJECT);
return null;
}
Expand All @@ -1064,7 +1078,8 @@ public boolean nextName(SerializableString sstr) throws JacksonException
if (_state == STATE_ROOT_KEY) {
if (_inputPtr >= _inputEnd) {
if (!loadMore()) {
close();
_state = STATE_ROOT_END;
_streamReadContext.setCurrentName(null);
_updateToken(JsonToken.END_OBJECT);
return false;
}
Expand Down Expand Up @@ -1137,7 +1152,8 @@ public boolean nextName(SerializableString sstr) throws JacksonException
return name.equals(sstr.getValue());
}
if (_state == STATE_MESSAGE_END) {
close(); // sets state to STATE_CLOSED
_state = STATE_ROOT_END;
_streamReadContext.setCurrentName(null);
_updateToken(JsonToken.END_OBJECT);
return false;
}
Expand All @@ -1150,7 +1166,8 @@ public int nextNameMatch(PropertyNameMatcher matcher) throws JacksonException
if (_state == STATE_ROOT_KEY) {
if (_inputPtr >= _inputEnd) {
if (!loadMore()) {
close();
_state = STATE_ROOT_END;
_streamReadContext.setCurrentName(null);
_updateToken(JsonToken.END_OBJECT);
return PropertyNameMatcher.MATCH_END_OBJECT;
}
Expand Down Expand Up @@ -1227,7 +1244,8 @@ public int nextNameMatch(PropertyNameMatcher matcher) throws JacksonException
return matcher.matchName(name);
}
if (_state == STATE_MESSAGE_END) {
close(); // sets state to STATE_CLOSED
_state = STATE_ROOT_END;
_streamReadContext.setCurrentName(null);
_updateToken(JsonToken.END_OBJECT);
return PropertyNameMatcher.MATCH_END_OBJECT;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
package tools.jackson.dataformat.protobuf;

import org.junit.jupiter.api.Test;

import tools.jackson.core.*;

import tools.jackson.dataformat.protobuf.schema.ProtobufSchema;
import tools.jackson.dataformat.protobuf.schema.ProtobufSchemaLoader;

import static org.junit.jupiter.api.Assertions.*;

/**
* Test for issue #598: Protobuf parser state handling wrong for implicit close (END_OBJECT)
*/
public class ParserStateEndTest extends ProtobufTestBase
{
private final ProtobufMapper MAPPER = newObjectMapper();

/**
* Test that verifies the parser properly handles the end-of-input state.
* The parser should NOT be closed when returning the final END_OBJECT token;
* it should only be closed on the subsequent nextToken() call.
*/
@Test
public void testParserStateAtEndObject() throws Exception
{
// Use a simple Point schema
ProtobufSchema schema = ProtobufSchemaLoader.std.parse(PROTOC_POINT);

// Create test data
Point input = new Point(42, 13);
byte[] bytes = MAPPER.writerFor(Point.class)
.with(schema)
.writeValueAsBytes(input);

// Parse with streaming parser
try (JsonParser p = MAPPER.reader()
.with(schema)
.createParser(bytes)) {
assertToken(JsonToken.START_OBJECT, p.nextToken());

// First field: "x"
assertToken(JsonToken.PROPERTY_NAME, p.nextToken());
assertEquals("x", p.currentName());

assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals(42, p.getIntValue());

// Second field: "y"
assertToken(JsonToken.PROPERTY_NAME, p.nextToken());
assertEquals("y", p.currentName());

assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals(13, p.getIntValue());

// END_OBJECT - This is the critical test
assertToken(JsonToken.END_OBJECT, p.nextToken());

// THIS IS THE KEY ASSERTION: Parser should NOT be closed yet
// The parser should only be closed on the NEXT nextToken() call
assertFalse(p.isClosed(),
"Parser should NOT be closed immediately after returning END_OBJECT");

// Verify currentToken() returns END_OBJECT (not null)
assertEquals(JsonToken.END_OBJECT, p.currentToken(),
"currentToken() should return END_OBJECT, not null");

// Now the next token should be null AND close the parser
assertNull(p.nextToken(), "After END_OBJECT, nextToken() should return null");
assertTrue(p.isClosed(), "Parser should be closed after nextToken() returns null");
}
}

/**
* Similar test but using nextName() optimization
*/
@Test
public void testParserStateAtEndObjectWithNextName() throws Exception
{
ProtobufSchema schema = ProtobufSchemaLoader.std.parse(PROTOC_POINT);

Point input = new Point(42, 13);
byte[] bytes = MAPPER.writerFor(Point.class)
.with(schema)
.writeValueAsBytes(input);

try (JsonParser p = MAPPER.reader()
.with(schema)
.createParser(bytes)) {
assertToken(JsonToken.START_OBJECT, p.nextToken());

// Use nextName() for field access
assertEquals("x", p.nextName());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());

assertEquals("y", p.nextName());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());

// Should get null from nextName() at end
assertNull(p.nextName());

// Current token should be END_OBJECT
assertEquals(JsonToken.END_OBJECT, p.currentToken(),
"currentToken() should return END_OBJECT after nextName() returns null");

// Parser should NOT be closed yet
assertFalse(p.isClosed(),
"Parser should NOT be closed when currentToken is END_OBJECT");

// Next token should be null and close parser
assertNull(p.nextToken());
assertTrue(p.isClosed());
}
}

/**
* Test with empty message (no fields)
*/
@Test
public void testParserStateWithEmptyMessage() throws Exception
{
final String PROTOC_EMPTY = "message Empty {}\n";
ProtobufSchema schema = ProtobufSchemaLoader.std.parse(PROTOC_EMPTY);

// Empty message = just START_OBJECT, END_OBJECT
byte[] bytes = MAPPER.writer()
.with(schema)
.writeValueAsBytes(new Object());

try (JsonParser p = MAPPER.reader()
.with(schema)
.createParser(bytes)) {
// START_OBJECT
assertToken(JsonToken.START_OBJECT, p.nextToken());
assertFalse(p.isClosed());

// END_OBJECT immediately
assertToken(JsonToken.END_OBJECT, p.nextToken());

// Parser should NOT be closed yet
assertFalse(p.isClosed(),
"Parser should NOT be closed immediately after END_OBJECT");
assertEquals(JsonToken.END_OBJECT, p.currentToken());

// Next token closes
assertNull(p.nextToken());
assertTrue(p.isClosed());
}
}
}
2 changes: 2 additions & 0 deletions release-notes/VERSION
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ implementations)

3.1.0 (not yet released)

#598: (protobuf) Protobuf parser state handling wrong for implicit close (END_OBJECT)
(fix by @cowtowncoder, w/ Claude code)
#619: (avro, cbor, ion, smile) Add `isEnabled()` methods for format-specific features
(like `CBORReadFeature` and `CBORWriteFeature`) to mappers
(requested by Andy W)
Expand Down