From 950b50b9b14f860ced7ec8f60ba1e16fb933f08d Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Mon, 2 Dec 2024 16:25:12 +0100 Subject: [PATCH 1/2] http: add setDefaultHeaders option to http.request This makes it possible to disable the various default headers directly from the constructor. While this is possible for many use cases by manually calling removeHeader on the request object instead, when passing a raw header array to the request constructor the headers are serialized and prepared to send immediately, and removeHeader cannot subsequently be used. With this change, it's now possible to 100% control sent request headers by passing 'setDefaultHeaders: false' and a raw headers array to http.request. --- doc/api/http.md | 7 +++- lib/_http_client.js | 8 ++++- ...ont-set-default-headers-with-set-header.js | 33 +++++++++++++++++++ ...p-dont-set-default-headers-with-setHost.js | 23 +++++++++++++ .../test-http-dont-set-default-headers.js | 31 +++++++++++++++++ 5 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-http-dont-set-default-headers-with-set-header.js create mode 100644 test/parallel/test-http-dont-set-default-headers-with-setHost.js create mode 100644 test/parallel/test-http-dont-set-default-headers.js diff --git a/doc/api/http.md b/doc/api/http.md index 9b9175a003f56c..886bbe26ecc95c 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -3848,8 +3848,13 @@ changes: * `port` {number} Port of remote server. **Default:** `defaultPort` if set, else `80`. * `protocol` {string} Protocol to use. **Default:** `'http:'`. + * `setDefaultHeaders` {boolean}: Specifies whether or not to automatically add + default headers such as `Connection`, `Content-Length`, `Transfer-Encoding`, + and `Host`. If set to `false` then all necessary headers must be added + manually. Defaults to `true`. * `setHost` {boolean}: Specifies whether or not to automatically add the - `Host` header. Defaults to `true`. + `Host` header. If provided, this overrides `setDefaultHeaders`. Defaults to + `true`. * `signal` {AbortSignal}: An AbortSignal that may be used to abort an ongoing request. * `socketPath` {string} Unix domain socket. Cannot be used if one of `host` diff --git a/lib/_http_client.js b/lib/_http_client.js index 91ba264339fa4f..00b59f357fa45d 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -199,7 +199,13 @@ function ClientRequest(input, options, cb) { const host = optsWithoutSignal.host = validateHost(options.hostname, 'hostname') || validateHost(options.host, 'host') || 'localhost'; - const setHost = (options.setHost === undefined || Boolean(options.setHost)); + const setHost = options.setHost !== undefined ? + Boolean(options.setHost) : + options.setDefaultHeaders !== false; + + this._removedConnection = options.setDefaultHeaders === false; + this._removedContLen = options.setDefaultHeaders === false; + this._removedTE = options.setDefaultHeaders === false; this.socketPath = options.socketPath; diff --git a/test/parallel/test-http-dont-set-default-headers-with-set-header.js b/test/parallel/test-http-dont-set-default-headers-with-set-header.js new file mode 100644 index 00000000000000..4b9fd3f7cae39c --- /dev/null +++ b/test/parallel/test-http-dont-set-default-headers-with-set-header.js @@ -0,0 +1,33 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +const server = http.createServer(common.mustCall(function(req, res) { + assert.deepStrictEqual(req.rawHeaders, [ + 'test', 'value', + 'HOST', `127.0.0.1:${server.address().port}`, + 'foo', 'bar', + 'foo', 'baz', + 'connection', 'close', + ]); + + res.end('ok'); + server.close(); +})); +server.listen(0, '127.0.0.1', function() { + const req = http.request({ + method: 'POST', + host: '127.0.0.1', + port: this.address().port, + setDefaultHeaders: false, + }); + + req.setHeader('test', 'value'); + req.setHeader('HOST', `127.0.0.1:${server.address().port}`); + req.setHeader('foo', ['bar', 'baz']); + req.setHeader('connection', 'close'); + + req.end(); +}); diff --git a/test/parallel/test-http-dont-set-default-headers-with-setHost.js b/test/parallel/test-http-dont-set-default-headers-with-setHost.js new file mode 100644 index 00000000000000..5ee4f75fcf5ac3 --- /dev/null +++ b/test/parallel/test-http-dont-set-default-headers-with-setHost.js @@ -0,0 +1,23 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +const server = http.createServer(common.mustCall(function(req, res) { + assert.deepStrictEqual(req.rawHeaders, [ + 'Host', `127.0.0.1:${server.address().port}`, + ]); + + res.end('ok'); + server.close(); +})); +server.listen(0, '127.0.0.1', function() { + http.request({ + method: 'POST', + host: '127.0.0.1', + port: this.address().port, + setDefaultHeaders: false, + setHost: true + }).end(); +}); diff --git a/test/parallel/test-http-dont-set-default-headers.js b/test/parallel/test-http-dont-set-default-headers.js new file mode 100644 index 00000000000000..35d93a2de92ed5 --- /dev/null +++ b/test/parallel/test-http-dont-set-default-headers.js @@ -0,0 +1,31 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +const server = http.createServer(common.mustCall(function(req, res) { + assert.deepStrictEqual(req.rawHeaders, [ + 'host', `127.0.0.1:${server.address().port}`, + 'foo', 'bar', + 'test', 'value', + 'foo', 'baz', + ]); + + res.end('ok'); + server.close(); +})); +server.listen(0, '127.0.0.1', function() { + http.request({ + method: 'POST', + host: '127.0.0.1', + port: this.address().port, + setDefaultHeaders: false, + headers: [ + 'host', `127.0.0.1:${server.address().port}`, + 'foo', 'bar', + 'test', 'value', + 'foo', 'baz', + ] + }).end(); +}); From ddfdc0f2d59f3e2c10d75e802eb8b4d42114755a Mon Sep 17 00:00:00 2001 From: Tim Perry <1526883+pimterry@users.noreply.github.com> Date: Tue, 10 Dec 2024 12:48:44 +0100 Subject: [PATCH 2/2] Update tests to use common.localhostIPv4 Suggestions from review Co-authored-by: Luigi Pinca --- .../test-http-dont-set-default-headers-with-set-header.js | 6 +++--- .../test-http-dont-set-default-headers-with-setHost.js | 6 +++--- test/parallel/test-http-dont-set-default-headers.js | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/test/parallel/test-http-dont-set-default-headers-with-set-header.js b/test/parallel/test-http-dont-set-default-headers-with-set-header.js index 4b9fd3f7cae39c..bafdae5571e33c 100644 --- a/test/parallel/test-http-dont-set-default-headers-with-set-header.js +++ b/test/parallel/test-http-dont-set-default-headers-with-set-header.js @@ -16,16 +16,16 @@ const server = http.createServer(common.mustCall(function(req, res) { res.end('ok'); server.close(); })); -server.listen(0, '127.0.0.1', function() { +server.listen(0, common.localhostIPv4, function() { const req = http.request({ method: 'POST', - host: '127.0.0.1', + host: common.localhostIPv4, port: this.address().port, setDefaultHeaders: false, }); req.setHeader('test', 'value'); - req.setHeader('HOST', `127.0.0.1:${server.address().port}`); + req.setHeader('HOST', `${common.localhostIPv4}:${server.address().port}`); req.setHeader('foo', ['bar', 'baz']); req.setHeader('connection', 'close'); diff --git a/test/parallel/test-http-dont-set-default-headers-with-setHost.js b/test/parallel/test-http-dont-set-default-headers-with-setHost.js index 5ee4f75fcf5ac3..e2a4e39c24b837 100644 --- a/test/parallel/test-http-dont-set-default-headers-with-setHost.js +++ b/test/parallel/test-http-dont-set-default-headers-with-setHost.js @@ -6,16 +6,16 @@ const http = require('http'); const server = http.createServer(common.mustCall(function(req, res) { assert.deepStrictEqual(req.rawHeaders, [ - 'Host', `127.0.0.1:${server.address().port}`, + 'Host', `${common.localhostIPv4}:${server.address().port}`, ]); res.end('ok'); server.close(); })); -server.listen(0, '127.0.0.1', function() { +server.listen(0, common.localhostIPv4, function() { http.request({ method: 'POST', - host: '127.0.0.1', + host: common.localhostIPv4, port: this.address().port, setDefaultHeaders: false, setHost: true diff --git a/test/parallel/test-http-dont-set-default-headers.js b/test/parallel/test-http-dont-set-default-headers.js index 35d93a2de92ed5..3f73c11e5112ee 100644 --- a/test/parallel/test-http-dont-set-default-headers.js +++ b/test/parallel/test-http-dont-set-default-headers.js @@ -6,7 +6,7 @@ const http = require('http'); const server = http.createServer(common.mustCall(function(req, res) { assert.deepStrictEqual(req.rawHeaders, [ - 'host', `127.0.0.1:${server.address().port}`, + 'host', `${common.localhostIPv4}:${server.address().port}`, 'foo', 'bar', 'test', 'value', 'foo', 'baz', @@ -15,14 +15,14 @@ const server = http.createServer(common.mustCall(function(req, res) { res.end('ok'); server.close(); })); -server.listen(0, '127.0.0.1', function() { +server.listen(0, common.localhostIPv4, function() { http.request({ method: 'POST', - host: '127.0.0.1', + host: common.localhostIPv4, port: this.address().port, setDefaultHeaders: false, headers: [ - 'host', `127.0.0.1:${server.address().port}`, + 'host', `${common.localhostIPv4}:${server.address().port}`, 'foo', 'bar', 'test', 'value', 'foo', 'baz',