protoparse should not return dynamic extensions, take 2: must also examine dependencies

Author: jhump

desc/protoparse/options_test.go

@@ -3,7 +3,6 @@ package protoparse
 import (
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"strings"
 	"testing"
@@ -147,7 +146,7 @@ func TestOptionsInUnlinkedFiles(t *testing.T) {
 func accessorFor(name, contents string) FileAccessor {
 	return func(n string) (io.ReadCloser, error) {
 		if n == name {
-			return ioutil.NopCloser(strings.NewReader(contents)), nil
+			return io.NopCloser(strings.NewReader(contents)), nil
 		}
 		return nil, os.ErrNotExist
 	}

desc/protoparse/parser.go

@@ -179,11 +179,10 @@ func (p Parser) ParseFiles(filenames ...string) ([]*desc.FileDescriptor, error)
 	}
 
 	fds := make([]protoreflect.FileDescriptor, len(results))
+	alreadySeen := make(map[string]struct{}, len(results))
 	for i, res := range results {
-		if linkRes, ok := res.(linker.Result); ok {
-			removeDynamicExtensions(linkRes.FileDescriptorProto())
-		}
-		fds[i] = results[i]
+		removeDynamicExtensions(res, alreadySeen)
+		fds[i] = res
 	}
 	return desc.WrapFiles(fds)
 }
@@ -261,7 +260,7 @@ func (p Parser) ParseFilesButDoNotLink(filenames ...string) ([]*descriptorpb.Fil
 			if err != nil {
 				return nil, err
 			}
-			removeDynamicExtensions(protos[i])
+			removeDynamicExtensionsFromProto(protos[i])
 		}
 		if p.IncludeSourceCodeInfo {
 			protos[i].SourceCodeInfo = sourceinfo.GenerateSourceInfo(res.AST(), optsIndex)
@@ -597,7 +596,23 @@ func fixupFilenames(protos map[string]parser.Result) map[string]parser.Result {
 	return revisedProtos
 }
 
-func removeDynamicExtensions(fd *descriptorpb.FileDescriptorProto) {
+func removeDynamicExtensions(fd protoreflect.FileDescriptor, alreadySeen map[string]struct{}) {
+	if _, ok := alreadySeen[fd.Path()]; ok {
+		// already processed
+		return
+	}
+	alreadySeen[fd.Path()] = struct{}{}
+	res, ok := fd.(linker.Result)
+	if ok {
+		removeDynamicExtensionsFromProto(res.FileDescriptorProto())
+	}
+	// also remove extensions from dependencies
+	for i, length := 0, fd.Imports().Len(); i < length; i++ {
+		removeDynamicExtensions(fd.Imports().Get(i).FileDescriptor, alreadySeen)
+	}
+}
+
+func removeDynamicExtensionsFromProto(fd *descriptorpb.FileDescriptorProto) {
 	// protocompile returns descriptors with dynamic extension fields for custom options.
 	// But protoparse only used known custom options and everything else defined in the
 	// sources would be stored as unrecognized fields. So to bridge the difference in

desc/protoparse/parser_test.go

@@ -7,14 +7,18 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"sort"
+	"strings"
 	"testing"
 
 	"github.com/bufbuild/protocompile/parser"
 	"github.com/bufbuild/protocompile/reporter"
+	"github.com/golang/protobuf/proto"
 	"google.golang.org/protobuf/types/descriptorpb"
 
 	"github.com/jhump/protoreflect/desc"
+	"github.com/jhump/protoreflect/internal/testprotos"
 	"github.com/jhump/protoreflect/internal/testutil"
 )
 
@@ -370,6 +374,33 @@ func TestParseFilesWithDependencies(t *testing.T) {
 	})
 }
 
+func TestParseFilesReturnsKnownExtensions(t *testing.T) {
+	accessor := func(filename string) (io.ReadCloser, error) {
+		if filename == "desc_test3.proto" {
+			return io.NopCloser(strings.NewReader(`
+				syntax = "proto3";
+				import "desc_test_complex.proto";
+				message Foo {
+					foo.bar.Simple field = 1;
+				}
+			`)), nil
+		}
+		return os.Open(filepath.Join("../../internal/testprotos", filename))
+	}
+	p := Parser{
+		Accessor: accessor,
+	}
+	fds, err := p.ParseFiles("desc_test3.proto")
+	testutil.Ok(t, err)
+	fd := fds[0].GetDependencies()[0]
+	md := fd.FindMessage("foo.bar.Test.Nested._NestedNested")
+	testutil.Require(t, md != nil)
+	val, err := proto.GetExtension(md.GetOptions(), testprotos.E_Rept)
+	testutil.Ok(t, err)
+	_, ok := val.([]*testprotos.Test)
+	testutil.Require(t, ok)
+}
+
 func TestParseCommentsBeforeDot(t *testing.T) {
 	accessor := FileContentsFromMap(map[string]string{
 		"test.proto": `