Skip to content

Commit 7c0511f

Browse files
committed
fix(grpc-reflection): handle references to root-level message types in default package
1 parent 0ba7d70 commit 7c0511f

File tree

6 files changed

+41
-6
lines changed

6 files changed

+41
-6
lines changed

packages/grpc-reflection/proto/sample/sample.proto

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ syntax = "proto3";
33
package sample;
44

55
import 'vendor.proto';
6+
import 'unscoped.proto';
67

78
service SampleService {
89
rpc Hello (HelloRequest) returns (HelloResponse) {}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
syntax = "proto3";
2+
3+
message TopLevelMessage {
4+
bool value = 1;
5+
}
6+
7+
message ProcessRequest {
8+
string key = 1;
9+
TopLevelMessage message = 2;
10+
}

packages/grpc-reflection/src/implementations/common/utils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
* @example scope('grpc.reflection.v1.Type') == 'grpc.reflection.v1'
44
*/
55
export const scope = (path: string, separator: string = '.') => {
6-
if (!path.includes(separator)) {
6+
if (!path.includes(separator) || path === separator) {
77
return '';
88
}
99

10-
return path.split(separator).slice(0, -1).join(separator);
10+
return path.split(separator).slice(0, -1).join(separator) || separator;
1111
};

packages/grpc-reflection/src/implementations/reflection-v1.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,12 @@ export class ReflectionV1Implementation {
114114
let referencedFile: IFileDescriptorProto | null = null;
115115
if (ref.startsWith('.')) {
116116
// absolute reference -- just remove the leading '.' and use the ref directly
117-
referencedFile = this.symbols[ref.replace(/^\./, '')];
117+
referencedFile = this.symbols[ref.slice(1)];
118118
} else {
119119
// relative reference -- need to seek upwards up the current package scope until we find it
120120
let pkg = pkgScope;
121121
while (pkg && !referencedFile) {
122-
referencedFile = this.symbols[`${pkg}.${ref}`];
122+
referencedFile = this.symbols[`${pkg.replace(/\.$/, '')}.${ref}`];
123123
pkg = scope(pkg);
124124
}
125125

packages/grpc-reflection/test/test-reflection-v1-implementation.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ describe('GrpcReflectionService', () => {
1010

1111
beforeEach(async () => {
1212
const root = protoLoader.loadSync(path.join(__dirname, '../proto/sample/sample.proto'), {
13-
includeDirs: [path.join(__dirname, '../proto/sample/vendor')]
13+
includeDirs: [
14+
path.join(__dirname, '../proto/sample/'),
15+
path.join(__dirname, '../proto/sample/vendor')
16+
]
1417
});
1518

1619
reflectionService = new ReflectionV1Implementation(root);
@@ -25,7 +28,10 @@ describe('GrpcReflectionService', () => {
2528

2629
it('whitelists services properly', () => {
2730
const root = protoLoader.loadSync(path.join(__dirname, '../proto/sample/sample.proto'), {
28-
includeDirs: [path.join(__dirname, '../proto/sample/vendor')]
31+
includeDirs: [
32+
path.join(__dirname, '../proto/sample/'),
33+
path.join(__dirname, '../proto/sample/vendor')
34+
]
2935
});
3036

3137
reflectionService = new ReflectionV1Implementation(root, { services: ['sample.SampleService'] });
@@ -127,6 +133,18 @@ describe('GrpcReflectionService', () => {
127133
);
128134
});
129135

136+
it('finds unscoped package types', () => {
137+
const descriptors = reflectionService
138+
.fileContainingSymbol('.TopLevelMessage')
139+
.fileDescriptorProto.map(f => FileDescriptorProto.decode(f) as IFileDescriptorProto);
140+
141+
const names = descriptors.map((desc) => desc.name);
142+
assert.deepEqual(
143+
new Set(names),
144+
new Set(['root.proto']),
145+
);
146+
});
147+
130148
it('merges files based on package name', () => {
131149
const descriptors = reflectionService
132150
.fileContainingSymbol('vendor.CommonMessage')

packages/grpc-reflection/test/test-utils.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,14 @@ describe('scope', () => {
77
assert.strictEqual(scope('grpc.health.v1.HealthCheckResponse.ServiceStatus'), 'grpc.health.v1.HealthCheckResponse');
88
assert.strictEqual(scope(scope(scope(scope('grpc.health.v1.HealthCheckResponse.ServiceStatus')))), 'grpc');
99
});
10+
1011
it('returns an empty package when at the top', () => {
1112
assert.strictEqual(scope('Message'), '');
1213
assert.strictEqual(scope(''), '');
1314
});
15+
16+
it('handles globally scoped references', () => {
17+
assert.strictEqual(scope('.Message'), '.');
18+
assert.strictEqual(scope(scope('.Message')), '');
19+
});
1420
});

0 commit comments

Comments
 (0)