Skip to content

Commit 7cbd915

Browse files
committed
reflect/protodesc: fix panic when working with dynamicpb
Thanks to Joshua Humphries and Edward McFarlane for the excellent bug report, reproducer and fix suggestion! Fixes golang/protobuf#1669 Change-Id: I03df76f789e6e11b53396396a1f6b58bb3fb840b Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/642575 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Damien Neil <[email protected]> Reviewed-by: Chressie Himpel <[email protected]>
1 parent 2f60868 commit 7cbd915

File tree

2 files changed

+68
-10
lines changed

2 files changed

+68
-10
lines changed

reflect/protodesc/editions.go

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -125,16 +125,27 @@ func mergeEditionFeatures(parentDesc protoreflect.Descriptor, child *descriptorp
125125
parentFS.IsJSONCompliant = *jf == descriptorpb.FeatureSet_ALLOW
126126
}
127127

128-
if goFeatures, ok := proto.GetExtension(child, gofeaturespb.E_Go).(*gofeaturespb.GoFeatures); ok && goFeatures != nil {
129-
if luje := goFeatures.LegacyUnmarshalJsonEnum; luje != nil {
130-
parentFS.GenerateLegacyUnmarshalJSON = *luje
131-
}
132-
if sep := goFeatures.StripEnumPrefix; sep != nil {
133-
parentFS.StripEnumPrefix = int(*sep)
134-
}
135-
if al := goFeatures.ApiLevel; al != nil {
136-
parentFS.APILevel = int(*al)
137-
}
128+
// We must not use proto.GetExtension(child, gofeaturespb.E_Go)
129+
// because that only works for messages we generated, but not for
130+
// dynamicpb messages. See golang/protobuf#1669.
131+
goFeatures := child.ProtoReflect().Get(gofeaturespb.E_Go.TypeDescriptor())
132+
if !goFeatures.IsValid() {
133+
return parentFS
134+
}
135+
// gf.Interface() could be *dynamicpb.Message or *gofeaturespb.GoFeatures.
136+
gf := goFeatures.Message()
137+
fields := gf.Descriptor().Fields()
138+
139+
if fd := fields.ByName("legacy_unmarshal_json_enum"); gf.Has(fd) {
140+
parentFS.GenerateLegacyUnmarshalJSON = gf.Get(fd).Bool()
141+
}
142+
143+
if fd := fields.ByName("strip_enum_prefix"); gf.Has(fd) {
144+
parentFS.StripEnumPrefix = int(gf.Get(fd).Enum())
145+
}
146+
147+
if fd := fields.ByName("api_level"); gf.Has(fd) {
148+
parentFS.APILevel = int(gf.Get(fd).Enum())
138149
}
139150

140151
return parentFS

reflect/protodesc/editions_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package protodesc
6+
7+
import (
8+
"testing"
9+
10+
"google.golang.org/protobuf/proto"
11+
"google.golang.org/protobuf/reflect/protoreflect"
12+
"google.golang.org/protobuf/types/descriptorpb"
13+
"google.golang.org/protobuf/types/dynamicpb"
14+
"google.golang.org/protobuf/types/gofeaturespb"
15+
)
16+
17+
func TestGoFeaturesDynamic(t *testing.T) {
18+
md := (*gofeaturespb.GoFeatures)(nil).ProtoReflect().Descriptor()
19+
gf := dynamicpb.NewMessage(md)
20+
opaque := protoreflect.ValueOfEnum(gofeaturespb.GoFeatures_API_OPAQUE.Number())
21+
gf.Set(md.Fields().ByName("api_level"), opaque)
22+
featureSet := &descriptorpb.FeatureSet{}
23+
dynamicExt := dynamicpb.NewExtensionType(gofeaturespb.E_Go.TypeDescriptor().Descriptor())
24+
proto.SetExtension(featureSet, dynamicExt, gf)
25+
26+
fd := &descriptorpb.FileDescriptorProto{
27+
Name: proto.String("a.proto"),
28+
Dependency: []string{
29+
"google/protobuf/go_features.proto",
30+
},
31+
Edition: descriptorpb.Edition_EDITION_2023.Enum(),
32+
Syntax: proto.String("editions"),
33+
Options: &descriptorpb.FileOptions{
34+
Features: featureSet,
35+
},
36+
}
37+
fds := &descriptorpb.FileDescriptorSet{
38+
File: []*descriptorpb.FileDescriptorProto{
39+
ToFileDescriptorProto(descriptorpb.File_google_protobuf_descriptor_proto),
40+
ToFileDescriptorProto(gofeaturespb.File_google_protobuf_go_features_proto),
41+
fd,
42+
},
43+
}
44+
if _, err := NewFiles(fds); err != nil {
45+
t.Fatal(err)
46+
}
47+
}

0 commit comments

Comments
 (0)