From d29f5ef55e7e21fbfd26139c36a692717bc7f2ed Mon Sep 17 00:00:00 2001 From: Gerrard Lindsay Date: Mon, 1 May 2023 15:27:06 -0400 Subject: [PATCH] http: rejecting invalid body writes configurable with option rejectNonStandardBodyWrites --- doc/api/errors.md | 3 +- lib/_http_outgoing.js | 11 +- lib/_http_server.js | 14 ++- lib/http.js | 1 + ...-http-head-throw-on-response-body-write.js | 100 ++++++++++++++---- 5 files changed, 104 insertions(+), 25 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 17a16e70ac4bf8..b11e8c8105120c 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1346,7 +1346,8 @@ Response body size doesn't match with the specified content-length header value. ### `ERR_HTTP_CONTENT_LENGTH_MISMATCH` -An error is thrown if when writing to an HTTP response which does not allow contents. +An error is thrown if when writing to an HTTP response which does not allow +contents. diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 6e58bffdc93b0b..f06da7a29aba0f 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -86,6 +86,7 @@ const kUniqueHeaders = Symbol('kUniqueHeaders'); const kBytesWritten = Symbol('kBytesWritten'); const kErrored = Symbol('errored'); const kHighWaterMark = Symbol('kHighWaterMark'); +const kRejectNonStandardBodyWrites = Symbol('kRejectNonStandardBodyWrites'); const nop = () => {}; @@ -151,6 +152,7 @@ function OutgoingMessage(options) { this[kErrored] = null; this[kHighWaterMark] = options?.highWaterMark ?? getDefaultHighWaterMark(); + this[kRejectNonStandardBodyWrites] = options?.rejectNonStandardBodyWrites ?? true; } ObjectSetPrototypeOf(OutgoingMessage.prototype, Stream.prototype); ObjectSetPrototypeOf(OutgoingMessage, Stream); @@ -886,7 +888,14 @@ function write_(msg, chunk, encoding, callback, fromEnd) { } if (!msg._hasBody) { - throw ERR_HTTP_BODY_NOT_ALLOWED(); + if (msg[kRejectNonStandardBodyWrites]) { + throw new ERR_HTTP_BODY_NOT_ALLOWED(); + } else { + debug('This type of response MUST NOT have a body. ' + + 'Ignoring write() calls.'); + process.nextTick(callback); + return true; + } } if (err) { diff --git a/lib/_http_server.js b/lib/_http_server.js index e6e592451b3230..b2cce201009c52 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -484,6 +484,14 @@ function storeHTTPOptions(options) { validateBoolean(joinDuplicateHeaders, 'options.joinDuplicateHeaders'); } this.joinDuplicateHeaders = joinDuplicateHeaders; + + const rejectNonStandardBodyWrites = options.rejectNonStandardBodyWrites; + if (rejectNonStandardBodyWrites !== undefined) { + validateBoolean(rejectNonStandardBodyWrites, 'options.rejectNonStandardBodyWrites'); + this.rejectNonStandardBodyWrites = rejectNonStandardBodyWrites; + } else { + this.rejectNonStandardBodyWrites = true + } } function setupConnectionsTracking(server) { @@ -1020,7 +1028,11 @@ function parserOnIncoming(server, socket, state, req, keepAlive) { } } - const res = new server[kServerResponse](req, { highWaterMark: socket.writableHighWaterMark }); + const res = new server[kServerResponse](req, + { + highWaterMark: socket.writableHighWaterMark, + rejectNonStandardBodyWrites: server.rejectNonStandardBodyWrites + }); res._keepAliveTimeout = server.keepAliveTimeout; res._maxRequestsPerSocket = server.maxRequestsPerSocket; res._onPendingData = updateOutgoingData.bind(undefined, diff --git a/lib/http.js b/lib/http.js index a1f811c6955b47..d86d36d12c2b14 100644 --- a/lib/http.js +++ b/lib/http.js @@ -55,6 +55,7 @@ let maxHeaderSize; * requireHostHeader?: boolean; * joinDuplicateHeaders?: boolean; * highWaterMark?: number; + * rejectNonStandardBodyWrites?: boolean; * }} [opts] * @param {Function} [requestListener] * @returns {Server} diff --git a/test/parallel/test-http-head-throw-on-response-body-write.js b/test/parallel/test-http-head-throw-on-response-body-write.js index 40e2a58ffa2063..6c3c3cf9f52e50 100644 --- a/test/parallel/test-http-head-throw-on-response-body-write.js +++ b/test/parallel/test-http-head-throw-on-response-body-write.js @@ -24,30 +24,86 @@ const common = require('../common'); const assert = require('assert'); const http = require('http'); +// should be using common.mustCall on both req res callbacks provided to createServer +{ + const server = http.createServer((req, res) => { + assert.throws(() => { + res.write('this is content'); + }, { + code: 'ERR_HTTP_BODY_NOT_ALLOWED', + name: 'Error', + message: 'Adding content for this request method or response status is not allowed.' + }); + res.end(); + }); + server.listen(0); + + server.on('listening', common.mustCall(function() { + const req = http.request({ + port: this.address().port, + method: 'HEAD', + path: '/' + }, common.mustCall((res) => { + res.resume(); + res.on('end', common.mustCall(function() { + server.close(); + })); + })); + req.end(); + })); +} + +{ + const server = http.createServer({ + rejectNonStandardBodyWrites: true, + }, (req, res) => { + res.writeHead(204); + assert.throws(() => { + res.write('this is content'); + }, { + code: 'ERR_HTTP_BODY_NOT_ALLOWED', + name: 'Error', + message: 'Adding content for this request method or response status is not allowed.' + }); + res.end(); + }); + server.listen(0); + + server.on('listening', common.mustCall(function() { + const req = http.request({ + port: this.address().port, + method: 'GET', + path: '/' + }, common.mustCall((res) => { + res.resume(); + res.on('end', common.mustCall(function() { + server.close(); + })); + })); + req.end(); + })); +} -const server = http.createServer((req, res) => { - res.writeHead(204); - assert.throws(() => { - res.write('this is content'); - }, { - code: 'ERR_HTTP_BODY_NOT_ALLOWED', - name: 'Error', - message: 'Adding content for this request method or response status is not allowed.' +{ + const server = http.createServer({ + rejectNonStandardBodyWrites: false, + }, (req, res) => { + res.writeHead(200); + res.end('this is content'); }); - res.end(); -}); -server.listen(0); + server.listen(0); -server.on('listening', common.mustCall(function() { - const req = http.request({ - port: this.address().port, - method: 'GET', - path: '/' - }, common.mustCall((res) => { - res.resume(); - res.on('end', common.mustCall(function() { - server.close(); + server.on('listening', common.mustCall(function() { + const req = http.request({ + port: this.address().port, + method: 'HEAD', + path: '/' + }, common.mustCall((res) => { + res.resume(); + res.on('end', common.mustCall(function() { + server.close(); + })); })); + req.end(); })); - req.end(); -})); +}