From dd414b6ddcccf39a33b9a02419606b31190839e4 Mon Sep 17 00:00:00 2001 From: James Sharp Date: Wed, 20 Nov 2019 17:36:05 +0000 Subject: [PATCH 1/2] grpc-js: fix error messages truncating at commas --- packages/grpc-js/src/metadata-status-filter.ts | 2 +- packages/grpc-js/src/server-call.ts | 4 +++- packages/grpc-js/test/test-server-errors.ts | 12 ++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/grpc-js/src/metadata-status-filter.ts b/packages/grpc-js/src/metadata-status-filter.ts index 5ca401f7..52c27682 100644 --- a/packages/grpc-js/src/metadata-status-filter.ts +++ b/packages/grpc-js/src/metadata-status-filter.ts @@ -38,7 +38,7 @@ export class MetadataStatusFilter extends BaseFilter implements Filter { metadata.remove('grpc-status'); } if (typeof metadataMap['grpc-message'] === 'string') { - details = decodeURI(metadataMap['grpc-message'] as string); + details = decodeURIComponent(metadataMap['grpc-message'] as string); metadata.remove('grpc-message'); } return { code, details, metadata }; diff --git a/packages/grpc-js/src/server-call.ts b/packages/grpc-js/src/server-call.ts index 02353bbe..b755b5e6 100644 --- a/packages/grpc-js/src/server-call.ts +++ b/packages/grpc-js/src/server-call.ts @@ -484,7 +484,9 @@ export class Http2ServerCallStream< const trailersToSend = Object.assign( { [GRPC_STATUS_HEADER]: statusObj.code, - [GRPC_MESSAGE_HEADER]: encodeURI(statusObj.details as string), + [GRPC_MESSAGE_HEADER]: encodeURIComponent( + statusObj.details as string + ), }, statusObj.metadata.toHttp2Headers() ); diff --git a/packages/grpc-js/test/test-server-errors.ts b/packages/grpc-js/test/test-server-errors.ts index ffabaac5..7c611b9b 100644 --- a/packages/grpc-js/test/test-server-errors.ts +++ b/packages/grpc-js/test/test-server-errors.ts @@ -699,6 +699,18 @@ describe('Other conditions', () => { } ); }); + + it('for an error message with a comma', done => { + client.unary( + { error: true, message: 'an error message, with a comma' }, + (err: ServiceError, data: any) => { + assert(err); + assert.strictEqual(err.code, grpc.status.UNKNOWN); + assert.strictEqual(err.details, 'an error message, with a comma'); + done(); + } + ); + }); }); }); From 2dce08dc99fd8ca010dba249c4104ec6e1f75105 Mon Sep 17 00:00:00 2001 From: James Sharp Date: Thu, 21 Nov 2019 21:48:50 +0000 Subject: [PATCH 2/2] Only custom-metadata headers should be parsed as comma-separated --- .../grpc-js/src/metadata-status-filter.ts | 2 +- packages/grpc-js/src/metadata.ts | 20 +++++++++++++++---- packages/grpc-js/src/server-call.ts | 4 +--- test/api/error_test.js | 8 ++++++++ 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/packages/grpc-js/src/metadata-status-filter.ts b/packages/grpc-js/src/metadata-status-filter.ts index 52c27682..5ca401f7 100644 --- a/packages/grpc-js/src/metadata-status-filter.ts +++ b/packages/grpc-js/src/metadata-status-filter.ts @@ -38,7 +38,7 @@ export class MetadataStatusFilter extends BaseFilter implements Filter { metadata.remove('grpc-status'); } if (typeof metadataMap['grpc-message'] === 'string') { - details = decodeURIComponent(metadataMap['grpc-message'] as string); + details = decodeURI(metadataMap['grpc-message'] as string); metadata.remove('grpc-message'); } return { code, details, metadata }; diff --git a/packages/grpc-js/src/metadata.ts b/packages/grpc-js/src/metadata.ts index 873e5ff5..4b41aa25 100644 --- a/packages/grpc-js/src/metadata.ts +++ b/packages/grpc-js/src/metadata.ts @@ -36,6 +36,10 @@ function isBinaryKey(key: string): boolean { return key.endsWith('-bin'); } +function isCustomMetadata(key: string): boolean { + return !key.startsWith('grpc-'); +} + function normalizeKey(key: string): string { return key.toLowerCase(); } @@ -260,9 +264,13 @@ export class Metadata { result.add(key, Buffer.from(value, 'base64')); }); } else if (values !== undefined) { - values.split(',').forEach(v => { - result.add(key, Buffer.from(v.trim(), 'base64')); - }); + if (isCustomMetadata(key)) { + values.split(',').forEach(v => { + result.add(key, Buffer.from(v.trim(), 'base64')); + }); + } else { + result.add(key, Buffer.from(values, 'base64')); + } } } else { if (Array.isArray(values)) { @@ -270,7 +278,11 @@ export class Metadata { result.add(key, value); }); } else if (values !== undefined) { - values.split(',').forEach(v => result.add(key, v.trim())); + if (isCustomMetadata(key)) { + values.split(',').forEach(v => result.add(key, v.trim())); + } else { + result.add(key, values); + } } } } catch (error) { diff --git a/packages/grpc-js/src/server-call.ts b/packages/grpc-js/src/server-call.ts index b755b5e6..02353bbe 100644 --- a/packages/grpc-js/src/server-call.ts +++ b/packages/grpc-js/src/server-call.ts @@ -484,9 +484,7 @@ export class Http2ServerCallStream< const trailersToSend = Object.assign( { [GRPC_STATUS_HEADER]: statusObj.code, - [GRPC_MESSAGE_HEADER]: encodeURIComponent( - statusObj.details as string - ), + [GRPC_MESSAGE_HEADER]: encodeURI(statusObj.details as string), }, statusObj.metadata.toHttp2Headers() ); diff --git a/test/api/error_test.js b/test/api/error_test.js index 23a8d0d8..ea3bec91 100644 --- a/test/api/error_test.js +++ b/test/api/error_test.js @@ -553,6 +553,14 @@ describe(`${anyGrpc.clientName} client -> ${anyGrpc.serverName} server`, functio done(); }); }); + it('for an error message with a comma', function(done) { + client.unary({error: true, message: 'a message, with a comma'}, function(err, data) { + assert(err); + assert.strictEqual(err.code, clientGrpc.status.UNKNOWN); + assert.strictEqual(err.details, 'a message, with a comma'); + done(); + }); + }); }); }); }); \ No newline at end of file