Skip to content

Complete fix of #134 #709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cowtowncoder merged 1 commit into from
Jul 4, 2026
+120 −7
Merged

Complete fix of #134 #709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 24 additions & 6 deletions protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,10 @@ private JsonToken _handleRootKey(int tag) throws IOException
}
// array?
if (f.repeated) {
if (f.packed) {
// 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from
// the actual wire type, not just the schema's declared `packed` flag:
// proto3 permits either encoding for repeated scalar/enum fields.
if (f.isPackedInWire(wireType)) {
_state = STATE_ARRAY_START_PACKED;
} else {
_state = STATE_ARRAY_START;
Expand Down Expand Up @@ -807,7 +810,10 @@ private JsonToken _handleNestedKey(int tag) throws IOException

// array?
if (f.repeated) {
if (f.packed) {
// 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from
// the actual wire type, not just the schema's declared `packed` flag:
// proto3 permits either encoding for repeated scalar/enum fields.
if (f.isPackedInWire(wireType)) {
_state = STATE_ARRAY_START_PACKED;
} else {
_state = STATE_ARRAY_START;
Expand Down Expand Up @@ -1074,7 +1080,10 @@ public boolean nextFieldName(SerializableString sstr) throws IOException

// array?
if (_currentField.repeated) {
if (_currentField.packed) {
// 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from
// the actual wire type, not just the schema's declared `packed` flag:
// proto3 permits either encoding for repeated scalar/enum fields.
if (_currentField.isPackedInWire(wireType)) {
_state = STATE_ARRAY_START_PACKED;
} else {
_state = STATE_ARRAY_START;
Expand Down Expand Up @@ -1110,7 +1119,10 @@ public boolean nextFieldName(SerializableString sstr) throws IOException

// array?
if (_currentField.repeated) {
if (_currentField.packed) {
// 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from
// the actual wire type, not just the schema's declared `packed` flag:
// proto3 permits either encoding for repeated scalar/enum fields.
if (_currentField.isPackedInWire(wireType)) {
_state = STATE_ARRAY_START_PACKED;
} else {
_state = STATE_ARRAY_START;
Expand Down Expand Up @@ -1159,7 +1171,10 @@ public String nextFieldName() throws IOException

// array?
if (_currentField.repeated) {
if (_currentField.packed) {
// 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from
// the actual wire type, not just the schema's declared `packed` flag:
// proto3 permits either encoding for repeated scalar/enum fields.
if (_currentField.isPackedInWire(wireType)) {
_state = STATE_ARRAY_START_PACKED;
} else {
_state = STATE_ARRAY_START;
Expand Down Expand Up @@ -1198,7 +1213,10 @@ public String nextFieldName() throws IOException

// array?
if (_currentField.repeated) {
if (_currentField.packed) {
// 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from
// the actual wire type, not just the schema's declared `packed` flag:
// proto3 permits either encoding for repeated scalar/enum fields.
if (_currentField.isPackedInWire(wireType)) {
_state = STATE_ARRAY_START_PACKED;
} else {
_state = STATE_ARRAY_START;
Expand Down
29 changes: 28 additions & 1 deletion protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,34 @@ public final boolean isArray() {
public final boolean isValidFor(int typeTag) {
return (typeTag == wireType)
// 13-Apr-2017, tatu: to fix [dataformats-binary#76]
|| (packed && repeated && typeTag == WireType.LENGTH_PREFIXED);
// 03-Jul-2026, tatu: [dataformats-binary#134] A repeated scalar/enum
// field may arrive packed (LENGTH_PREFIXED) regardless of the schema's
// declared `packed` flag -- proto3 permits either encoding on the wire,
// so tolerance must key off the type, not the schema default.
|| (repeated && type.isPackable() && typeTag == WireType.LENGTH_PREFIXED);
}

/**
* Accessor for deciding whether an incoming, repeated field should be read
* using "packed" (single length-prefixed block) encoding.
*<p>
* For genuinely packable types (scalar numeric/enum/boolean) the native
* unpacked wire type differs from {@code LENGTH_PREFIXED}, so the actual wire
* type is unambiguous and authoritative: proto3 permits either encoding on the
* wire regardless of the schema's declared {@code packed} flag.
*<p>
* For non-packable types (String/Bytes/Message) a single element and a
* jackson-style "packed" block are <b>both</b> {@code LENGTH_PREFIXED}, so the
* wire type cannot distinguish them; there we must fall back to the schema's
* declared {@code packed} flag.
*
* @since 2.21.5 [dataformats-binary#134]
*/
public final boolean isPackedInWire(int typeTag) {
if (type.isPackable()) {
return repeated && (typeTag == WireType.LENGTH_PREFIXED);
}
return packed;
}

@Override
Expand Down
68 changes: 68 additions & 0 deletions ...f/src/test/java/com/fasterxml/jackson/dataformat/protobuf/Proto3PackedDefault134Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,72 @@ public void testProto3WriteDefaultsToPacked() throws Exception
assertEquals(100, t.get("f").get(0).asInt());
assertEquals(200, t.get("f").get(1).asInt());
}

// [dataformats-binary#134]: even though a proto3 schema declares the field
// "packed" by default, the wire is authoritative -- an unpacked proto3 stream
// (legal per spec) must still decode. Decoder keys off the actual wire type.
@Test
public void testProto3PackedSchemaStillReadsUnpackedWire() throws Exception
{
final String SCHEMA_STR = "syntax = \"proto3\";\n"
+ "message t {\n"
+ " repeated uint32 f = 1;\n"
+ "}\n";
// unpacked encoding despite proto3 default being packed
final byte[] pb = { 0x8, 0x64, 0x8, (byte) 0xc8, 0x1 }; // f = [100, 200], unpacked

ProtobufSchema schema = ProtobufSchemaLoader.std.parse(SCHEMA_STR);
JsonNode t = MAPPER.readerFor(JsonNode.class).with(schema).readValue(pb);

assertEquals(2, t.get("f").size());
assertEquals(100, t.get("f").get(0).asInt());
assertEquals(200, t.get("f").get(1).asInt());
}

// [dataformats-binary#134]: conversely, an unpacked-by-default schema (proto2,
// or proto3 with `[packed=false]`) must still decode a packed wire stream.
@Test
public void testUnpackedSchemaStillReadsPackedWire() throws Exception
{
final String SCHEMA_STR = "syntax = \"proto2\";\n"
+ "message t {\n"
+ " repeated uint32 f = 1;\n"
+ "}\n";
// packed encoding despite proto2 default being unpacked
final byte[] pb = { 0xa, 0x3, 0x64, (byte) 0xc8, 0x1 }; // f = [100, 200], packed

ProtobufSchema schema = ProtobufSchemaLoader.std.parse(SCHEMA_STR);
JsonNode t = MAPPER.readerFor(JsonNode.class).with(schema).readValue(pb);

assertEquals(2, t.get("f").size());
assertEquals(100, t.get("f").get(0).asInt());
assertEquals(200, t.get("f").get(1).asInt());
}

// [dataformats-binary#134]: mismatch tolerance must also work for a repeated
// field nested inside a message (exercises `_handleNestedKey`, not just root).
// Uses proto2 + explicit `[packed=true]` to declare the nested field packed
// while feeding an unpacked wire stream (the old square protoparser does not
// accept proto3 singular message fields, so we avoid proto3 syntax here).
@Test
public void testNestedRepeatedPackedSchemaReadsUnpackedWire() throws Exception
{
final String SCHEMA_STR = "message Outer {\n"
+ " optional Inner inner = 1;\n"
+ "}\n"
+ "message Inner {\n"
+ " repeated uint32 f = 1 [packed=true];\n"
+ "}\n";
// Outer.inner (field 1, length-delimited) wrapping Inner with unpacked f=[100,200]
// inner payload: 0x8,0x64, 0x8,0xc8,0x1 (5 bytes)
final byte[] pb = { 0xa, 0x5, 0x8, 0x64, 0x8, (byte) 0xc8, 0x1 };

ProtobufSchema schema = ProtobufSchemaLoader.std.parse(SCHEMA_STR, "Outer");
JsonNode t = MAPPER.readerFor(JsonNode.class).with(schema).readValue(pb);

JsonNode arr = t.get("inner").get("f");
assertEquals(2, arr.size());
assertEquals(100, arr.get(0).asInt());
assertEquals(200, arr.get(1).asInt());
}
}