Skip to content

Commit 4197a9a

Browse files
gerrard00targos
authored andcommitted
http: prevent writing to the body when not allowed by HTTP spec
PR-URL: #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>
1 parent e6685f9 commit 4197a9a

8 files changed

+142
-12
lines changed

doc/api/errors.md

+5
Original file line numberDiff line numberDiff line change
@@ -1338,6 +1338,11 @@ When using [`fs.cp()`][], `src` or `dest` pointed to an invalid path.
13381338

13391339
<a id="ERR_FS_CP_FIFO_PIPE"></a>
13401340

1341+
### `ERR_HTTP_BODY_NOT_ALLOWED`
1342+
1343+
An error is thrown when writing to an HTTP response which does not allow
1344+
contents. <a id="ERR_HTTP_BODY_NOT_ALLOWED"></a>
1345+
13411346
### `ERR_HTTP_CONTENT_LENGTH_MISMATCH`
13421347

13431348
Response body size doesn't match with the specified content-length header value.

doc/api/http.md

+4-3
Original file line numberDiff line numberDiff line change
@@ -2144,9 +2144,10 @@ it will switch to implicit header mode and flush the implicit headers.
21442144
This sends a chunk of the response body. This method may
21452145
be called multiple times to provide successive parts of the body.
21462146

2147-
In the `node:http` module, the response body is omitted when the
2148-
request is a HEAD request. Similarly, the `204` and `304` responses
2149-
_must not_ include a message body.
2147+
Writing to the body is not allowed when the request method or response status
2148+
do not support content. If an attempt is made to write to the body for a
2149+
HEAD request or as part of a `204` or `304`response, a synchronous `Error`
2150+
with the code `ERR_HTTP_BODY_NOT_ALLOWED` is thrown.
21502151

21512152
`chunk` can be a string or a buffer. If `chunk` is a string,
21522153
the second parameter specifies how to encode it into a byte stream.

lib/_http_outgoing.js

+14-7
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ const {
6060
ERR_HTTP_HEADERS_SENT,
6161
ERR_HTTP_INVALID_HEADER_VALUE,
6262
ERR_HTTP_TRAILER_INVALID,
63+
ERR_HTTP_BODY_NOT_ALLOWED,
6364
ERR_INVALID_HTTP_TOKEN,
6465
ERR_INVALID_ARG_TYPE,
6566
ERR_INVALID_ARG_VALUE,
@@ -85,6 +86,7 @@ const kUniqueHeaders = Symbol('kUniqueHeaders');
8586
const kBytesWritten = Symbol('kBytesWritten');
8687
const kErrored = Symbol('errored');
8788
const kHighWaterMark = Symbol('kHighWaterMark');
89+
const kRejectNonStandardBodyWrites = Symbol('kRejectNonStandardBodyWrites');
8890

8991
const nop = () => {};
9092

@@ -150,6 +152,7 @@ function OutgoingMessage(options) {
150152

151153
this[kErrored] = null;
152154
this[kHighWaterMark] = options?.highWaterMark ?? getDefaultHighWaterMark();
155+
this[kRejectNonStandardBodyWrites] = options?.rejectNonStandardBodyWrites ?? false;
153156
}
154157
ObjectSetPrototypeOf(OutgoingMessage.prototype, Stream.prototype);
155158
ObjectSetPrototypeOf(OutgoingMessage, Stream);
@@ -884,6 +887,17 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
884887
err = new ERR_STREAM_DESTROYED('write');
885888
}
886889

890+
if (!msg._hasBody) {
891+
if (msg[kRejectNonStandardBodyWrites]) {
892+
throw new ERR_HTTP_BODY_NOT_ALLOWED();
893+
} else {
894+
debug('This type of response MUST NOT have a body. ' +
895+
'Ignoring write() calls.');
896+
process.nextTick(callback);
897+
return true;
898+
}
899+
}
900+
887901
if (err) {
888902
if (!msg.destroyed) {
889903
onError(msg, err, callback);
@@ -916,13 +930,6 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
916930
msg._implicitHeader();
917931
}
918932

919-
if (!msg._hasBody) {
920-
debug('This type of response MUST NOT have a body. ' +
921-
'Ignoring write() calls.');
922-
process.nextTick(callback);
923-
return true;
924-
}
925-
926933
if (!fromEnd && msg.socket && !msg.socket.writableCorked) {
927934
msg.socket.cork();
928935
process.nextTick(connectionCorkNT, msg.socket);

lib/_http_server.js

+13-1
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,14 @@ function storeHTTPOptions(options) {
487487
validateBoolean(joinDuplicateHeaders, 'options.joinDuplicateHeaders');
488488
}
489489
this.joinDuplicateHeaders = joinDuplicateHeaders;
490+
491+
const rejectNonStandardBodyWrites = options.rejectNonStandardBodyWrites;
492+
if (rejectNonStandardBodyWrites !== undefined) {
493+
validateBoolean(rejectNonStandardBodyWrites, 'options.rejectNonStandardBodyWrites');
494+
this.rejectNonStandardBodyWrites = rejectNonStandardBodyWrites;
495+
} else {
496+
this.rejectNonStandardBodyWrites = false;
497+
}
490498
}
491499

492500
function setupConnectionsTracking(server) {
@@ -1023,7 +1031,11 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
10231031
}
10241032
}
10251033

1026-
const res = new server[kServerResponse](req, { highWaterMark: socket.writableHighWaterMark });
1034+
const res = new server[kServerResponse](req,
1035+
{
1036+
highWaterMark: socket.writableHighWaterMark,
1037+
rejectNonStandardBodyWrites: server.rejectNonStandardBodyWrites,
1038+
});
10271039
res._keepAliveTimeout = server.keepAliveTimeout;
10281040
res._maxRequestsPerSocket = server.maxRequestsPerSocket;
10291041
res._onPendingData = updateOutgoingData.bind(undefined,

lib/http.js

+1
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ let maxHeaderSize;
5555
* requireHostHeader?: boolean;
5656
* joinDuplicateHeaders?: boolean;
5757
* highWaterMark?: number;
58+
* rejectNonStandardBodyWrites?: boolean;
5859
* }} [opts]
5960
* @param {Function} [requestListener]
6061
* @returns {Server}

lib/internal/errors.js

+2
Original file line numberDiff line numberDiff line change
@@ -1154,6 +1154,8 @@ E('ERR_HTTP2_TRAILERS_NOT_READY',
11541154
'Trailing headers cannot be sent until after the wantTrailers event is ' +
11551155
'emitted', Error);
11561156
E('ERR_HTTP2_UNSUPPORTED_PROTOCOL', 'protocol "%s" is unsupported.', Error);
1157+
E('ERR_HTTP_BODY_NOT_ALLOWED',
1158+
'Adding content for this request method or response status is not allowed.', Error);
11571159
E('ERR_HTTP_CONTENT_LENGTH_MISMATCH',
11581160
'Response body\'s content-length of %s byte(s) does not match the content-length of %s byte(s) set in header', Error);
11591161
E('ERR_HTTP_HEADERS_SENT',

test/parallel/test-http-head-response-has-no-body-end.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const http = require('http');
2929

3030
const server = http.createServer(function(req, res) {
3131
res.writeHead(200);
32-
res.end('FAIL'); // broken: sends FAIL from hot path.
32+
res.end();
3333
});
3434
server.listen(0);
3535

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
22+
'use strict';
23+
const common = require('../common');
24+
const assert = require('assert');
25+
const http = require('http');
26+
27+
{
28+
const server = http.createServer((req, res) => {
29+
res.writeHead(200);
30+
res.end('this is content');
31+
});
32+
server.listen(0);
33+
34+
server.on('listening', common.mustCall(function() {
35+
const req = http.request({
36+
port: this.address().port,
37+
method: 'HEAD',
38+
path: '/'
39+
}, common.mustCall((res) => {
40+
res.resume();
41+
res.on('end', common.mustCall(function() {
42+
server.close();
43+
}));
44+
}));
45+
req.end();
46+
}));
47+
}
48+
49+
{
50+
const server = http.createServer({
51+
rejectNonStandardBodyWrites: true,
52+
}, (req, res) => {
53+
res.writeHead(204);
54+
assert.throws(() => {
55+
res.write('this is content');
56+
}, {
57+
code: 'ERR_HTTP_BODY_NOT_ALLOWED',
58+
name: 'Error',
59+
message: 'Adding content for this request method or response status is not allowed.'
60+
});
61+
res.end();
62+
});
63+
server.listen(0);
64+
65+
server.on('listening', common.mustCall(function() {
66+
const req = http.request({
67+
port: this.address().port,
68+
method: 'GET',
69+
path: '/'
70+
}, common.mustCall((res) => {
71+
res.resume();
72+
res.on('end', common.mustCall(function() {
73+
server.close();
74+
}));
75+
}));
76+
req.end();
77+
}));
78+
}
79+
80+
{
81+
const server = http.createServer({
82+
rejectNonStandardBodyWrites: false,
83+
}, (req, res) => {
84+
res.writeHead(200);
85+
res.end('this is content');
86+
});
87+
server.listen(0);
88+
89+
server.on('listening', common.mustCall(function() {
90+
const req = http.request({
91+
port: this.address().port,
92+
method: 'HEAD',
93+
path: '/'
94+
}, common.mustCall((res) => {
95+
res.resume();
96+
res.on('end', common.mustCall(function() {
97+
server.close();
98+
}));
99+
}));
100+
req.end();
101+
}));
102+
}

0 commit comments

Comments
 (0)