Preserve unknown fields when converting between FileDescriptorProto…

… and `ImageFile` (#1855)

This way, if a field is ever added to the descriptors and then
serialized by a newer version of `buf`, an older version of `buf` won't
mangle it and inadvertently drop those newer fields.

private/bufpkg/bufimage/bufimagetesting/bufimagetesting_test.go

@@ -455,22 +455,19 @@ func TestBasic(t *testing.T) {
 	)
 	diff := cmp.Diff(protoImage, bufimage.ImageToProtoImage(newImage), protocmp.Transform())
 	require.Equal(t, "", diff)
-	// TODO: all of the below proto equality checks should use cmp.Diff
-	require.Equal(
-		t,
-		&descriptorpb.FileDescriptorSet{
-			File: []*descriptorpb.FileDescriptorProto{
-				testProtoImageFileToFileDescriptorProto(protoImageFileAA),
-				testProtoImageFileToFileDescriptorProto(protoImageFileImport),
-				testProtoImageFileToFileDescriptorProto(protoImageFileWellKnownTypeImport),
-				testProtoImageFileToFileDescriptorProto(protoImageFileAB),
-				testProtoImageFileToFileDescriptorProto(protoImageFileBA),
-				testProtoImageFileToFileDescriptorProto(protoImageFileBB),
-				testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
-			},
+	fileDescriptorSet := &descriptorpb.FileDescriptorSet{
+		File: []*descriptorpb.FileDescriptorProto{
+			testProtoImageFileToFileDescriptorProto(protoImageFileAA),
+			testProtoImageFileToFileDescriptorProto(protoImageFileImport),
+			testProtoImageFileToFileDescriptorProto(protoImageFileWellKnownTypeImport),
+			testProtoImageFileToFileDescriptorProto(protoImageFileAB),
+			testProtoImageFileToFileDescriptorProto(protoImageFileBA),
+			testProtoImageFileToFileDescriptorProto(protoImageFileBB),
+			testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
 		},
-		bufimage.ImageToFileDescriptorSet(image),
-	)
+	}
+	diff = cmp.Diff(fileDescriptorSet, bufimage.ImageToFileDescriptorSet(image), protocmp.Transform())
+	require.Equal(t, "", diff)
 	codeGeneratorRequest := &pluginpb.CodeGeneratorRequest{
 		ProtoFile: []*descriptorpb.FileDescriptorProto{
 			testProtoImageFileToFileDescriptorProto(protoImageFileAA),
@@ -490,17 +487,21 @@ func TestBasic(t *testing.T) {
 			"d/d.proto/d.proto",
 		},
 	}
-	require.Equal(
-		t,
+	diff = cmp.Diff(
 		codeGeneratorRequest,
 		bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, false),
+		protocmp.Transform(),
 	)
+	require.Equal(t, "", diff)
+
 	// verify that includeWellKnownTypes is a no-op if includeImports is false
-	require.Equal(
-		t,
+	diff = cmp.Diff(
 		codeGeneratorRequest,
 		bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, true),
+		protocmp.Transform(),
 	)
+	require.Equal(t, "", diff)
+
 	codeGeneratorRequestIncludeImports := &pluginpb.CodeGeneratorRequest{
 		ProtoFile: []*descriptorpb.FileDescriptorProto{
 			testProtoImageFileToFileDescriptorProto(protoImageFileAA),
@@ -522,11 +523,12 @@ func TestBasic(t *testing.T) {
 			"d/d.proto/d.proto",
 		},
 	}
-	require.Equal(
-		t,
+	diff = cmp.Diff(
 		codeGeneratorRequestIncludeImports,
 		bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, false),
+		protocmp.Transform(),
 	)
+	require.Equal(t, "", diff)
 	newImage, err = bufimage.NewImageForCodeGeneratorRequest(codeGeneratorRequest)
 	require.NoError(t, err)
 	AssertImageFilesEqual(
@@ -563,11 +565,12 @@ func TestBasic(t *testing.T) {
 			"d/d.proto/d.proto",
 		},
 	}
-	require.Equal(
-		t,
+	diff = cmp.Diff(
 		codeGeneratorRequestIncludeImportsAndWellKnownTypes,
 		bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, true),
+		protocmp.Transform(),
 	)
+	require.Equal(t, "", diff)
 	// imagesByDir and multiple Image tests
 	imagesByDir, err := bufimage.ImageByDir(image)
 	require.NoError(t, err)
@@ -642,11 +645,12 @@ func TestBasic(t *testing.T) {
 			},
 		},
 	}
-	require.Equal(
-		t,
-		codeGeneratorRequests,
-		bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, false, false),
-	)
+	requestsFromImages := bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, false, false)
+	require.Equal(t, len(codeGeneratorRequests), len(requestsFromImages))
+	for i := range codeGeneratorRequests {
+		diff = cmp.Diff(codeGeneratorRequests[i], requestsFromImages[i], protocmp.Transform())
+		require.Equal(t, "", diff)
+	}
 	codeGeneratorRequestsIncludeImports := []*pluginpb.CodeGeneratorRequest{
 		{
 			ProtoFile: []*descriptorpb.FileDescriptorProto{
@@ -688,11 +692,12 @@ func TestBasic(t *testing.T) {
 			},
 		},
 	}
-	require.Equal(
-		t,
-		codeGeneratorRequestsIncludeImports,
-		bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, true, false),
-	)
+	requestsFromImages = bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, true, false)
+	require.Equal(t, len(codeGeneratorRequestsIncludeImports), len(requestsFromImages))
+	for i := range codeGeneratorRequestsIncludeImports {
+		diff = cmp.Diff(codeGeneratorRequestsIncludeImports[i], requestsFromImages[i], protocmp.Transform())
+		require.Equal(t, "", diff)
+	}
 }
 
 func testProtoImageFileToFileDescriptorProto(imageFile *imagev1.ImageFile) *descriptorpb.FileDescriptorProto {

private/bufpkg/bufimage/util.go

@@ -24,11 +24,16 @@ import (
 	"github.com/bufbuild/buf/private/pkg/normalpath"
 	"github.com/bufbuild/buf/private/pkg/protodescriptor"
 	"github.com/bufbuild/buf/private/pkg/stringutil"
+	"google.golang.org/protobuf/encoding/protowire"
 	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/types/descriptorpb"
 	"google.golang.org/protobuf/types/pluginpb"
 )
 
+// Must match the tag number for ImageFile.buf_extensions defined in proto/buf/alpha/image/v1/image.proto.
+const bufExtensionFieldNumber = 8042
+
 // paths can be either files (ending in .proto) or directories
 // paths must be normalized and validated, and not duplicated
 // if a directory, all .proto files underneath will be included
@@ -345,7 +350,7 @@ func fileDescriptorProtoToProtoImageFile(
 	if len(unusedDependencyIndexes) == 0 {
 		unusedDependencyIndexes = nil
 	}
-	return &imagev1.ImageFile{
+	resultFile := &imagev1.ImageFile{
 		Name:             fileDescriptorProto.Name,
 		Package:          fileDescriptorProto.Package,
 		Syntax:           fileDescriptorProto.Syntax,
@@ -368,6 +373,53 @@ func fileDescriptorProtoToProtoImageFile(
 			ModuleInfo:          protoModuleInfo,
 		},
 	}
+	resultFile.ProtoReflect().SetUnknown(stripBufExtensionField(fileDescriptorProto.ProtoReflect().GetUnknown()))
+	return resultFile
+}
+
+func stripBufExtensionField(unknownFields protoreflect.RawFields) protoreflect.RawFields {
+	// We accumulate the new bytes in result. However, for efficiency, we don't do any
+	// allocation/copying until we have to (i.e. until we actually see the field we're
+	// trying to strip). So result will be left nil and initialized lazily if-and-only-if
+	// we actually need to strip data from unknownFields.
+	var result protoreflect.RawFields
+	bytesRemaining := unknownFields
+	for len(bytesRemaining) > 0 {
+		num, wireType, n := protowire.ConsumeTag(bytesRemaining)
+		if n < 0 {
+			// shouldn't be possible unless explicitly set to invalid bytes via reflection
+			return unknownFields
+		}
+		var skip bool
+		if num == bufExtensionFieldNumber {
+			// We need to strip this field.
+			skip = true
+			if result == nil {
+				// Lazily initialize result to the preface that we've already examined.
+				result = append(
+					make(protoreflect.RawFields, 0, len(unknownFields)),
+					unknownFields[:len(unknownFields)-len(bytesRemaining)]...,
+				)
+			}
+		} else if result != nil {
+			// accumulate data in result as we go
+			result = append(result, bytesRemaining[:n]...)
+		}
+		bytesRemaining = bytesRemaining[n:]
+		n = protowire.ConsumeFieldValue(num, wireType, bytesRemaining)
+		if n < 0 {
+			return unknownFields
+		}
+		if !skip && result != nil {
+			result = append(result, bytesRemaining[:n]...)
+		}
+		bytesRemaining = bytesRemaining[n:]
+	}
+	if result == nil {
+		// we did not have to remove anything
+		return unknownFields
+	}
+	return result
 }
 
 func imageToCodeGeneratorRequest(