Remove resolution of legacy protovalidate extension (#243)

It is unlikely this is being used anywhere anymore. Unfortunately, if it
is, this will cause validation to silently pass when it should not.

Author: jchadwick-buf

resolve/resolve.go

@@ -16,43 +16,32 @@ package resolve
 
 import (
 	"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
-	"google.golang.org/protobuf/encoding/protowire"
 	"google.golang.org/protobuf/proto"
-	"google.golang.org/protobuf/reflect/protodesc"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/reflect/protoregistry"
-	"google.golang.org/protobuf/types/descriptorpb"
-	"google.golang.org/protobuf/types/dynamicpb"
-)
-
-const (
-	legacyExtensionIndex protowire.Number = 51071 // protovalidate versions < v0.2.0
 )
 
 //nolint:gochecknoglobals // static data, only want single instance
 var resolver = newExtensionResolver()
 
-// MessageRules returns the MessageRules option set for the
-// MessageDescriptor.
+// MessageRules returns the MessageRules option set for the MessageDescriptor.
 func MessageRules(desc protoreflect.MessageDescriptor) (*validate.MessageRules, error) {
 	return resolve[*validate.MessageRules](desc.Options(), validate.E_Message)
 }
 
-// OneofRules returns the OneofRules option set for the
-// OneofDescriptor.
+// OneofRules returns the OneofRules option set for the OneofDescriptor.
 func OneofRules(desc protoreflect.OneofDescriptor) (*validate.OneofRules, error) {
 	return resolve[*validate.OneofRules](desc.Options(), validate.E_Oneof)
 }
 
-// FieldRules returns the FieldRules option set for the
-// FieldDescriptor.
+// FieldRules returns the FieldRules option set for the FieldDescriptor.
 func FieldRules(desc protoreflect.FieldDescriptor) (*validate.FieldRules, error) {
 	return resolve[*validate.FieldRules](desc.Options(), validate.E_Field)
 }
 
-// PredefinedRules returns the PredefinedRules option set for
-// the FieldDescriptor. Note that this value is only meaningful if it is set on
-// a field or extension of a field rule message. This method is provided for
+// PredefinedRules returns the PredefinedRules option set for the
+// FieldDescriptor. Note that this value is only meaningful if it is set on a
+// field or extension of a field rule message. This method is provided for
 // convenience.
 func PredefinedRules(desc protoreflect.FieldDescriptor) (*validate.PredefinedRules, error) {
 	return resolve[*validate.PredefinedRules](desc.Options(), validate.E_Predefined)
@@ -96,27 +85,18 @@ func resolve[C proto.Message](
 type extensionResolver struct {
 	// types is a types that just contains the protovalidate extensions.
 	types *protoregistry.Types
-
-	// legacyExtensionMap is a mapping from current protovalidate extensions to
-	// legacy protovalidate extensions, used for backwards compatibility. This
-	// map will not be modified, so it is safe to read concurrently.
-	legacyExtensionMap map[protoreflect.ExtensionType]protoreflect.ExtensionType
 }
 
 // newExtensionResolver creates a new extension resolver. This is only called at
 // init and will panic if it fails.
 func newExtensionResolver() extensionResolver {
 	resolver := extensionResolver{
-		types:              &protoregistry.Types{},
-		legacyExtensionMap: make(map[protoreflect.ExtensionType]protoreflect.ExtensionType),
+		types: &protoregistry.Types{},
 	}
 	resolver.register(validate.E_Field)
 	resolver.register(validate.E_Message)
 	resolver.register(validate.E_Oneof)
 	resolver.register(validate.E_Predefined)
-	resolver.registerLegacy(validate.E_Field)
-	resolver.registerLegacy(validate.E_Message)
-	resolver.registerLegacy(validate.E_Oneof)
 	return resolver
 }
 
@@ -129,53 +109,24 @@ func (resolver extensionResolver) register(extension protoreflect.ExtensionType)
 	}
 }
 
-// registerLegacy creates and registers a legacy extension.
-func (resolver extensionResolver) registerLegacy(extension protoreflect.ExtensionType) {
-	fileDescriptor, err := protodesc.NewFile(&descriptorpb.FileDescriptorProto{
-		Name:    proto.String("buf/validate/validate_legacy.proto"),
-		Package: proto.String("buf.validate"),
-		Dependency: []string{
-			"buf/validate/validate.proto",
-			"google/protobuf/descriptor.proto",
-		},
-		Extension: []*descriptorpb.FieldDescriptorProto{
-			{
-				Name:     proto.String(string(extension.TypeDescriptor().Name()) + "_legacy"),
-				Number:   proto.Int32(int32(legacyExtensionIndex)),
-				Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
-				Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
-				TypeName: proto.String(string(extension.TypeDescriptor().Message().FullName())),
-				Extendee: proto.String(string(extension.TypeDescriptor().ContainingMessage().FullName())),
-			},
-		},
-	}, protoregistry.GlobalFiles)
-	if err != nil {
-		//nolint:forbidigo // this needs to be a fatal at init
-		panic(err)
-	}
-	legacyExtension := dynamicpb.NewExtensionType(fileDescriptor.Extensions().Get(0))
-	resolver.register(legacyExtension)
-	resolver.legacyExtensionMap[extension] = legacyExtension
-}
-
 // resolve handles the majority of extension resolution logic. This will return
 // a proto.Message for the given extension if the message has the tag number of
-// the provided extension (or an equivalent legacy extension). If there was no
-// such tag number present in the known or unknown fields, this method will
-// return nil. Note that the returned message may be dynamicpb.Message or
-// another type, and thus may need to still be reparsed if needed.
+// the provided extension. If there was no such tag number present in the known
+// or unknown fields, this method will return nil. Note that the returned
+// message may be dynamicpb.Message or another type, and thus may need to still
+// be reparsed if needed.
 func (resolver extensionResolver) resolve(
 	options proto.Message,
 	extensionType protoreflect.ExtensionType,
 ) (msg proto.Message, err error) {
-	msg = resolver.getExtensionOrLegacy(options, extensionType)
+	msg = resolver.getExtension(options, extensionType)
 	if msg == nil {
 		if unknown := options.ProtoReflect().GetUnknown(); len(unknown) > 0 {
 			reparsedOptions := options.ProtoReflect().Type().New().Interface()
 			if err = (proto.UnmarshalOptions{
 				Resolver: resolver.types,
 			}).Unmarshal(unknown, reparsedOptions); err == nil {
-				msg = resolver.getExtensionOrLegacy(reparsedOptions, extensionType)
+				msg = resolver.getExtension(reparsedOptions, extensionType)
 			}
 		}
 	}
@@ -185,11 +136,11 @@ func (resolver extensionResolver) resolve(
 	return msg, nil
 }
 
-// getExtensionOrLegacy gets the extension extensionType on message, or if it is
-// not found, the corresponding legacy extensionType. Unlike proto.GetExtension,
-// this method will not panic if the runtime type of the extension is unexpected
-// and returns nil if the extension is not present.
-func (resolver extensionResolver) getExtensionOrLegacy(
+// getExtension gets the extension extensionType on message, or if it is not
+// found, nil. Unlike proto.GetExtension, this method will not panic if the
+// runtime type of the extension is unexpected and returns nil if the extension
+// is not present.
+func (resolver extensionResolver) getExtension(
 	message proto.Message,
 	extensionType protoreflect.ExtensionType,
 ) proto.Message {
@@ -198,9 +149,5 @@ func (resolver extensionResolver) getExtensionOrLegacy(
 		extension, _ := reflect.Get(extensionType.TypeDescriptor()).Interface().(protoreflect.Message)
 		return extension.Interface()
 	}
-	legacyExtensionType, ok := resolver.legacyExtensionMap[extensionType]
-	if !ok {
-		return nil
-	}
-	return resolver.getExtensionOrLegacy(message, legacyExtensionType)
+	return nil
 }

resolve/resolve_test.go

@@ -84,24 +84,6 @@ func TestResolve(t *testing.T) {
 				return options
 			},
 		},
-		{
-			name: "Legacy",
-			builder: func() proto.Message {
-				var unknownBytes []byte
-				unknownBytes = protowire.AppendTag(
-					unknownBytes,
-					legacyExtensionIndex,
-					protowire.BytesType,
-				)
-				unknownBytes = protowire.AppendBytes(
-					unknownBytes,
-					expectedRulesBytes,
-				)
-				options := &descriptorpb.FieldOptions{}
-				options.ProtoReflect().SetUnknown(protoreflect.RawFields(unknownBytes))
-				return options
-			},
-		},
 	}
 
 	for _, tc := range tests {