From a27f9e5f44e90071f75b2a24fce168c3778c3135 Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Thu, 5 Jan 2023 17:45:53 +0100 Subject: [PATCH 01/12] http: res.setHeaders first implementation --- lib/_http_outgoing.js | 28 ++++++++ lib/_http_server.js | 23 +------ .../parallel/test-http-response-setheaders.js | 69 +++++++++++++++++++ 3 files changed, 98 insertions(+), 22 deletions(-) create mode 100644 test/parallel/test-http-response-setheaders.js diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 8c80eabaec9e74..5ce90de6edfd7b 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -673,6 +673,34 @@ OutgoingMessage.prototype.setHeader = function setHeader(name, value) { return this; }; +OutgoingMessage.prototype.setHeaders = function setHeaders(obj) { + if (this._header) { + throw new ERR_HTTP_HEADERS_SENT('set'); + } + + let k; + if (ArrayIsArray(obj)) { + if (obj.length % 2 !== 0) { + throw new ERR_INVALID_ARG_VALUE('headers', obj); + } + + for (let n = 0; n < obj.length; n += 2) { + k = obj[n + 0]; + if (k) this.setHeader(k, obj[n + 1]); + } + } else if (obj) { + const keys = ObjectKeys(obj); + // Retain for(;;) loop for performance reasons + // Refs: https://github.com/nodejs/node/pull/30958 + for (let i = 0; i < keys.length; i++) { + k = keys[i]; + if (k) this.setHeader(k, obj[k]); + } + } + + return this; +}; + OutgoingMessage.prototype.appendHeader = function appendHeader(name, value) { if (this._header) { throw new ERR_HTTP_HEADERS_SENT('append'); diff --git a/lib/_http_server.js b/lib/_http_server.js index 62a26cdbaccd71..87d4c3fa6c9180 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -359,28 +359,7 @@ function writeHead(statusCode, reason, obj) { let headers; if (this[kOutHeaders]) { // Slow-case: when progressive API and header fields are passed. - let k; - if (ArrayIsArray(obj)) { - if (obj.length % 2 !== 0) { - throw new ERR_INVALID_ARG_VALUE('headers', obj); - } - - for (let n = 0; n < obj.length; n += 2) { - k = obj[n + 0]; - if (k) this.setHeader(k, obj[n + 1]); - } - } else if (obj) { - const keys = ObjectKeys(obj); - // Retain for(;;) loop for performance reasons - // Refs: https://github.com/nodejs/node/pull/30958 - for (let i = 0; i < keys.length; i++) { - k = keys[i]; - if (k) this.setHeader(k, obj[k]); - } - } - if (k === undefined && this._header) { - throw new ERR_HTTP_HEADERS_SENT('render'); - } + this.setHeaders(obj); // Only progressive api is used headers = this[kOutHeaders]; } else { diff --git a/test/parallel/test-http-response-setheaders.js b/test/parallel/test-http-response-setheaders.js new file mode 100644 index 00000000000000..533e128d8b1d6b --- /dev/null +++ b/test/parallel/test-http-response-setheaders.js @@ -0,0 +1,69 @@ +'use strict'; +const common = require('../common'); +const http = require('http'); +const assert = require('assert'); + + +{ + const server = http.createServer({ requireHostHeader: false }, common.mustCall((req, res) => { + res.setHeaders(['foo', '1', 'bar', '2' ]); + res.writeHead(200); + res.end(); + })); + + server.listen(0, common.mustCall(() => { + http.get({ port: server.address().port, headers: [] }, (res) => { + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers.foo, '1'); + assert.strictEqual(res.headers.bar, '2'); + res.resume().on('end', common.mustCall(() => { + server.close(); + })); + }); + })); +} + +{ + const server = http.createServer({ requireHostHeader: false }, common.mustCall((req, res) => { + res.setHeaders({ + foo: '1', + bar: '2' + }); + res.writeHead(200); + res.end(); + })); + + server.listen(0, common.mustCall(() => { + http.get({ port: server.address().port }, (res) => { + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers.foo, '1'); + assert.strictEqual(res.headers.bar, '2'); + res.resume().on('end', common.mustCall(() => { + server.close(); + })); + }); + })); +} + +{ + const server = http.createServer({ requireHostHeader: false }, common.mustCall((req, res) => { + res.writeHead(200); // headers already sent + assert.throws(() => { + res.setHeaders({ + foo: 'bar', + }); + }, { + code: 'ERR_HTTP_HEADERS_SENT' + }); + res.end(); + })); + + server.listen(0, common.mustCall(() => { + http.get({ port: server.address().port }, (res) => { + assert.strictEqual(res.headers.foo, undefined) + res.resume().on('end', common.mustCall(() => { + server.close(); + })); + }); + })); +} From e642c5736ef587d7e57ffacf904facca564e2955 Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Thu, 5 Jan 2023 18:16:33 +0100 Subject: [PATCH 02/12] http: setheaders documentation --- doc/api/http.md | 46 +++++++++++++++++++ lib/_http_server.js | 2 - .../parallel/test-http-response-setheaders.js | 6 +-- 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/doc/api/http.md b/doc/api/http.md index 553d611f846291..d1e93aa7180cad 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -1953,6 +1953,8 @@ non-string values. However, the non-string values will be converted to strings for network transmission. The same response object is returned to the caller, to enable call chaining. +> To set multiple header values at once see [`response.setHeaders()`][]. + ```js response.setHeader('Content-Type', 'text/html'); ``` @@ -1987,6 +1989,49 @@ header will not yield the expected result. If progressive population of headers is desired with potential future retrieval and modification, use [`response.setHeader()`][] instead of [`response.writeHead()`][]. +### `response.setHeaders(headers)` + + + +* `headers` {Object|Array} +* Returns: {http.ServerResponse} + +Returns the response object. + +Sets multiple header values for implicit headers. +`headers` may be an `Array` where the keys and values are in the same list. +It is _not_ a list of tuples. So, the even-numbered offsets are key values, +and the odd-numbered offsets are the associated values. The array is in the same +format as `request.rawHeaders`. + +```js +response.setHeaders(['foo', 'bar', 'fizz', 'buzz']); +``` + +or + +```js +response.setHeaders({ + foo: 'bar', + fizz: 'buzz', +}); +``` + +When headers have been set with [`response.setHeaders()`][], they will be merged +with any headers passed to [`response.writeHead()`][], with the headers passed +to [`response.writeHead()`][] given precedence. + +```js +// Returns content-type = text/plain +const server = http.createServer((req, res) => { + res.setHeaders(['Content-Type', 'text/html', 'X-Foo', 'bar']); + res.writeHead(200, { 'Content-Type': 'text/plain' }); + res.end('ok'); +}); +``` + ### `response.setTimeout(msecs[, callback])` -* `headers` {Object|Array} +* `headers` {Headers|Object|Array} * Returns: {http.ServerResponse} Returns the response object. @@ -2006,6 +2006,13 @@ It is _not_ a list of tuples. So, the even-numbered offsets are key values, and the odd-numbered offsets are the associated values. The array is in the same format as `request.rawHeaders`. +```js +const headers = new Headers({ foo: 'bar' }); +response.setHeaders(headers); +``` + +or + ```js response.setHeaders(['foo', 'bar', 'fizz', 'buzz']); ``` diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 9eb356afc69b9f..e7e17fbb31da3e 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -82,6 +82,8 @@ let debug = require('internal/util/debuglog').debuglog('http', (fn) => { debug = fn; }); +const { Headers } = require('internal/deps/undici/undici'); + const HIGH_WATER_MARK = getDefaultHighWaterMark(); const kCorked = Symbol('corked'); @@ -673,28 +675,34 @@ OutgoingMessage.prototype.setHeader = function setHeader(name, value) { return this; }; -OutgoingMessage.prototype.setHeaders = function setHeaders(obj) { +OutgoingMessage.prototype.setHeaders = function setHeaders(headers) { if (this._header) { throw new ERR_HTTP_HEADERS_SENT('set'); } - let k; - if (ArrayIsArray(obj)) { - if (obj.length % 2 !== 0) { - throw new ERR_INVALID_ARG_VALUE('headers', obj); + + if (headers instanceof Headers) { + const keys = [...headers.keys()]; + for (let i = 0; i < keys.length; i++) { + k = keys[i]; + if (k) this.setHeader(k, headers.get(k)); + } + } else if (ArrayIsArray(headers)) { + if (headers.length % 2 !== 0) { + throw new ERR_INVALID_ARG_VALUE('headers', headers); } - for (let n = 0; n < obj.length; n += 2) { - k = obj[n + 0]; - if (k) this.setHeader(k, obj[n + 1]); + for (let n = 0; n < headers.length; n += 2) { + k = headers[n + 0]; + if (k) this.setHeader(k, headers[n + 1]); } - } else if (obj) { - const keys = ObjectKeys(obj); + } else if (headers) { + const keys = ObjectKeys(headers); // Retain for(;;) loop for performance reasons // Refs: https://github.com/nodejs/node/pull/30958 for (let i = 0; i < keys.length; i++) { k = keys[i]; - if (k) this.setHeader(k, obj[k]); + if (k) this.setHeader(k, headers[k]); } } diff --git a/test/parallel/test-http-response-setheaders.js b/test/parallel/test-http-response-setheaders.js index 5b655b960b52d1..a3af552e69a451 100644 --- a/test/parallel/test-http-response-setheaders.js +++ b/test/parallel/test-http-response-setheaders.js @@ -67,3 +67,23 @@ const assert = require('assert'); }); })); } + +{ + const server = http.createServer({ requireHostHeader: false }, common.mustCall((req, res) => { + const headers = new globalThis.Headers({ foo: '1', bar: '2' }); + res.setHeaders(headers); + res.writeHead(200); + res.end(); + })); + + server.listen(0, common.mustCall(() => { + http.get({ port: server.address().port }, (res) => { + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers.foo, '1'); + assert.strictEqual(res.headers.bar, '2'); + res.resume().on('end', common.mustCall(() => { + server.close(); + })); + }); + })); +} From 0620d8c70a13ffc95203edaa109f76504802c9ef Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Thu, 5 Jan 2023 22:05:55 +0100 Subject: [PATCH 05/12] http: writeHead accepts headers --- doc/api/http.md | 19 ++++++++++++-- lib/_http_server.js | 4 +++ test/parallel/test-http-write-head-2.js | 35 +++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/doc/api/http.md b/doc/api/http.md index 616ca5e5f679cb..7bc58f1787a038 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -2001,7 +2001,8 @@ added: REPLACEME Returns the response object. Sets multiple header values for implicit headers. -`headers` may be an `Array` where the keys and values are in the same list. +`headers` may be an instance of [`Headers`][] or `Array` where the keys +and values are in the same list. It is _not_ a list of tuples. So, the even-numbered offsets are key values, and the odd-numbered offsets are the associated values. The array is in the same format as `request.rawHeaders`. @@ -2255,6 +2256,10 @@ response.writeEarlyHints({ -* `headers` {Headers|Object|Array} +* `headers` {Headers} * Returns: {http.ServerResponse} Returns the response object. Sets multiple header values for implicit headers. -`headers` may be an instance of [`Headers`][] or `Array` where the keys -and values are in the same list. -It is _not_ a list of tuples. So, the even-numbered offsets are key values, -and the odd-numbered offsets are the associated values. The array is in the same -format as `request.rawHeaders`. +`headers` must be an instance of [`Headers`][], if header already exists +in the to-be-sent headers, its value will be replaced. ```js const headers = new Headers({ foo: 'bar' }); response.setHeaders(headers); ``` -or - -```js -response.setHeaders(['foo', 'bar', 'fizz', 'buzz']); -``` - -or - -```js -response.setHeaders({ - foo: 'bar', - fizz: 'buzz', -}); -``` - When headers have been set with [`response.setHeaders()`][], they will be merged with any headers passed to [`response.writeHead()`][], with the headers passed to [`response.writeHead()`][] given precedence. @@ -2034,7 +2016,8 @@ to [`response.writeHead()`][] given precedence. ```js // Returns content-type = text/plain const server = http.createServer((req, res) => { - res.setHeaders(['Content-Type', 'text/html', 'X-Foo', 'bar']); + const headers = new Headers({ 'Content-Type': 'text/html' }); + res.setHeaders(headers); res.writeHead(200, { 'Content-Type': 'text/plain' }); res.end('ok'); }); @@ -2256,10 +2239,6 @@ response.writeEarlyHints({ - -* `headers` {Headers} -* Returns: {http.ServerResponse} - -Returns the response object. - -Sets multiple header values for implicit headers. -`headers` must be an instance of [`Headers`][], if header already exists -in the to-be-sent headers, its value will be replaced. - -```js -const headers = new Headers({ foo: 'bar' }); -response.setHeaders(headers); -``` - -When headers have been set with [`response.setHeaders()`][], they will be merged -with any headers passed to [`response.writeHead()`][], with the headers passed -to [`response.writeHead()`][] given precedence. - -```js -// Returns content-type = text/plain -const server = http.createServer((req, res) => { - const headers = new Headers({ 'Content-Type': 'text/html' }); - res.setHeaders(headers); - res.writeHead(200, { 'Content-Type': 'text/plain' }); - res.end('ok'); -}); -``` - ### `response.setTimeout(msecs[, callback])` + +* `headers` {Headers|Map} +* Returns: {http.ServerResponse} + +Returns the response object. + +Sets multiple header values for implicit headers. +`headers` must be an instance of [`Headers`][], if header already exists +in the to-be-sent headers, its value will be replaced. + +```js +const headers = new Headers({ foo: 'bar' }); +response.setHeaders(headers); +``` + +or + +```js +const headers = new Map([['foo', 'bar']]); +res.setHeaders(headers); +``` + +When headers have been set with [`outgoingMessage.setHeaders()`][], +they will be merged with any headers passed to [`response.writeHead()`][], +with the headers passed to [`response.writeHead()`][] given precedence. + +```js +// Returns content-type = text/plain +const server = http.createServer((req, res) => { + const headers = new Headers({ 'Content-Type': 'text/html' }); + res.setHeaders(headers); + res.writeHead(200, { 'Content-Type': 'text/plain' }); + res.end('ok'); +}); +``` + ### `outgoingMessage.setTimeout(msesc[, callback])`