http: frame client requests that carry a payload

lib/_http_outgoing.js

@@ -93,6 +93,7 @@ const kEndCallbacks = Symbol('kEndCallbacks');
 const kFlushError = Symbol('kFlushError');
 const kHighWaterMark = Symbol('kHighWaterMark');
 const kRejectNonStandardBodyWrites = Symbol('kRejectNonStandardBodyWrites');
+const kPayloadPending = Symbol('kPayloadPending');
 
 const nop = () => {};
 
@@ -142,6 +143,7 @@ function OutgoingMessage(options) {
 
   this.strictContentLength = false;
   this[kBytesWritten] = 0;
+  this[kPayloadPending] = false;
   this._contentLength = null;
   this._hasBody = true;
   this._trailer = '';
@@ -542,7 +544,12 @@ function _storeHeader(firstLine, headers) {
     if (!this._hasBody) {
       // Make sure we don't end the 0\r\n\r\n at the end of the message.
       this.chunkedEncoding = false;
-    } else if (!this.useChunkedEncodingByDefault) {
+    } else if (!this.useChunkedEncodingByDefault &&
+               (!this.method || this.method === 'CONNECT' ||
+                !this[kPayloadPending])) {
+      // Responses, and requests that are known to carry no payload, are framed
+      // by the absence of both headers. CONNECT is excluded because its payload
+      // is tunnelled data that must not be re-framed.
       this._last = true;
     } else if (!state.trailer &&
                !this._removedContLen &&
@@ -992,6 +999,7 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
       len ??= typeof chunk === 'string' ? Buffer.byteLength(chunk, encoding) : chunk.byteLength;
       msg._contentLength = len;
     }
+    msg[kPayloadPending] = true;
     msg._implicitHeader();
   }
 

test/parallel/test-http-clientrequest-end-contentlength.js

@@ -0,0 +1,42 @@
+'use strict';
+
+// Test that ClientRequest#end computes and sends a Content-Length header,
+// regardless of the request method.
+// Refs: https://github.com/nodejs/node/issues/27880
+
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+const upload = 'PUT / HTTP/1.1\r\n\r\n';
+
+const methods = ['GET', 'HEAD', 'DELETE', 'POST', 'PATCH', 'PUT', 'OPTIONS'];
+
+const server = http.createServer(common.mustCall(function(req, res) {
+  const chunks = [];
+  req.on('data', (chunk) => chunks.push(chunk));
+  req.on('end', common.mustCall(() => {
+    assert.deepStrictEqual(Buffer.concat(chunks), Buffer.from(upload));
+    assert.strictEqual(req.headers['content-length'],
+                       String(upload.length));
+    assert.strictEqual(req.headers['transfer-encoding'], undefined);
+
+    // A HEAD response carries no body, so report the outcome in a header
+    // instead of writing it to the response stream.
+    res.setHeader('x-received-method', req.method);
+    res.end();
+  }));
+}, methods.length));
+
+server.listen(0, common.mustCall(function tryNextRequest() {
+  const method = methods.pop();
+  if (method === undefined) return;
+  const port = server.address().port;
+  const req = http.request({ method, port }, common.mustCall((res) => {
+    assert.strictEqual(res.headers['x-received-method'], method);
+    res.resume();
+    res.on('end', common.mustCall(tryNextRequest));
+  }));
+
+  req.end(upload);
+})).unref();

test/parallel/test-http-clientrequest-write-chunked.js

@@ -0,0 +1,42 @@
+'use strict';
+
+// Test that ClientRequest#write uses a chunked Transfer-Encoding when the
+// payload length is not known in advance, regardless of the request method.
+// Refs: https://github.com/nodejs/node/issues/27880
+
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+const upload = 'PUT / HTTP/1.1\r\n\r\n';
+
+const methods = ['GET', 'HEAD', 'DELETE', 'POST', 'PATCH', 'PUT', 'OPTIONS'];
+
+const server = http.createServer(common.mustCall(function(req, res) {
+  const chunks = [];
+  req.on('data', (chunk) => chunks.push(chunk));
+  req.on('end', common.mustCall(() => {
+    assert.deepStrictEqual(Buffer.concat(chunks), Buffer.from(upload));
+    assert.strictEqual(req.headers['transfer-encoding'], 'chunked');
+    assert.strictEqual(req.headers['content-length'], undefined);
+
+    // A HEAD response carries no body, so report the outcome in a header
+    // instead of writing it to the response stream.
+    res.setHeader('x-received-method', req.method);
+    res.end();
+  }));
+}, methods.length));
+
+server.listen(0, common.mustCall(function tryNextRequest() {
+  const method = methods.pop();
+  if (method === undefined) return;
+  const port = server.address().port;
+  const req = http.request({ method, port }, common.mustCall((res) => {
+    assert.strictEqual(res.headers['x-received-method'], method);
+    res.resume();
+    res.on('end', common.mustCall(tryNextRequest));
+  }));
+
+  req.write(upload);
+  req.end();
+})).unref();

test/parallel/test-http-request-method-delete-payload.js

@@ -1,26 +1,33 @@
 'use strict';
 const common = require('../common');
+const assert = require('assert');
 
 const http = require('http');
 
+// A DELETE request with a payload must be framed like any other request, so
+// that the payload is received as a body instead of being parsed as the start
+// of a following request.
+// Refs: https://github.com/nodejs/node/issues/27880
+
 const data = 'PUT / HTTP/1.1\r\n\r\n';
 
 const server = http.createServer(common.mustCall(function(req, res) {
-  req.on('data', common.mustNotCall());
-  res.setHeader('Content-Type', 'text/plain');
-  for (let i = 0; i < req.rawHeaders.length; i += 2) {
-    if (req.rawHeaders[i].toLowerCase() === 'host') continue;
-    if (req.rawHeaders[i].toLowerCase() === 'connection') continue;
-    res.write(`${req.rawHeaders[i]}: ${req.rawHeaders[i + 1]}\r\n`);
-  }
-  res.end();
+  const chunks = [];
+  req.on('data', (chunk) => chunks.push(chunk));
+  req.on('end', common.mustCall(() => {
+    assert.deepStrictEqual(Buffer.concat(chunks), Buffer.from(data));
+    assert.strictEqual(req.headers['transfer-encoding'], 'chunked');
+
+    res.setHeader('Content-Type', 'text/plain');
+    res.end();
+  }));
 })).unref();
 
 server.listen(0, common.mustCall(() => {
   const port = server.address().port;
-  const req = http.request({ method: 'DELETE', port }, function(res) {
+  const req = http.request({ method: 'DELETE', port }, common.mustCall((res) => {
     res.resume();
-  });
+  }));
 
   req.write(data);
   req.end();

test/parallel/test-stream-pipeline.js

@@ -271,9 +271,10 @@ tmpdir.refresh();
 {
   const server = http.createServer(common.mustCallAtLeast((req, res) => {
     pipeline(req, res, common.mustCall((err) => {
-      // The client destroys the request body source before EOF below, so the
-      // echoed response cannot finish successfully either.
-      assert.strictEqual(err?.code, 'ERR_STREAM_PREMATURE_CLOSE');
+      // The client destroys the request body source before EOF below. The
+      // request is framed as chunked, so it ends without its terminating
+      // chunk and the connection is reset rather than closed cleanly.
+      assert.strictEqual(err?.code, 'ECONNRESET');
     }));
   }));