From d8ab8742a8eb0945d242db0b4bc78f5ecfd52f25 Mon Sep 17 00:00:00 2001 From: Deokjin Kim Date: Tue, 10 Jan 2023 00:25:41 +0900 Subject: [PATCH 1/4] http: refactor to use `validateHeaderName` Remove duplicate implementation by using validateHeaderName. --- lib/_http_outgoing.js | 4 +--- test/parallel/test-http-outgoing-proto.js | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 8c80eabaec9e74..7d2c6a6384dfe6 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -933,9 +933,7 @@ OutgoingMessage.prototype.addTrailers = function addTrailers(headers) { field = key; value = headers[key]; } - if (typeof field !== 'string' || !field || !checkIsHttpToken(field)) { - throw new ERR_INVALID_HTTP_TOKEN('Trailer name', field); - } + validateHeaderName(field); // Check if the field must be sent several times const isArrayValue = ArrayIsArray(value); diff --git a/test/parallel/test-http-outgoing-proto.js b/test/parallel/test-http-outgoing-proto.js index 2d265c7ba64968..1f405e2073f4ca 100644 --- a/test/parallel/test-http-outgoing-proto.js +++ b/test/parallel/test-http-outgoing-proto.js @@ -117,7 +117,6 @@ assert.throws(() => { }, { code: 'ERR_INVALID_HTTP_TOKEN', name: 'TypeError', - message: 'Trailer name must be a valid HTTP token ["あ"]' }); assert.throws(() => { From a23f8f638ac368b3fc4a4eaebbe8ed35620c4d2a Mon Sep 17 00:00:00 2001 From: Deokjin Kim Date: Tue, 10 Jan 2023 08:52:34 +0900 Subject: [PATCH 2/4] add `label` parameter to validateHeaderName To customize error message, add label parameter --- doc/api/http.md | 7 ++++++- lib/_http_outgoing.js | 6 +++--- test/parallel/test-http-outgoing-proto.js | 1 + 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/doc/api/http.md b/doc/api/http.md index fec7b07b8d8b41..cb85de0fa2c87f 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -3661,13 +3661,18 @@ Passing an `AbortSignal` and then calling `abort` on the corresponding `AbortController` will behave the same way as calling `.destroy()` on the request itself. -## `http.validateHeaderName(name)` +## `http.validateHeaderName(name[, label])` * `name` {string} +* `label` {string} Label for error message. **Default:** `Header name`. Performs the low-level validations on the provided `name` that are done when `res.setHeader(name, value)` is called. diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 7d2c6a6384dfe6..6a247690ea3bc2 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -628,9 +628,9 @@ function matchHeader(self, state, field, value) { } } -const validateHeaderName = hideStackFrames((name) => { +const validateHeaderName = hideStackFrames((name, label = 'Header name') => { if (typeof name !== 'string' || !name || !checkIsHttpToken(name)) { - throw new ERR_INVALID_HTTP_TOKEN('Header name', name); + throw new ERR_INVALID_HTTP_TOKEN(label, name); } }); @@ -933,7 +933,7 @@ OutgoingMessage.prototype.addTrailers = function addTrailers(headers) { field = key; value = headers[key]; } - validateHeaderName(field); + validateHeaderName(field, 'Trailer name'); // Check if the field must be sent several times const isArrayValue = ArrayIsArray(value); diff --git a/test/parallel/test-http-outgoing-proto.js b/test/parallel/test-http-outgoing-proto.js index 1f405e2073f4ca..2d265c7ba64968 100644 --- a/test/parallel/test-http-outgoing-proto.js +++ b/test/parallel/test-http-outgoing-proto.js @@ -117,6 +117,7 @@ assert.throws(() => { }, { code: 'ERR_INVALID_HTTP_TOKEN', name: 'TypeError', + message: 'Trailer name must be a valid HTTP token ["あ"]' }); assert.throws(() => { From b1681f73f3674ac6a768a8ee72bc1443f8d950aa Mon Sep 17 00:00:00 2001 From: Deokjin Kim Date: Tue, 10 Jan 2023 23:05:34 +0900 Subject: [PATCH 3/4] use `||` instead of default parameter --- lib/_http_outgoing.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 6a247690ea3bc2..60ea7ca5ef9c29 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -628,9 +628,9 @@ function matchHeader(self, state, field, value) { } } -const validateHeaderName = hideStackFrames((name, label = 'Header name') => { +const validateHeaderName = hideStackFrames((name, label) => { if (typeof name !== 'string' || !name || !checkIsHttpToken(name)) { - throw new ERR_INVALID_HTTP_TOKEN(label, name); + throw new ERR_INVALID_HTTP_TOKEN(label || 'Header name', name); } }); From 254670133f086aaf09c293bf298c91a44b4811a1 Mon Sep 17 00:00:00 2001 From: Deokjin Kim Date: Thu, 12 Jan 2023 08:11:46 +0900 Subject: [PATCH 4/4] describe default value of label as string Co-authored-by: mscdex --- doc/api/http.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/http.md b/doc/api/http.md index cb85de0fa2c87f..bf576ce2fa0e1d 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -3672,7 +3672,7 @@ changes: --> * `name` {string} -* `label` {string} Label for error message. **Default:** `Header name`. +* `label` {string} Label for error message. **Default:** `'Header name'`. Performs the low-level validations on the provided `name` that are done when `res.setHeader(name, value)` is called.