internal/impl: fix unmarshal of group containing their own field number

The fast-path unmarshal was getting confused when parsing a group
containing a field with a number the same as the group's own field
number. Separate the handling of EndGroup tags.

Change-Id: I637702b42c94a26102e693ee29a55e80b37d7f28
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214737
Reviewed-by: Joe Tsai <joetsai@google.com>
This commit is contained in:
Damien Neil 2020-01-14 11:12:21 -08:00
parent 1c8015fff5
commit 2ae60936c2
4 changed files with 834 additions and 803 deletions

View File

@ -90,6 +90,13 @@ func (mi *MessageInfo) unmarshalPointer(b []byte, p pointer, groupTag wire.Numbe
} }
b = b[n:] b = b[n:]
if wtyp == wire.EndGroupType {
if num != groupTag {
return 0, errors.New("mismatching end group marker")
}
return start - len(b), nil
}
var f *coderFieldInfo var f *coderFieldInfo
if int(num) < len(mi.denseCoderFields) { if int(num) < len(mi.denseCoderFields) {
f = mi.denseCoderFields[num] f = mi.denseCoderFields[num]
@ -103,9 +110,6 @@ func (mi *MessageInfo) unmarshalPointer(b []byte, p pointer, groupTag wire.Numbe
break break
} }
n, err = f.funcs.unmarshal(b, p.Apply(f.offset), wtyp, opts) n, err = f.funcs.unmarshal(b, p.Apply(f.offset), wtyp, opts)
case num == groupTag && wtyp == wire.EndGroupType:
// End of group.
return start - len(b), nil
default: default:
// Possible extension. // Possible extension.
if exts == nil && mi.extensionOffset.IsValid() { if exts == nil && mi.extensionOffset.IsValid() {

File diff suppressed because it is too large Load Diff

View File

@ -44,6 +44,7 @@ message TestAllTypes {
optional group OptionalGroup = 16 { optional group OptionalGroup = 16 {
optional int32 a = 17; optional int32 a = 17;
optional NestedMessage optional_nested_message = 1000; optional NestedMessage optional_nested_message = 1000;
optional int32 same_field_number = 16;
} }
optional NestedMessage optional_nested_message = 18; optional NestedMessage optional_nested_message = 18;
optional ForeignMessage optional_foreign_message = 19; optional ForeignMessage optional_foreign_message = 19;
@ -190,6 +191,7 @@ extend TestAllExtensions {
optional group OptionalGroup_extension = 16 { optional group OptionalGroup_extension = 16 {
optional int32 a = 17; optional int32 a = 17;
optional int32 same_field_number = 16;
optional TestAllTypes.NestedMessage optional_nested_message = 1000; optional TestAllTypes.NestedMessage optional_nested_message = 1000;
} }

View File

@ -158,17 +158,20 @@ var testValidMessages = []testProto{
desc: "groups", desc: "groups",
decodeTo: []proto.Message{&testpb.TestAllTypes{ decodeTo: []proto.Message{&testpb.TestAllTypes{
Optionalgroup: &testpb.TestAllTypes_OptionalGroup{ Optionalgroup: &testpb.TestAllTypes_OptionalGroup{
A: proto.Int32(1017), A: proto.Int32(1017),
SameFieldNumber: proto.Int32(1016),
}, },
}, build( }, build(
&testpb.TestAllExtensions{}, &testpb.TestAllExtensions{},
extend(testpb.E_OptionalgroupExtension, &testpb.OptionalGroupExtension{ extend(testpb.E_OptionalgroupExtension, &testpb.OptionalGroupExtension{
A: proto.Int32(1017), A: proto.Int32(1017),
SameFieldNumber: proto.Int32(1016),
}), }),
)}, )},
wire: pack.Message{ wire: pack.Message{
pack.Tag{16, pack.StartGroupType}, pack.Tag{16, pack.StartGroupType},
pack.Tag{17, pack.VarintType}, pack.Varint(1017), pack.Tag{17, pack.VarintType}, pack.Varint(1017),
pack.Tag{16, pack.VarintType}, pack.Varint(1016),
pack.Tag{16, pack.EndGroupType}, pack.Tag{16, pack.EndGroupType},
}.Marshal(), }.Marshal(),
}, },