diff options
author | Gerrard Lindsay <gerrardalindsay@gmail.com> | 2023-05-13 13:09:26 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-05-13 17:09:26 +0000 |
commit | 5ec0f39a7a565b5a82fe90ba9f095731a7b8b005 (patch) | |
tree | 20c8778d8d43f2862b59f97b3d84002753fd4759 | |
parent | 23e6b12edb0b792b82ff948e3b6c5cb2038c17bf (diff) | |
download | node-new-5ec0f39a7a565b5a82fe90ba9f095731a7b8b005.tar.gz |
http: prevent writing to the body when not allowed by HTTP spec
PR-URL: https://github.com/nodejs/node/pull/47732
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
-rw-r--r-- | doc/api/errors.md | 5 | ||||
-rw-r--r-- | doc/api/http.md | 7 | ||||
-rw-r--r-- | lib/_http_outgoing.js | 21 | ||||
-rw-r--r-- | lib/_http_server.js | 14 | ||||
-rw-r--r-- | lib/http.js | 1 | ||||
-rw-r--r-- | lib/internal/errors.js | 2 | ||||
-rw-r--r-- | test/parallel/test-http-head-response-has-no-body-end.js | 2 | ||||
-rw-r--r-- | test/parallel/test-http-head-throw-on-response-body-write.js | 102 |
8 files changed, 142 insertions, 12 deletions
diff --git a/doc/api/errors.md b/doc/api/errors.md index 992ab1e7a2..642d2445e5 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1338,6 +1338,11 @@ When using [`fs.cp()`][], `src` or `dest` pointed to an invalid path. <a id="ERR_FS_CP_FIFO_PIPE"></a> +### `ERR_HTTP_BODY_NOT_ALLOWED` + +An error is thrown when writing to an HTTP response which does not allow +contents. <a id="ERR_HTTP_BODY_NOT_ALLOWED"></a> + ### `ERR_HTTP_CONTENT_LENGTH_MISMATCH` Response body size doesn't match with the specified content-length header value. diff --git a/doc/api/http.md b/doc/api/http.md index 9493c6a269..b6fa32a3c0 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -2144,9 +2144,10 @@ it will switch to implicit header mode and flush the implicit headers. This sends a chunk of the response body. This method may be called multiple times to provide successive parts of the body. -In the `node:http` module, the response body is omitted when the -request is a HEAD request. Similarly, the `204` and `304` responses -_must not_ include a message body. +Writing to the body is not allowed when the request method or response status +do not support content. If an attempt is made to write to the body for a +HEAD request or as part of a `204` or `304`response, a synchronous `Error` +with the code `ERR_HTTP_BODY_NOT_ALLOWED` is thrown. `chunk` can be a string or a buffer. If `chunk` is a string, the second parameter specifies how to encode it into a byte stream. diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 55a2f8bef3..73bd2aad8f 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -60,6 +60,7 @@ const { ERR_HTTP_HEADERS_SENT, ERR_HTTP_INVALID_HEADER_VALUE, ERR_HTTP_TRAILER_INVALID, + ERR_HTTP_BODY_NOT_ALLOWED, ERR_INVALID_HTTP_TOKEN, ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, @@ -85,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 = () => {}; @@ -150,6 +152,7 @@ function OutgoingMessage(options) { this[kErrored] = null; this[kHighWaterMark] = options?.highWaterMark ?? getDefaultHighWaterMark(); + this[kRejectNonStandardBodyWrites] = options?.rejectNonStandardBodyWrites ?? false; } ObjectSetPrototypeOf(OutgoingMessage.prototype, Stream.prototype); ObjectSetPrototypeOf(OutgoingMessage, Stream); @@ -884,6 +887,17 @@ function write_(msg, chunk, encoding, callback, fromEnd) { err = new ERR_STREAM_DESTROYED('write'); } + if (!msg._hasBody) { + 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) { if (!msg.destroyed) { onError(msg, err, callback); @@ -916,13 +930,6 @@ function write_(msg, chunk, encoding, callback, fromEnd) { msg._implicitHeader(); } - if (!msg._hasBody) { - debug('This type of response MUST NOT have a body. ' + - 'Ignoring write() calls.'); - process.nextTick(callback); - return true; - } - if (!fromEnd && msg.socket && !msg.socket.writableCorked) { msg.socket.cork(); process.nextTick(connectionCorkNT, msg.socket); diff --git a/lib/_http_server.js b/lib/_http_server.js index 774bdc368f..c6a0701155 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -487,6 +487,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 = false; + } } function setupConnectionsTracking(server) { @@ -1023,7 +1031,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 a1f811c695..d86d36d12c 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/lib/internal/errors.js b/lib/internal/errors.js index 18f4a2c42c..8a490ea3a4 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1154,6 +1154,8 @@ E('ERR_HTTP2_TRAILERS_NOT_READY', 'Trailing headers cannot be sent until after the wantTrailers event is ' + 'emitted', Error); E('ERR_HTTP2_UNSUPPORTED_PROTOCOL', 'protocol "%s" is unsupported.', Error); +E('ERR_HTTP_BODY_NOT_ALLOWED', + 'Adding content for this request method or response status is not allowed.', Error); E('ERR_HTTP_CONTENT_LENGTH_MISMATCH', 'Response body\'s content-length of %s byte(s) does not match the content-length of %s byte(s) set in header', Error); E('ERR_HTTP_HEADERS_SENT', diff --git a/test/parallel/test-http-head-response-has-no-body-end.js b/test/parallel/test-http-head-response-has-no-body-end.js index 824a1bafe3..3e0e6cec24 100644 --- a/test/parallel/test-http-head-response-has-no-body-end.js +++ b/test/parallel/test-http-head-response-has-no-body-end.js @@ -29,7 +29,7 @@ const http = require('http'); const server = http.createServer(function(req, res) { res.writeHead(200); - res.end('FAIL'); // broken: sends FAIL from hot path. + res.end(); }); server.listen(0); 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 new file mode 100644 index 0000000000..7352b20d84 --- /dev/null +++ b/test/parallel/test-http-head-throw-on-response-body-write.js @@ -0,0 +1,102 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +{ + const server = http.createServer((req, res) => { + res.writeHead(200); + res.end('this is content'); + }); + 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({ + rejectNonStandardBodyWrites: false, + }, (req, res) => { + res.writeHead(200); + res.end('this is content'); + }); + 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(); + })); +} |