Skip to content

Commit 6a26748

Browse files
cowtowncodercowtowncoder
authored
Complete fix of #134 (#709)
1 parent 26581fd commit 6a26748

3 files changed

Lines changed: 120 additions & 7 deletions

File tree

‎protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,10 @@ private JsonToken _handleRootKey(int tag) throws IOException
763763
}
764764
// array?
765765
if (f.repeated) {
766-
if (f.packed) {
766+
// 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from
767+
// the actual wire type, not just the schema's declared `packed` flag:
768+
// proto3 permits either encoding for repeated scalar/enum fields.
769+
if (f.isPackedInWire(wireType)) {
767770
_state = STATE_ARRAY_START_PACKED;
768771
} else {
769772
_state = STATE_ARRAY_START;
@@ -807,7 +810,10 @@ private JsonToken _handleNestedKey(int tag) throws IOException
807810

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

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

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

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

11991214
// array?
12001215
if (_currentField.repeated) {
1201-
if (_currentField.packed) {
1216+
// 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from
1217+
// the actual wire type, not just the schema's declared `packed` flag:
1218+
// proto3 permits either encoding for repeated scalar/enum fields.
1219+
if (_currentField.isPackedInWire(wireType)) {
12021220
_state = STATE_ARRAY_START_PACKED;
12031221
} else {
12041222
_state = STATE_ARRAY_START;

‎protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufField.java‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,34 @@ public final boolean isArray() {
221221
public final boolean isValidFor(int typeTag) {
222222
return (typeTag == wireType)
223223
// 13-Apr-2017, tatu: to fix [dataformats-binary#76]
224-
|| (packed && repeated && typeTag == WireType.LENGTH_PREFIXED);
224+
// 03-Jul-2026, tatu: [dataformats-binary#134] A repeated scalar/enum
225+
// field may arrive packed (LENGTH_PREFIXED) regardless of the schema's
226+
// declared `packed` flag -- proto3 permits either encoding on the wire,
227+
// so tolerance must key off the type, not the schema default.
228+
|| (repeated && type.isPackable() && typeTag == WireType.LENGTH_PREFIXED);
229+
}
230+
231+
/**
232+
* Accessor for deciding whether an incoming, repeated field should be read
233+
* using "packed" (single length-prefixed block) encoding.
234+
*<p>
235+
* For genuinely packable types (scalar numeric/enum/boolean) the native
236+
* unpacked wire type differs from {@code LENGTH_PREFIXED}, so the actual wire
237+
* type is unambiguous and authoritative: proto3 permits either encoding on the
238+
* wire regardless of the schema's declared {@code packed} flag.
239+
*<p>
240+
* For non-packable types (String/Bytes/Message) a single element and a
241+
* jackson-style "packed" block are <b>both</b> {@code LENGTH_PREFIXED}, so the
242+
* wire type cannot distinguish them; there we must fall back to the schema's
243+
* declared {@code packed} flag.
244+
*
245+
* @since 2.21.5 [dataformats-binary#134]
246+
*/
247+
public final boolean isPackedInWire(int typeTag) {
248+
if (type.isPackable()) {
249+
return repeated && (typeTag == WireType.LENGTH_PREFIXED);
250+
}
251+
return packed;
225252
}
226253

227254
@Override

‎protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/Proto3PackedDefault134Test.java‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,72 @@ public void testProto3WriteDefaultsToPacked() throws Exception
116116
assertEquals(100, t.get("f").get(0).asInt());
117117
assertEquals(200, t.get("f").get(1).asInt());
118118
}
119+
120+
// [dataformats-binary#134]: even though a proto3 schema declares the field
121+
// "packed" by default, the wire is authoritative -- an unpacked proto3 stream
122+
// (legal per spec) must still decode. Decoder keys off the actual wire type.
123+
@Test
124+
public void testProto3PackedSchemaStillReadsUnpackedWire() throws Exception
125+
{
126+
final String SCHEMA_STR = "syntax = \"proto3\";\n"
127+
+ "message t {\n"
128+
+ " repeated uint32 f = 1;\n"
129+
+ "}\n";
130+
// unpacked encoding despite proto3 default being packed
131+
final byte[] pb = { 0x8, 0x64, 0x8, (byte) 0xc8, 0x1 }; // f = [100, 200], unpacked
132+
133+
ProtobufSchema schema = ProtobufSchemaLoader.std.parse(SCHEMA_STR);
134+
JsonNode t = MAPPER.readerFor(JsonNode.class).with(schema).readValue(pb);
135+
136+
assertEquals(2, t.get("f").size());
137+
assertEquals(100, t.get("f").get(0).asInt());
138+
assertEquals(200, t.get("f").get(1).asInt());
139+
}
140+
141+
// [dataformats-binary#134]: conversely, an unpacked-by-default schema (proto2,
142+
// or proto3 with `[packed=false]`) must still decode a packed wire stream.
143+
@Test
144+
public void testUnpackedSchemaStillReadsPackedWire() throws Exception
145+
{
146+
final String SCHEMA_STR = "syntax = \"proto2\";\n"
147+
+ "message t {\n"
148+
+ " repeated uint32 f = 1;\n"
149+
+ "}\n";
150+
// packed encoding despite proto2 default being unpacked
151+
final byte[] pb = { 0xa, 0x3, 0x64, (byte) 0xc8, 0x1 }; // f = [100, 200], packed
152+
153+
ProtobufSchema schema = ProtobufSchemaLoader.std.parse(SCHEMA_STR);
154+
JsonNode t = MAPPER.readerFor(JsonNode.class).with(schema).readValue(pb);
155+
156+
assertEquals(2, t.get("f").size());
157+
assertEquals(100, t.get("f").get(0).asInt());
158+
assertEquals(200, t.get("f").get(1).asInt());
159+
}
160+
161+
// [dataformats-binary#134]: mismatch tolerance must also work for a repeated
162+
// field nested inside a message (exercises `_handleNestedKey`, not just root).
163+
// Uses proto2 + explicit `[packed=true]` to declare the nested field packed
164+
// while feeding an unpacked wire stream (the old square protoparser does not
165+
// accept proto3 singular message fields, so we avoid proto3 syntax here).
166+
@Test
167+
public void testNestedRepeatedPackedSchemaReadsUnpackedWire() throws Exception
168+
{
169+
final String SCHEMA_STR = "message Outer {\n"
170+
+ " optional Inner inner = 1;\n"
171+
+ "}\n"
172+
+ "message Inner {\n"
173+
+ " repeated uint32 f = 1 [packed=true];\n"
174+
+ "}\n";
175+
// Outer.inner (field 1, length-delimited) wrapping Inner with unpacked f=[100,200]
176+
// inner payload: 0x8,0x64, 0x8,0xc8,0x1 (5 bytes)
177+
final byte[] pb = { 0xa, 0x5, 0x8, 0x64, 0x8, (byte) 0xc8, 0x1 };
178+
179+
ProtobufSchema schema = ProtobufSchemaLoader.std.parse(SCHEMA_STR, "Outer");
180+
JsonNode t = MAPPER.readerFor(JsonNode.class).with(schema).readValue(pb);
181+
182+
JsonNode arr = t.get("inner").get("f");
183+
assertEquals(2, arr.size());
184+
assertEquals(100, arr.get(0).asInt());
185+
assertEquals(200, arr.get(1).asInt());
186+
}
119187
}

0 commit comments

Comments
 (0)