http: optimize IncomingMessage._dump

Author: ronag

doc/api/http.md

@@ -3537,6 +3537,9 @@ Found'`.
 <!-- YAML
 added: v0.1.13
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/59747
+    description: Add dumpGET option.
   - version:
     - v20.1.0
     - v18.17.0
@@ -3632,6 +3635,10 @@ changes:
   * `rejectNonStandardBodyWrites` {boolean} If set to `true`, an error is thrown
     when writing to an HTTP response which does not have a body.
     **Default:** `false`.
+  * `dumpGETBody` {boolean} If set to `true`, request body for `HEAD` and `GET`
+    requests will be immediatly dumped and the `end` and `close` events for
+    `IncomingMessage` will be emitted before the server emits `'request'`. This
+    enables some optimizations that would otherwise not be possible.
 
 * `requestListener` {Function}
 

lib/_http_server.js

@@ -23,6 +23,7 @@
 
 const {
   ArrayIsArray,
+  Boolean,
   Error,
   MathMin,
   NumberIsFinite,
@@ -32,6 +33,7 @@ const {
   Symbol,
   SymbolAsyncDispose,
   SymbolFor,
+  globalThis,
 } = primordials;
 
 const net = require('net');
@@ -105,6 +107,7 @@ const onResponseFinishChannel = dc.channel('http.server.response.finish');
 
 const kServerResponse = Symbol('ServerResponse');
 const kServerResponseStatistics = Symbol('ServerResponseStatistics');
+const kDumpGet = Symbol('kDumpGet');
 
 const {
   hasObserver,
@@ -447,6 +450,8 @@ function storeHTTPOptions(options) {
   this[kIncomingMessage] = options.IncomingMessage || IncomingMessage;
   this[kServerResponse] = options.ServerResponse || ServerResponse;
 
+  this[kDumpGet] = Boolean(options.dumpGETBody ?? globalThis.IS_NODE_TEST);
+
   const maxHeaderSize = options.maxHeaderSize;
   if (maxHeaderSize !== undefined)
     validateInteger(maxHeaderSize, 'maxHeaderSize', 0);
@@ -1102,6 +1107,24 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
     });
   }
 
+  if (req[kDumpGet]) {
+    if (req.method === 'HEAD' || req.method === 'GET') {
+      // Fast dump where request "has" already emitted all lifecycle events.
+      // This avoid a lot of unnecessary overhead otherwise introduced by
+      // stream.Readable life cycle rules. The downside is that this will
+      // break some servers that read GET bodies.
+
+      req._dumped = true;
+      req._readableState.ended = true;
+      req._readableState.endEmitted = true;
+      req._readableState.destroyed = true;
+      req._readableState.closed = true;
+      req._readableState.closeEmitted = true;
+
+      req._read();
+    }
+  }
+
   if (socket._httpMessage) {
     // There are already pending outgoing res, append.
     state.outgoing.push(res);

test/common/index.js

@@ -36,6 +36,8 @@ const bits = ['arm64', 'loong64', 'mips', 'mipsel', 'ppc64', 'riscv64', 's390x',
   .includes(process.arch) ? 64 : 32;
 const hasIntl = !!process.config.variables.v8_enable_i18n_support;
 
+globalThis.IS_NODE_TEST = true;
+
 const {
   atob,
   btoa,

test/parallel/test-http-chunk-extensions-limit.js

@@ -124,7 +124,7 @@ const assert = require('assert');
     }));
 
     sock.end('' +
-      'GET / HTTP/1.1\r\n' +
+      'PUT / HTTP/1.1\r\n' +
       `Host: localhost:${port}\r\n` +
       'Transfer-Encoding: chunked\r\n\r\n' +
       '2;' + 'A'.repeat(10000) + '=bar\r\nAA\r\n' +

test/parallel/test-http.js

@@ -52,12 +52,18 @@ const server = http.Server(common.mustCall((req, res) => {
   if (expectedRequests.length === 0)
     server.close();
 
-  req.on('end', () => {
+  if (req.readableEnded) {
     res.writeHead(200, { 'Content-Type': 'text/plain' });
     res.write(`The path was ${url.parse(req.url).pathname}`);
     res.end();
-  });
-  req.resume();
+  } else {
+    req.on('end', () => {
+      res.writeHead(200, { 'Content-Type': 'text/plain' });
+      res.write(`The path was ${url.parse(req.url).pathname}`);
+      res.end();
+    });
+    req.resume();
+  }
 }, 3));
 server.listen(0);