From 03a4615949d8eb91d391002a6688e6de76391c9f Mon Sep 17 00:00:00 2001 From: Gerrard Lindsay Date: Wed, 26 Apr 2023 12:29:12 -0400 Subject: [PATCH] If request method is HEAD don't allow writing body bytes. --- doc/api/http.md | 7 +-- lib/_http_outgoing.js | 12 ++--- lib/internal/errors.js | 2 + ...test-http-head-response-has-no-body-end.js | 2 +- ...-http-head-throw-on-response-body-write.js | 53 +++++++++++++++++++ 5 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 test/parallel/test-http-head-throw-on-response-body-write.js diff --git a/doc/api/http.md b/doc/api/http.md index 30ce1ae3f43671..137c86ba2364fb 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 55a2f8bef39801..9df9d73023e046 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, @@ -884,6 +885,10 @@ function write_(msg, chunk, encoding, callback, fromEnd) { err = new ERR_STREAM_DESTROYED('write'); } + if (!msg._hasBody) { + throw ERR_HTTP_BODY_NOT_ALLOWED() + } + if (err) { if (!msg.destroyed) { onError(msg, err, callback); @@ -916,13 +921,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/internal/errors.js b/lib/internal/errors.js index aa4a149b189699..6dacacd4eb4a42 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1159,6 +1159,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 824a1bafe3a5e3..3e0e6cec240ba8 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 00000000000000..c1ece0419fb673 --- /dev/null +++ b/test/parallel/test-http-head-throw-on-response-body-write.js @@ -0,0 +1,53 @@ +// 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(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(); +}));