From e2bccb0e1092d93ee545f17c50c8a4c068f49b38 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Mon, 9 Sep 2024 15:47:49 -0400 Subject: [PATCH 1/4] Fix for protoc plugin extension reparse issue (Fixes #3306) --- go.mod | 3 +++ go.sum | 4 ++-- private/buf/cmd/protoc-gen-buf-breaking/breaking.go | 9 ++++++++- private/buf/cmd/protoc-gen-buf-lint/lint.go | 9 ++++++++- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index fc9cdb0d7d..1c1f37c778 100644 --- a/go.mod +++ b/go.mod @@ -120,3 +120,6 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20240903143218-8af14fe29dc1 // indirect google.golang.org/grpc v1.66.0 // indirect ) + +// temporary until something akin to https://github.com/bufbuild/protoplugin/pull/17 is merged +replace github.com/bufbuild/protoplugin => github.com/jchadwick-buf/protoplugin v0.0.0-20240909191235-edfe697940bd diff --git a/go.sum b/go.sum index 6fe0d21a81..a9868cbdab 100644 --- a/go.sum +++ b/go.sum @@ -28,8 +28,6 @@ github.com/bufbuild/bufplugin-go v0.1.0 h1:3LmgSHaSf8mPvwoFunimgm8uKJFLg+YePdi7N github.com/bufbuild/bufplugin-go v0.1.0/go.mod h1:gIbsJlcYJRLylxxNN3FPNd91fYxJmGVQgbZ67xLVrXk= github.com/bufbuild/protocompile v0.14.1 h1:iA73zAf/fyljNjQKwYzUHD6AD4R8KMasmwa/FBatYVw= github.com/bufbuild/protocompile v0.14.1/go.mod h1:ppVdAIhbr2H8asPk6k4pY7t9zB1OU5DoEw9xY/FUi1c= -github.com/bufbuild/protoplugin v0.0.0-20240323223605-e2735f6c31ee h1:E6ET8YUcYJ1lAe6ctR3as7yqzW2BNItDFnaB5zQq/8M= -github.com/bufbuild/protoplugin v0.0.0-20240323223605-e2735f6c31ee/go.mod h1:HjGFxsck9RObrTJp2hXQZfWhPgZqnR6sR1U5fCA/Kus= github.com/bufbuild/protovalidate-go v0.6.5 h1:WucDKXIbK22WjkO8A8J6Yyxxy0jl91Oe9LSMduq3YEE= github.com/bufbuild/protovalidate-go v0.6.5/go.mod h1:LHDiGCWSM3GagZEnyEZ1sPtFwi6Ja4tVTi/DCc+iDFI= github.com/bufbuild/protoyaml-go v0.1.12 h1:tIJrwvGxumVpNwLsw/AevT1QnkPDBuAObBSuBAdmAWY= @@ -157,6 +155,8 @@ github.com/ianlancetaylor/demangle v0.0.0-20210905161508-09a460cdf81d/go.mod h1: github.com/ianlancetaylor/demangle v0.0.0-20230524184225-eabc099b10ab/go.mod h1:gx7rwoVhcfuVKG5uya9Hs3Sxj7EIvldVofAWIUtGouw= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/jchadwick-buf/protoplugin v0.0.0-20240909191235-edfe697940bd h1:SzJXnUiE0hmuD+OeMIOYeePT1cPUTcoi2y9Kl4qME+k= +github.com/jchadwick-buf/protoplugin v0.0.0-20240909191235-edfe697940bd/go.mod h1:orVwXsUHOxZCBYP6hjO2KhCxyx3KB5hTHNs7nEWRrUY= github.com/jdx/go-netrc v1.0.0 h1:QbLMLyCZGj0NA8glAhxUpf1zDg6cxnWgMBbjq40W0gQ= github.com/jdx/go-netrc v1.0.0/go.mod h1:Gh9eFQJnoTNIRHXl2j5bJXA1u84hQWJWgGh569zF3v8= github.com/jhump/protoreflect v1.16.0 h1:54fZg+49widqXYQ0b+usAFHbMkBGR4PpXrsHc8+TBDg= diff --git a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go index dcd19521c9..e102855cb6 100644 --- a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go +++ b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go @@ -34,6 +34,8 @@ import ( "github.com/bufbuild/buf/private/pkg/protodescriptor" "github.com/bufbuild/buf/private/pkg/tracing" "github.com/bufbuild/protoplugin" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/reflect/protoregistry" ) const ( @@ -43,7 +45,12 @@ const ( // Main is the main. func Main() { - protoplugin.Main(protoplugin.HandlerFunc(handle)) + protoplugin.Main( + protoplugin.HandlerFunc(handle), + protoplugin.WithUnmarshalOptions(proto.UnmarshalOptions{ + Resolver: (*protoregistry.Types)(nil), + }), + ) } func handle( diff --git a/private/buf/cmd/protoc-gen-buf-lint/lint.go b/private/buf/cmd/protoc-gen-buf-lint/lint.go index 41bb9d099b..4b302acec4 100644 --- a/private/buf/cmd/protoc-gen-buf-lint/lint.go +++ b/private/buf/cmd/protoc-gen-buf-lint/lint.go @@ -33,6 +33,8 @@ import ( "github.com/bufbuild/buf/private/pkg/protodescriptor" "github.com/bufbuild/buf/private/pkg/tracing" "github.com/bufbuild/protoplugin" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/reflect/protoregistry" ) const ( @@ -42,7 +44,12 @@ const ( // Main is the main. func Main() { - protoplugin.Main(protoplugin.HandlerFunc(handle)) + protoplugin.Main( + protoplugin.HandlerFunc(handle), + protoplugin.WithUnmarshalOptions(proto.UnmarshalOptions{ + Resolver: (*protoregistry.Types)(nil), + }), + ) } func handle( From 3c6c6f19bc53f1ff5bf096bb00dceec3a59af98c Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 10 Sep 2024 14:07:10 -0400 Subject: [PATCH 2/4] Update protoplugin + cleaner empty resolver --- go.mod | 5 +-- go.sum | 4 +-- .../cmd/protoc-gen-buf-breaking/breaking.go | 7 ++-- private/buf/cmd/protoc-gen-buf-lint/lint.go | 7 ++-- private/pkg/protoencoding/resolver.go | 32 +++++++++++++++++++ 5 files changed, 39 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index 1c1f37c778..ca0849a891 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( connectrpc.com/otelconnect v0.7.1 github.com/bufbuild/bufplugin-go v0.1.0 github.com/bufbuild/protocompile v0.14.1 - github.com/bufbuild/protoplugin v0.0.0-20240323223605-e2735f6c31ee + github.com/bufbuild/protoplugin v0.0.0-20240910161437-27b7a335254b github.com/bufbuild/protovalidate-go v0.6.5 github.com/bufbuild/protoyaml-go v0.1.12 github.com/docker/docker v27.2.0+incompatible @@ -120,6 +120,3 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20240903143218-8af14fe29dc1 // indirect google.golang.org/grpc v1.66.0 // indirect ) - -// temporary until something akin to https://github.com/bufbuild/protoplugin/pull/17 is merged -replace github.com/bufbuild/protoplugin => github.com/jchadwick-buf/protoplugin v0.0.0-20240909191235-edfe697940bd diff --git a/go.sum b/go.sum index a9868cbdab..9df93c6173 100644 --- a/go.sum +++ b/go.sum @@ -28,6 +28,8 @@ github.com/bufbuild/bufplugin-go v0.1.0 h1:3LmgSHaSf8mPvwoFunimgm8uKJFLg+YePdi7N github.com/bufbuild/bufplugin-go v0.1.0/go.mod h1:gIbsJlcYJRLylxxNN3FPNd91fYxJmGVQgbZ67xLVrXk= github.com/bufbuild/protocompile v0.14.1 h1:iA73zAf/fyljNjQKwYzUHD6AD4R8KMasmwa/FBatYVw= github.com/bufbuild/protocompile v0.14.1/go.mod h1:ppVdAIhbr2H8asPk6k4pY7t9zB1OU5DoEw9xY/FUi1c= +github.com/bufbuild/protoplugin v0.0.0-20240910161437-27b7a335254b h1:WWwyYP+sr2V9XpCveKnOSsOgvXLIlBCCKCmGgmxZ+/0= +github.com/bufbuild/protoplugin v0.0.0-20240910161437-27b7a335254b/go.mod h1:orVwXsUHOxZCBYP6hjO2KhCxyx3KB5hTHNs7nEWRrUY= github.com/bufbuild/protovalidate-go v0.6.5 h1:WucDKXIbK22WjkO8A8J6Yyxxy0jl91Oe9LSMduq3YEE= github.com/bufbuild/protovalidate-go v0.6.5/go.mod h1:LHDiGCWSM3GagZEnyEZ1sPtFwi6Ja4tVTi/DCc+iDFI= github.com/bufbuild/protoyaml-go v0.1.12 h1:tIJrwvGxumVpNwLsw/AevT1QnkPDBuAObBSuBAdmAWY= @@ -155,8 +157,6 @@ github.com/ianlancetaylor/demangle v0.0.0-20210905161508-09a460cdf81d/go.mod h1: github.com/ianlancetaylor/demangle v0.0.0-20230524184225-eabc099b10ab/go.mod h1:gx7rwoVhcfuVKG5uya9Hs3Sxj7EIvldVofAWIUtGouw= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/jchadwick-buf/protoplugin v0.0.0-20240909191235-edfe697940bd h1:SzJXnUiE0hmuD+OeMIOYeePT1cPUTcoi2y9Kl4qME+k= -github.com/jchadwick-buf/protoplugin v0.0.0-20240909191235-edfe697940bd/go.mod h1:orVwXsUHOxZCBYP6hjO2KhCxyx3KB5hTHNs7nEWRrUY= github.com/jdx/go-netrc v1.0.0 h1:QbLMLyCZGj0NA8glAhxUpf1zDg6cxnWgMBbjq40W0gQ= github.com/jdx/go-netrc v1.0.0/go.mod h1:Gh9eFQJnoTNIRHXl2j5bJXA1u84hQWJWgGh569zF3v8= github.com/jhump/protoreflect v1.16.0 h1:54fZg+49widqXYQ0b+usAFHbMkBGR4PpXrsHc8+TBDg= diff --git a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go index e102855cb6..c83d1b52ff 100644 --- a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go +++ b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go @@ -32,10 +32,9 @@ import ( "github.com/bufbuild/buf/private/pkg/encoding" "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/protodescriptor" + "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/tracing" "github.com/bufbuild/protoplugin" - "google.golang.org/protobuf/proto" - "google.golang.org/protobuf/reflect/protoregistry" ) const ( @@ -47,9 +46,7 @@ const ( func Main() { protoplugin.Main( protoplugin.HandlerFunc(handle), - protoplugin.WithUnmarshalOptions(proto.UnmarshalOptions{ - Resolver: (*protoregistry.Types)(nil), - }), + protoplugin.WithExtensionTypeResolver(protoencoding.EmptyResolver), ) } diff --git a/private/buf/cmd/protoc-gen-buf-lint/lint.go b/private/buf/cmd/protoc-gen-buf-lint/lint.go index 4b302acec4..c208f86afd 100644 --- a/private/buf/cmd/protoc-gen-buf-lint/lint.go +++ b/private/buf/cmd/protoc-gen-buf-lint/lint.go @@ -31,10 +31,9 @@ import ( "github.com/bufbuild/buf/private/pkg/encoding" "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/protodescriptor" + "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/tracing" "github.com/bufbuild/protoplugin" - "google.golang.org/protobuf/proto" - "google.golang.org/protobuf/reflect/protoregistry" ) const ( @@ -46,9 +45,7 @@ const ( func Main() { protoplugin.Main( protoplugin.HandlerFunc(handle), - protoplugin.WithUnmarshalOptions(proto.UnmarshalOptions{ - Resolver: (*protoregistry.Types)(nil), - }), + protoplugin.WithExtensionTypeResolver(protoencoding.EmptyResolver), ) } diff --git a/private/pkg/protoencoding/resolver.go b/private/pkg/protoencoding/resolver.go index 9e41aff248..e03945eca5 100644 --- a/private/pkg/protoencoding/resolver.go +++ b/private/pkg/protoencoding/resolver.go @@ -26,6 +26,8 @@ import ( const maxTagNumber = 536870911 // 2^29 - 1 +var EmptyResolver Resolver = emptyResolver{} + func newResolver[F protodescriptor.FileDescriptor](fileDescriptors ...F) (Resolver, error) { if len(fileDescriptors) == 0 { return nil, nil @@ -218,3 +220,33 @@ func (c combinedResolver) FindEnumByName(enum protoreflect.FullName) (protorefle } return nil, protoregistry.NotFound } + +type emptyResolver struct{} + +func (emptyResolver) FindFileByPath(string) (protoreflect.FileDescriptor, error) { + return nil, protoregistry.NotFound +} + +func (emptyResolver) FindDescriptorByName(protoreflect.FullName) (protoreflect.Descriptor, error) { + return nil, protoregistry.NotFound +} + +func (emptyResolver) FindEnumByName(protoreflect.FullName) (protoreflect.EnumType, error) { + return nil, protoregistry.NotFound +} + +func (emptyResolver) FindExtensionByName(protoreflect.FullName) (protoreflect.ExtensionType, error) { + return nil, protoregistry.NotFound +} + +func (emptyResolver) FindExtensionByNumber(protoreflect.FullName, protoreflect.FieldNumber) (protoreflect.ExtensionType, error) { + return nil, protoregistry.NotFound +} + +func (emptyResolver) FindMessageByName(protoreflect.FullName) (protoreflect.MessageType, error) { + return nil, protoregistry.NotFound +} + +func (emptyResolver) FindMessageByURL(string) (protoreflect.MessageType, error) { + return nil, protoregistry.NotFound +} From e28a3e2206d4e5c02f24a0c4e819b9c6a0c6bbfd Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 10 Sep 2024 14:17:30 -0400 Subject: [PATCH 3/4] Move EmptyResolver and add comment --- private/pkg/protoencoding/protoencoding.go | 3 +++ private/pkg/protoencoding/resolver.go | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/private/pkg/protoencoding/protoencoding.go b/private/pkg/protoencoding/protoencoding.go index c2750fed1b..11a1b302bf 100644 --- a/private/pkg/protoencoding/protoencoding.go +++ b/private/pkg/protoencoding/protoencoding.go @@ -23,6 +23,9 @@ import ( "google.golang.org/protobuf/reflect/protoregistry" ) +// EmptyResolver is a resolver that never resolves any descriptors. All methods will return (nil, NotFound). +var EmptyResolver Resolver = emptyResolver{} + // Resolver can resolve files, messages, enums, and extensions. type Resolver interface { protodesc.Resolver diff --git a/private/pkg/protoencoding/resolver.go b/private/pkg/protoencoding/resolver.go index e03945eca5..c75c08615b 100644 --- a/private/pkg/protoencoding/resolver.go +++ b/private/pkg/protoencoding/resolver.go @@ -26,8 +26,6 @@ import ( const maxTagNumber = 536870911 // 2^29 - 1 -var EmptyResolver Resolver = emptyResolver{} - func newResolver[F protodescriptor.FileDescriptor](fileDescriptors ...F) (Resolver, error) { if len(fileDescriptors) == 0 { return nil, nil From 6e064b2cdcab9a45ff7fb0359ff5b336c3a9bfd1 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 10 Sep 2024 16:31:57 -0400 Subject: [PATCH 4/4] Add comment explaining `EmptyResolver` usage --- private/buf/cmd/protoc-gen-buf-breaking/breaking.go | 4 ++++ private/buf/cmd/protoc-gen-buf-lint/lint.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go index c83d1b52ff..ef32924b88 100644 --- a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go +++ b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go @@ -46,6 +46,10 @@ const ( func Main() { protoplugin.Main( protoplugin.HandlerFunc(handle), + // An `EmptyResolver` is passed to protoplugin for unmarshalling instead of defaulting to + // protoregistry.GlobalTypes so that extensions are not inadvertently parsed from generated + // code linked into the binary. Extensions are later reparsed with the descriptorset itself. + // https://github.com/bufbuild/buf/issues/3306 protoplugin.WithExtensionTypeResolver(protoencoding.EmptyResolver), ) } diff --git a/private/buf/cmd/protoc-gen-buf-lint/lint.go b/private/buf/cmd/protoc-gen-buf-lint/lint.go index c208f86afd..d957b2801b 100644 --- a/private/buf/cmd/protoc-gen-buf-lint/lint.go +++ b/private/buf/cmd/protoc-gen-buf-lint/lint.go @@ -45,6 +45,10 @@ const ( func Main() { protoplugin.Main( protoplugin.HandlerFunc(handle), + // An `EmptyResolver` is passed to protoplugin for unmarshalling instead of defaulting to + // protoregistry.GlobalTypes so that extensions are not inadvertently parsed from generated + // code linked into the binary. Extensions are later reparsed with the descriptorset itself. + // https://github.com/bufbuild/buf/issues/3306 protoplugin.WithExtensionTypeResolver(protoencoding.EmptyResolver), ) }