fixup! http: add optimizeEmptyRequests server option

Author: RafaelGSS

doc/api/http.md

@@ -3638,7 +3638,8 @@ changes:
   * `optimizeEmptyRequests` {boolean} If set to `true`, requests without `Content-Length`
     or `Transfer-Encoding` headers (indicating no body) will be initialized with an
     already-ended body stream, so they will never emit any stream events
-    (like `data` or `end`). You can use `req.readableEnded` to detect this case.
+    (like `'data'` or `'end'`). You can use `req.readableEnded` to detect this case.
+    This option is still under experimental phase.
     **Default:** `false`.
 
 * `requestListener` {Function}

lib/_http_server.js

@@ -1058,6 +1058,10 @@ function emitCloseNT(self) {
   }
 }
 
+function hasBodyHeaders(headers) {
+  return ('content-length' in headers) || ('transfer-encoding' in headers);
+}
+
 // The following callback is issued after the headers have been read on a
 // new message. In this callback we setup the response object and pass it
 // to the user.
@@ -1110,11 +1114,11 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
   }
 
   // Check if we should optimize empty requests (those without Content-Length or Transfer-Encoding headers)
-  const hasBodyHeaders = ('content-length' in req.headers) || ('transfer-encoding' in req.headers);
-  const shouldOptimize = server[kOptimizeEmptyRequests] === true && !hasBodyHeaders;
+  const shouldOptimize = server[kOptimizeEmptyRequests] === true && !hasBodyHeaders(req.headers);
 
   if (shouldOptimize) {
-    // Fast processing where request "has" already emitted all lifecycle events.
+    // Fast processing where emitting 'data', 'end' and 'close' events is
+    // skipped and data is dumped.
     // This avoids a lot of unnecessary overhead otherwise introduced by
     // stream.Readable life cycle rules. The downside is that this will
     // break some servers that read bodies for methods that don't have body headers.

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

@@ -124,7 +124,7 @@ const assert = require('assert');
     }));
 
     sock.end('' +
-      'PUT / HTTP/1.1\r\n' +
+      'GET / 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-server-optimize-empty-requests.js

@@ -6,18 +6,20 @@ const http = require('http');
 const net = require('net');
 
 let reqs = 0;
+let optimizedReqs = 0;
 const server = http.createServer({
   optimizeEmptyRequests: true
 }, (req, res) => {
   reqs++;
-  req.on('data', common.mustNotCall());
-  req.on('end', common.mustNotCall());
-
-  assert.strictEqual(req._dumped, true);
-  assert.strictEqual(req._readableState.ended, true);
-  assert.strictEqual(req._readableState.endEmitted, true);
-  assert.strictEqual(req._readableState.destroyed, true);
+  if (req._dumped) {
+    optimizedReqs++;
+    req.on('data', common.mustNotCall());
+    req.on('end', common.mustNotCall());
 
+    assert.strictEqual(req._dumped, true);
+    assert.strictEqual(req.readableEnded, true);
+    assert.strictEqual(req.destroyed, true);
+  }
   res.writeHead(200);
   res.end('ok');
 });
@@ -38,9 +40,27 @@ server.listen(0, common.mustCall(async () => {
   // DELETE request without body headers (should be optimized)
   const deleteWithoutBodyHeaders = 'DELETE / HTTP/1.1\r\nHost: localhost\r\n\r\n';
   await makeRequest(deleteWithoutBodyHeaders);
+
+  // POST request with Content-Length header (should not be optimized)
+  const postWithContentLength = 'POST / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 0\r\n\r\n';
+  await makeRequest(postWithContentLength);
+
+  // GET request with Content-Length header (should not be optimized)
+  const getWithContentLength = 'GET / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 0\r\n\r\n';
+  await makeRequest(getWithContentLength);
+
+  // POST request with Transfer-Encoding header (should not be optimized)
+  const postWithTransferEncoding = 'POST / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked\r\n\r\n';
+  await makeRequest(postWithTransferEncoding);
+
+  // GET request with Transfer-Encoding header (should not be optimized)
+  const getWithTransferEncoding = 'GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked\r\n\r\n';
+  await makeRequest(getWithTransferEncoding);
+
   server.close();
 
-  assert.strictEqual(reqs, 4);
+  assert.strictEqual(reqs, 8, `Expected 8 requests but got ${reqs}`);
+  assert.strictEqual(optimizedReqs, 4, `Expected 4 optimized requests but got ${optimizedReqs}`);
 }));
 
 function makeRequest(str) {

test/parallel/test-http.js

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