Fix #584 (missing END_OBJECT) (#585)

Author: cowtowncoder

protobuf/src/main/java/tools/jackson/dataformat/protobuf/ProtobufParser.java

@@ -679,12 +679,12 @@ private JsonToken _handleRootKey(int tag) throws JacksonException
         if (_currentField != null) {
             if ((f = _currentField.nextOrThisIf(id)) == null) {
                 if ((f = _currentMessage.field(id)) == null) {
-                    return _skipUnknownField(id, wireType);
+                    return _skipUnknownField(id, wireType, false);
                 }
             }
         } else {
             if ((f = _currentMessage.field(id)) == null) {
-                return _skipUnknownField(id, wireType);
+                return _skipUnknownField(id, wireType, false);
             }
         }
         _streamReadContext.setCurrentName(f.name);
@@ -715,12 +715,12 @@ private JsonToken _handleNestedKey(int tag) throws JacksonException
         if (_currentField != null) {
             if ((f = _currentField.nextOrThisIf(id)) == null) {
                 if ((f = _currentMessage.field(id)) == null) {
-                    return _skipUnknownField(id, wireType);
+                    return _skipUnknownField(id, wireType, true);
                 }
             }
         } else {
             if ((f = _currentMessage.field(id)) == null) {
-                return _skipUnknownField(id, wireType);
+                return _skipUnknownField(id, wireType, true);
             }
         }
 
@@ -729,7 +729,7 @@ private JsonToken _handleNestedKey(int tag) throws JacksonException
         }
         // Note: may be null; if so, value needs to be skipped
         if (f == null) {
-            return _skipUnknownField(id, wireType);
+            return _skipUnknownField(id, wireType, true);
         }
         _streamReadContext.setCurrentName(f.name);
         if (!f.isValidFor(wireType)) {
@@ -891,8 +891,10 @@ private JsonToken _readNextValue(FieldType t, int nextState) throws JacksonExcep
         return type;
     }
 
-    private JsonToken _skipUnknownField(int tag, int wireType) throws JacksonException
+    private JsonToken _skipUnknownField(int tag, int wireType, boolean nestedField) throws JacksonException
     {
+//System.out.println(" _skipUnknownField(tag="+tag+", wireType="+wireType+")");
+
         // First: is this even allowed?
         if (!isEnabled(StreamReadFeature.IGNORE_UNDEFINED)) {
             _reportErrorF("Undefined property (id %d, wire type %d) for message type %s: not allowed to ignore, as `JsonParser.Feature.IGNORE_UNDEFINED` disabled",
@@ -905,7 +907,7 @@ private JsonToken _skipUnknownField(int tag, int wireType) throws JacksonExcepti
             if (_checkEnd()) { // updates _parsingContext
                 return _updateToken(JsonToken.END_OBJECT);
             }
-            if (_state == STATE_NESTED_KEY) {
+            if (nestedField) {
                 if (_inputPtr >= _inputEnd) {
                     loadMoreGuaranteed();
                 }
@@ -924,7 +926,14 @@ private JsonToken _skipUnknownField(int tag, int wireType) throws JacksonExcepti
                 continue;
             }
             _streamReadContext.setCurrentName(_currentField.name);
-            _state = STATE_ROOT_VALUE;
+
+            // 30-Apr-2025, tatu: [dataformats-binary#584] may be called for root and nested
+            if (nestedField) {
+                _state = STATE_NESTED_VALUE;
+            } else {
+                _state = STATE_ROOT_VALUE;
+            }
+
             // otherwise quickly validate compatibility
             if (!_currentField.isValidFor(wireType)) {
                 _reportIncompatibleType(_currentField, wireType);
@@ -980,7 +989,7 @@ public String nextName() throws JacksonException
 
             ProtobufField f = _findField(id);
             if (f == null) {
-                if (_skipUnknownField(id, wireType) != JsonToken.PROPERTY_NAME) {
+                if (_skipUnknownField(id, wireType, false) != JsonToken.PROPERTY_NAME) {
                     return null;
                 }
                 // sub-optimal as skip method already set it, but:
@@ -1017,7 +1026,7 @@ public String nextName() throws JacksonException
 
             ProtobufField f = _findField(id);
             if (f == null) {
-                if (_skipUnknownField(id, wireType) != JsonToken.PROPERTY_NAME) {
+                if (_skipUnknownField(id, wireType, true) != JsonToken.PROPERTY_NAME) {
                     return null;
                 }
                 // sub-optimal as skip method already set it, but:
@@ -1068,7 +1077,7 @@ public boolean nextName(SerializableString sstr) throws JacksonException
 
             ProtobufField f = _findField(id);
             if (f == null) {
-                _skipUnknownField(id, wireType);
+                _skipUnknownField(id, wireType, false);
                 // may or may not match, but let caller figure it out
                 return false;
             }
@@ -1104,7 +1113,7 @@ public boolean nextName(SerializableString sstr) throws JacksonException
 
             ProtobufField f = _findField(id);
             if (f == null) {
-                _skipUnknownField(id, wireType);
+                _skipUnknownField(id, wireType, true);
                 // may or may not match, but let caller figure it out
                 return false;
             }
@@ -1154,7 +1163,7 @@ public int nextNameMatch(PropertyNameMatcher matcher) throws JacksonException
 
             ProtobufField f = _findField(id);
             if (f == null) {
-                JsonToken t = _skipUnknownField(id, wireType);
+                JsonToken t = _skipUnknownField(id, wireType, false);
                 if (t != JsonToken.PROPERTY_NAME) {
                     return (t == JsonToken.END_OBJECT)
                             ? PropertyNameMatcher.MATCH_END_OBJECT : PropertyNameMatcher.MATCH_ODD_TOKEN;
@@ -1192,7 +1201,7 @@ public int nextNameMatch(PropertyNameMatcher matcher) throws JacksonException
 
             ProtobufField f = _findField(id);
             if (f == null) {
-                JsonToken t = _skipUnknownField(id, wireType);
+                JsonToken t = _skipUnknownField(id, wireType, true);
                 if (t != JsonToken.PROPERTY_NAME) {
                     return (t == JsonToken.END_OBJECT)
                             ? PropertyNameMatcher.MATCH_END_OBJECT : PropertyNameMatcher.MATCH_ODD_TOKEN;

protobuf/src/test/java/tools/jackson/dataformat/protobuf/ReadNestedUnknownFieldsTest.java

@@ -9,7 +9,6 @@
 import com.fasterxml.jackson.annotation.JsonPropertyOrder;
 
 import tools.jackson.core.*;
-import tools.jackson.databind.MapperFeature;
 import tools.jackson.databind.ObjectReader;
 import tools.jackson.dataformat.protobuf.schema.ProtobufSchema;
 
@@ -124,21 +123,14 @@ public static class Embed {
     }
 
     /*
-    /**********************************************************
+    /**********************************************************************
     /* Test methods
-    /**********************************************************
+    /**********************************************************************
      */
 
-    // 30-Apr-2025, tatu: Looks like we have a bug of some kind, exposed
-    //   by change to `MapperFeature.DEFAULT_VIEW_INCLUSION` defaults
-    //   (changed to `false`) but probably not caused by the change
+    private final ProtobufMapper MAPPER = newObjectMapper();
 
-//    private final ProtobufMapper MAPPER = newObjectMapper();
-    private final ProtobufMapper MAPPER = newMapperBuilder()
-            .enable(MapperFeature.DEFAULT_VIEW_INCLUSION)
-            .build();
-
-    // [dataformats-binary#108]
+    // [dataformats-binary#108], [dataformats-binary#584]
     @Test
     public void testMultipleUnknown() throws Exception
     {
@@ -159,9 +151,7 @@ public void testMultipleUnknown() throws Exception
 
         //System.err.println("-> "+MAPPER.valueToTree(protoR.readValue(doc)).toPrettyString());
 
-        // 30-Apr-2025, tatu: First, iterate over tokens
-        //   ... alas, does not actually 
-        /*
+        // 30-Apr-2025, tatu: [dataformats-binary#584]: First, iterate over tokens
         try (JsonParser p = protoR.createParser(doc)) {
             assertToken(JsonToken.START_OBJECT, p.nextToken());
             assertToken(JsonToken.PROPERTY_NAME, p.nextToken());
@@ -176,9 +166,8 @@ public void testMultipleUnknown() throws Exception
             assertToken(JsonToken.END_OBJECT, p.nextToken());
             assertNull(p.nextToken());
         }
-        */
 
-        // and only then use databinding
+        // and only then test databinding
 
         LessNestedField lesser = protoR.readValue(doc);
 
@@ -201,7 +190,7 @@ public void testCheckEndAfterSkip() throws Exception
 
         OuterV2 v2Expected = new OuterV2();
         v2Expected.embed = embedV2;
-        v2Expected.state="state";
+        v2Expected.state = "state";
 
         // serialize type with extra field
         byte[] doc = mapper.writer(schemaV2).writeValueAsBytes(v2Expected);