Merge pull request #2673 from jtimmons/fix/reflection-root-package-refs

fix(grpc-reflection): [#2671] handle references to root-level message types in default package
grpc · Feb 23, 2024 · 9886ee2 · 9886ee2
2 parents 0ba7d70 + 7c0511f
commit 9886ee2
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 6 deletions.
diff --git a/packages/grpc-reflection/proto/sample/sample.proto b/packages/grpc-reflection/proto/sample/sample.proto
@@ -3,6 +3,7 @@ syntax = "proto3";
 package sample;
 
 import 'vendor.proto';
+import 'unscoped.proto';
 
 service SampleService {
   rpc Hello (HelloRequest) returns (HelloResponse) {}

diff --git a/packages/grpc-reflection/proto/sample/unscoped.proto b/packages/grpc-reflection/proto/sample/unscoped.proto
@@ -0,0 +1,10 @@
+syntax = "proto3";
+
+message TopLevelMessage {
+  bool value = 1;
+}
+
+message ProcessRequest {
+  string key = 1;
+  TopLevelMessage message = 2;
+}
diff --git a/packages/grpc-reflection/src/implementations/common/utils.ts b/packages/grpc-reflection/src/implementations/common/utils.ts
@@ -3,9 +3,9 @@
  * @example scope('grpc.reflection.v1.Type') == 'grpc.reflection.v1'
  */
 export const scope = (path: string, separator: string = '.') => {
-  if (!path.includes(separator)) {
+  if (!path.includes(separator) || path === separator) {
     return '';
   }
 
-  return path.split(separator).slice(0, -1).join(separator);
+  return path.split(separator).slice(0, -1).join(separator) || separator;
 };
diff --git a/packages/grpc-reflection/src/implementations/reflection-v1.ts b/packages/grpc-reflection/src/implementations/reflection-v1.ts
@@ -114,12 +114,12 @@ export class ReflectionV1Implementation {
       let referencedFile: IFileDescriptorProto | null = null;
       if (ref.startsWith('.')) {
         // absolute reference -- just remove the leading '.' and use the ref directly
-        referencedFile = this.symbols[ref.replace(/^\./, '')];
+        referencedFile = this.symbols[ref.slice(1)];
       } else {
         // relative reference -- need to seek upwards up the current package scope until we find it
         let pkg = pkgScope;
         while (pkg && !referencedFile) {
-          referencedFile = this.symbols[`${pkg}.${ref}`];
+          referencedFile = this.symbols[`${pkg.replace(/\.$/, '')}.${ref}`];
           pkg = scope(pkg);
         }
 

diff --git a/packages/grpc-reflection/test/test-reflection-v1-implementation.ts b/packages/grpc-reflection/test/test-reflection-v1-implementation.ts
@@ -10,7 +10,10 @@ describe('GrpcReflectionService', () => {
 
   beforeEach(async () => {
     const root = protoLoader.loadSync(path.join(__dirname, '../proto/sample/sample.proto'), {
-      includeDirs: [path.join(__dirname, '../proto/sample/vendor')]
+      includeDirs: [
+        path.join(__dirname, '../proto/sample/'),
+        path.join(__dirname, '../proto/sample/vendor')
+      ]
     });
 
     reflectionService = new ReflectionV1Implementation(root);
@@ -25,7 +28,10 @@ describe('GrpcReflectionService', () => {
 
     it('whitelists services properly', () => {
       const root = protoLoader.loadSync(path.join(__dirname, '../proto/sample/sample.proto'), {
-        includeDirs: [path.join(__dirname, '../proto/sample/vendor')]
+        includeDirs: [
+          path.join(__dirname, '../proto/sample/'),
+          path.join(__dirname, '../proto/sample/vendor')
+        ]
       });
 
       reflectionService = new ReflectionV1Implementation(root, { services: ['sample.SampleService'] });
@@ -127,6 +133,18 @@ describe('GrpcReflectionService', () => {
       );
     });
 
+    it('finds unscoped package types', () => {
+      const descriptors = reflectionService
+        .fileContainingSymbol('.TopLevelMessage')
+        .fileDescriptorProto.map(f => FileDescriptorProto.decode(f) as IFileDescriptorProto);
+
+      const names = descriptors.map((desc) => desc.name);
+      assert.deepEqual(
+        new Set(names),
+        new Set(['root.proto']),
+      );
+    });
+
     it('merges files based on package name', () => {
       const descriptors = reflectionService
         .fileContainingSymbol('vendor.CommonMessage')

diff --git a/packages/grpc-reflection/test/test-utils.ts b/packages/grpc-reflection/test/test-utils.ts
@@ -7,8 +7,14 @@ describe('scope', () => {
     assert.strictEqual(scope('grpc.health.v1.HealthCheckResponse.ServiceStatus'), 'grpc.health.v1.HealthCheckResponse');
     assert.strictEqual(scope(scope(scope(scope('grpc.health.v1.HealthCheckResponse.ServiceStatus')))), 'grpc');
   });
+
   it('returns an empty package when at the top', () => {
     assert.strictEqual(scope('Message'), '');
     assert.strictEqual(scope(''), '');
   });
+
+  it('handles globally scoped references', () => {
+    assert.strictEqual(scope('.Message'), '.');
+    assert.strictEqual(scope(scope('.Message')), '');
+  });
 });