From 7c0511f2df79226d1575dff74b559945da44e2ec Mon Sep 17 00:00:00 2001 From: Justin Timmons Date: Wed, 21 Feb 2024 22:19:37 -0500 Subject: [PATCH] fix(grpc-reflection): handle references to root-level message types in default package --- .../grpc-reflection/proto/sample/sample.proto | 1 + .../proto/sample/unscoped.proto | 10 +++++++++ .../src/implementations/common/utils.ts | 4 ++-- .../src/implementations/reflection-v1.ts | 4 ++-- .../test/test-reflection-v1-implementation.ts | 22 +++++++++++++++++-- packages/grpc-reflection/test/test-utils.ts | 6 +++++ 6 files changed, 41 insertions(+), 6 deletions(-) create mode 100644 packages/grpc-reflection/proto/sample/unscoped.proto diff --git a/packages/grpc-reflection/proto/sample/sample.proto b/packages/grpc-reflection/proto/sample/sample.proto index acf969c1a..6785ab51a 100644 --- 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 new file mode 100644 index 000000000..607ee35ca --- /dev/null +++ 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 index 4490abd6e..e57b13655 100644 --- 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 index 3858d7f42..e3702a02e 100644 --- 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 index 552160f74..436206173 100644 --- 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 index 2f197652a..7242f8e32 100644 --- 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')), ''); + }); });