Repository: nodejs/node
Pull request: http: add setDefaultHeaders option to http.request

When sending an HTTP request, for some use cases you want _full_ control of the request headers: controlling the raw format directly and setting only the headers provided, with no other headers automatically included whatsoever.

That is not currently possible, because:

* The only way to disable various default headers like Connection & Transfer-Encoding is to call `req.removeHeader` for each of them individually on a request instance, before the headers are sent. Otherwise these are set automatically, even if you pass a raw header array.
* The only way to set raw headers (i.e. passing a header array instead of an object, to directly control order, casing & duplicate handling etc) is by passing them upfront in the `http.request()` method, and when you do so the headers are passed to `_storeHeader` and serialized into `_header` immediately, meaning you can't call `removeHeader` afterwards.

That means these two use cases (removing default headers & setting raw header formats) are mutually exclusive, which isn't great since they're closely related.

This PR fixes that by adding a `setDefaultHeaders` option to `http.request`, defaulting to `true`. If you pass `setDefaultHeaders: false` then you take responsibility for providing all required headers and Node gets out of the way and gives you full control.

With this, you can now control all headers, in raw format, with:

```js
http.request({
  /* ... */
  setDefaultHeaders: false,
  headers: [/* raw header array */]
})
```

Alternative ideas I've avoided, but might be interesting:
* Adding separate `setContentLength`, `setTransferEncoding` etc parameters, analogous to the existing `setHost` option. Results in too many options, and I think in the case where this is necessary you almost always want to control _all_ headers. In fact, I think if this PR is merged we could consider deprecating `setHost` entirely, and suggesting users migrate to this option instead (I expect most `setHost: false` cases will want this too) but I haven't looked into that for now, we can wait and see how this is used instead.
* Extending `setHeaders` to allow setting raw headers _after_ initial request construction, to handle this use case with just `removeHeader` + `setHeaders` calls. This would require that either `setHeaders` immediately serialize the raw headers to `_headers` in this case (blocking any further changes like an implicit flushHeaders, but only in the raw header argument case, with potentially weird differences between setHeaders behaviour for requests vs responses), or some fairly major refactoring to change all internal HTTP header representation to support incrementally adding raw headers (all current raw header APIs - `writeHead` and `http.request` - always serialize & fix the headers immediately). The former is quite an awkward API, the latter is messy and risky, and I'd rather not.
* Adding a new `request.writeHeaders` method analogous to `response.writeHead`. This works, but it's yet another headers API with subtly different behaviour that would really only exist for this one use case.
* Removing implicit headers automatically if you provide a full set of raw headers in `http.request` - this seems sensible to me as an API, but it's a huge breaking change that isn't really practical imo.

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: doc/api/http.md
patch: @@ -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`

filename: lib/_http_client.js
patch: @@ -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;
 

filename: test/parallel/test-http-dont-set-default-headers-with-set-header.js
patch: @@ -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, common.localhostIPv4, function() {
+  const req = http.request({
+    method: 'POST',
+    host: common.localhostIPv4,
+    port: this.address().port,
+    setDefaultHeaders: false,
+  });
+
+  req.setHeader('test', 'value');
+  req.setHeader('HOST', `${common.localhostIPv4}:${server.address().port}`);
+  req.setHeader('foo', ['bar', 'baz']);
+  req.setHeader('connection', 'close');
+
+  req.end();
+});

filename: test/parallel/test-http-dont-set-default-headers-with-setHost.js
patch: @@ -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', `${common.localhostIPv4}:${server.address().port}`,
+  ]);
+
+  res.end('ok');
+  server.close();
+}));
+server.listen(0, common.localhostIPv4, function() {
+  http.request({
+    method: 'POST',
+    host: common.localhostIPv4,
+    port: this.address().port,
+    setDefaultHeaders: false,
+    setHost: true
+  }).end();
+});

filename: test/parallel/test-http-dont-set-default-headers.js
patch: @@ -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', `${common.localhostIPv4}:${server.address().port}`,
+    'foo', 'bar',
+    'test', 'value',
+    'foo', 'baz',
+  ]);
+
+  res.end('ok');
+  server.close();
+}));
+server.listen(0, common.localhostIPv4, function() {
+  http.request({
+    method: 'POST',
+    host: common.localhostIPv4,
+    port: this.address().port,
+    setDefaultHeaders: false,
+    headers: [
+      'host', `${common.localhostIPv4}:${server.address().port}`,
+      'foo', 'bar',
+      'test', 'value',
+      'foo', 'baz',
+    ]
+  }).end();
+});