From eac26c0793b431eb24f5c67e255f4616370017ab Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 21 Nov 2022 14:36:29 -0800 Subject: [PATCH] Revert "http: headers(Distinct), trailers(Distinct) setters to be no-op" This reverts commit 4d723c7fd6f8b2ecd0dbc1d6f0595645f6471928. I'm not sure if we should re-apply this as a semver-major change or if we should accept it as valid and add tests/documentation, but either way, we have to revert it at least temporarily. Closes: https://github.com/nodejs/node/issues/45510 PR-URL: https://github.com/nodejs/node/pull/45527 Fixes: https://github.com/nodejs/node/issues/45510 Reviewed-By: Richard Lau Reviewed-By: Antoine du Hamel Reviewed-By: Luigi Pinca Reviewed-By: Filip Skokan Reviewed-By: Matteo Collina Reviewed-By: Robert Nagy --- lib/_http_incoming.js | 16 ++++-- .../test-http-set-headers-distinct.js | 54 ------------------- .../test-set-incoming-message-header.js | 11 ++-- 3 files changed, 18 insertions(+), 63 deletions(-) delete mode 100644 test/parallel/test-http-set-headers-distinct.js diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js index a83c8660db589b..950c8c6e08bbc9 100644 --- a/lib/_http_incoming.js +++ b/lib/_http_incoming.js @@ -122,7 +122,9 @@ ObjectDefineProperty(IncomingMessage.prototype, 'headers', { } return this[kHeaders]; }, - set: function(val) {} + set: function(val) { + this[kHeaders] = val; + } }); ObjectDefineProperty(IncomingMessage.prototype, 'headersDistinct', { @@ -140,7 +142,9 @@ ObjectDefineProperty(IncomingMessage.prototype, 'headersDistinct', { } return this[kHeadersDistinct]; }, - set: function(val) {} + set: function(val) { + this[kHeadersDistinct] = val; + } }); ObjectDefineProperty(IncomingMessage.prototype, 'trailers', { @@ -158,7 +162,9 @@ ObjectDefineProperty(IncomingMessage.prototype, 'trailers', { } return this[kTrailers]; }, - set: function(val) {} + set: function(val) { + this[kTrailers] = val; + } }); ObjectDefineProperty(IncomingMessage.prototype, 'trailersDistinct', { @@ -176,7 +182,9 @@ ObjectDefineProperty(IncomingMessage.prototype, 'trailersDistinct', { } return this[kTrailersDistinct]; }, - set: function(val) {} + set: function(val) { + this[kTrailersDistinct] = val; + } }); IncomingMessage.prototype.setTimeout = function setTimeout(msecs, callback) { diff --git a/test/parallel/test-http-set-headers-distinct.js b/test/parallel/test-http-set-headers-distinct.js deleted file mode 100644 index 3a3c99cdcc5f00..00000000000000 --- a/test/parallel/test-http-set-headers-distinct.js +++ /dev/null @@ -1,54 +0,0 @@ -'use strict'; - -const common = require('../common'); -const assert = require('assert'); -const { createServer, request } = require('http'); - -const server = createServer( - common.mustCall((req, res) => { - req.headersDistinct = { 'x-req-a': ['zzz'] }; - - // headersDistinct setter should be a No-Op - assert.deepStrictEqual(req.headersDistinct, { - 'transfer-encoding': [ - 'chunked', - ], - 'connection': [ - 'keep-alive', - ], - 'host': [ - `127.0.0.1:${server.address().port}`, - ] - }); - - req.on('end', function() { - res.write('BODY'); - res.end(); - }); - - req.resume(); - }) -); - -server.listen( - 0, - common.mustCall(() => { - const req = request( - { - host: '127.0.0.1', - port: server.address().port, - path: '/', - method: 'POST', - }, - common.mustCall((res) => { - res.on('end', function() { - server.close(); - }); - res.resume(); - }) - ); - - req.write('BODY'); - req.end(); - }) -); diff --git a/test/parallel/test-set-incoming-message-header.js b/test/parallel/test-set-incoming-message-header.js index 1fab6cd1e5a724..9ac05a8138d445 100644 --- a/test/parallel/test-set-incoming-message-header.js +++ b/test/parallel/test-set-incoming-message-header.js @@ -4,23 +4,24 @@ require('../common'); const { IncomingMessage } = require('http'); const assert = require('assert'); -// Headers setter should be a No-Op +// Headers setter function set a header correctly { const im = new IncomingMessage(); im.headers = { key: 'value' }; - assert.deepStrictEqual(im.headers, {}); + assert.deepStrictEqual(im.headers, { key: 'value' }); } -// Trailers setter should be a No-Op +// Trailers setter function set a header correctly { const im = new IncomingMessage(); im.trailers = { key: 'value' }; - assert.deepStrictEqual(im.trailers, {}); + assert.deepStrictEqual(im.trailers, { key: 'value' }); } // _addHeaderLines function set a header correctly { const im = new IncomingMessage(); + im.headers = { key1: 'value1' }; im._addHeaderLines(['key2', 'value2'], 2); - assert.deepStrictEqual(im.headers, { key2: 'value2' }); + assert.deepStrictEqual(im.headers, { key1: 'value1', key2: 'value2' }); }