diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 0d0507c93f47..6d983b69c7d4 100644 --- a/lib/_http_outgoing.js +++ b/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(); } diff --git a/test/known_issues/test-http-clientrequest-end-contentlength.js b/test/known_issues/test-http-clientrequest-end-contentlength.js deleted file mode 100644 index 988dd08bd26e..000000000000 --- a/test/known_issues/test-http-clientrequest-end-contentlength.js +++ /dev/null @@ -1,51 +0,0 @@ -'use strict'; - -// Refs: https://github.com/nodejs/node/pull/34066 - -const common = require('../common'); -const assert = require('assert'); -const http = require('http'); - -// Test that ClientRequest#end with default options -// computes and sends a Content-Length header -const upload = 'PUT / HTTP/1.1\r\n\r\n'; -const response = 'content-length: 19\r\n'; - -// Test that the upload is properly received with the same headers, -// regardless of request method -const methods = [ 'GET', 'HEAD', 'DELETE', 'POST', 'PATCH', 'PUT', 'OPTIONS' ]; - -const server = http.createServer(common.mustCall(function(req, res) { - req.on('data', common.mustCall((chunk) => { - assert.strictEqual(chunk, Buffer.from(upload)); - })); - res.setHeader('Content-Type', 'text/plain'); - let payload = `${req.method}\r\n`; - for (let i = 0; i < req.rawHeaders.length; i += 2) { - // Ignore a couple headers that may vary - if (req.rawHeaders[i].toLowerCase() === 'host') continue; - if (req.rawHeaders[i].toLowerCase() === 'connection') continue; - payload += `${req.rawHeaders[i]}: ${req.rawHeaders[i + 1]}\r\n`; - } - res.end(payload); -}), 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) => { - const chunks = []; - res.on('data', function(chunk) { - chunks.push(chunk); - }); - res.on('end', common.mustCall(() => { - const received = Buffer.concat(chunks).toString(); - const expected = method.toLowerCase() + '\r\n' + response; - assert.strictEqual(received.toLowerCase(), expected); - tryNextRequest(); - })); - })); - - req.end(upload); -})).unref(); diff --git a/test/known_issues/test-http-clientrequest-write-chunked.js b/test/known_issues/test-http-clientrequest-write-chunked.js deleted file mode 100644 index 77d9f503fbd0..000000000000 --- a/test/known_issues/test-http-clientrequest-write-chunked.js +++ /dev/null @@ -1,52 +0,0 @@ -'use strict'; - -// Refs: https://github.com/nodejs/node/pull/34066 - -const common = require('../common'); -const assert = require('assert'); -const http = require('http'); - -// Test that ClientRequest#write with default options -// uses a chunked Transfer-Encoding -const upload = 'PUT / HTTP/1.1\r\n\r\n'; -const response = 'transfer-encoding: chunked\r\n'; - -// Test that the upload is properly received with the same headers, -// regardless of request method. -const methods = [ 'GET', 'HEAD', 'DELETE', 'POST', 'PATCH', 'PUT', 'OPTIONS' ]; - -const server = http.createServer(common.mustCall(function(req, res) { - req.on('data', common.mustCall((chunk) => { - assert.strictEqual(chunk.toString(), upload); - })); - res.setHeader('Content-Type', 'text/plain'); - res.write(`${req.method}\r\n`); - for (let i = 0; i < req.rawHeaders.length; i += 2) { - // Ignore a couple headers that may vary - 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(); -}), 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) => { - const chunks = []; - res.on('data', function(chunk) { - chunks.push(chunk); - }); - res.on('end', common.mustCall(() => { - const received = Buffer.concat(chunks).toString(); - const expected = method.toLowerCase() + '\r\n' + response; - assert.strictEqual(received.toLowerCase(), expected); - tryNextRequest(); - })); - })); - - req.write(upload); - req.end(); -})).unref(); diff --git a/test/parallel/test-http-clientrequest-end-contentlength.js b/test/parallel/test-http-clientrequest-end-contentlength.js new file mode 100644 index 000000000000..6c5f490eeed5 --- /dev/null +++ b/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(); diff --git a/test/parallel/test-http-clientrequest-write-chunked.js b/test/parallel/test-http-clientrequest-write-chunked.js new file mode 100644 index 000000000000..b251812bf18d --- /dev/null +++ b/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(); diff --git a/test/parallel/test-http-request-method-delete-payload.js b/test/parallel/test-http-request-method-delete-payload.js index ffefd7d470d8..dca16cfe6095 100644 --- a/test/parallel/test-http-request-method-delete-payload.js +++ b/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(); diff --git a/test/parallel/test-stream-pipeline.js b/test/parallel/test-stream-pipeline.js index 8ee197b44e0b..ef0d5492c579 100644 --- a/test/parallel/test-stream-pipeline.js +++ b/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'); })); }));