Skip to content

Commit dc318f2

Browse files
gerrard00MoLow
authored andcommittedJul 6, 2023
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 4b2aa3d commit dc318f2

8 files changed

+142
-12
lines changed
 

‎doc/api/errors.md

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

13321332
<a id="ERR_FS_CP_FIFO_PIPE"></a>
13331333

1334+
### `ERR_HTTP_BODY_NOT_ALLOWED`
1335+
1336+
An error is thrown when writing to an HTTP response which does not allow
1337+
contents. <a id="ERR_HTTP_BODY_NOT_ALLOWED"></a>
1338+
13341339
### `ERR_HTTP_CONTENT_LENGTH_MISMATCH`
13351340

13361341
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
@@ -2129,9 +2129,10 @@ it will switch to implicit header mode and flush the implicit headers.
21292129
This sends a chunk of the response body. This method may
21302130
be called multiple times to provide successive parts of the body.
21312131

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

21362137
`chunk` can be a string or a buffer. If `chunk` is a string,
21372138
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
@@ -61,6 +61,7 @@ const {
6161
ERR_HTTP_HEADERS_SENT,
6262
ERR_HTTP_INVALID_HEADER_VALUE,
6363
ERR_HTTP_TRAILER_INVALID,
64+
ERR_HTTP_BODY_NOT_ALLOWED,
6465
ERR_INVALID_HTTP_TOKEN,
6566
ERR_INVALID_ARG_TYPE,
6667
ERR_INVALID_ARG_VALUE,
@@ -86,6 +87,7 @@ const kUniqueHeaders = Symbol('kUniqueHeaders');
8687
const kBytesWritten = Symbol('kBytesWritten');
8788
const kErrored = Symbol('errored');
8889
const kHighWaterMark = Symbol('kHighWaterMark');
90+
const kRejectNonStandardBodyWrites = Symbol('kRejectNonStandardBodyWrites');
8991

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

@@ -151,6 +153,7 @@ function OutgoingMessage(options) {
151153

152154
this[kErrored] = null;
153155
this[kHighWaterMark] = options?.highWaterMark ?? getDefaultHighWaterMark();
156+
this[kRejectNonStandardBodyWrites] = options?.rejectNonStandardBodyWrites ?? false;
154157
}
155158
ObjectSetPrototypeOf(OutgoingMessage.prototype, Stream.prototype);
156159
ObjectSetPrototypeOf(OutgoingMessage, Stream);
@@ -880,6 +883,17 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
880883
err = new ERR_STREAM_DESTROYED('write');
881884
}
882885

886+
if (!msg._hasBody) {
887+
if (msg[kRejectNonStandardBodyWrites]) {
888+
throw new ERR_HTTP_BODY_NOT_ALLOWED();
889+
} else {
890+
debug('This type of response MUST NOT have a body. ' +
891+
'Ignoring write() calls.');
892+
process.nextTick(callback);
893+
return true;
894+
}
895+
}
896+
883897
if (err) {
884898
if (!msg.destroyed) {
885899
onError(msg, err, callback);
@@ -912,13 +926,6 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
912926
msg._implicitHeader();
913927
}
914928

915-
if (!msg._hasBody) {
916-
debug('This type of response MUST NOT have a body. ' +
917-
'Ignoring write() calls.');
918-
process.nextTick(callback);
919-
return true;
920-
}
921-
922929
if (!fromEnd && msg.socket && !msg.socket.writableCorked) {
923930
msg.socket.cork();
924931
process.nextTick(connectionCorkNT, msg.socket);

‎lib/_http_server.js

+13-1
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,14 @@ function storeHTTPOptions(options) {
483483
validateBoolean(joinDuplicateHeaders, 'options.joinDuplicateHeaders');
484484
}
485485
this.joinDuplicateHeaders = joinDuplicateHeaders;
486+
487+
const rejectNonStandardBodyWrites = options.rejectNonStandardBodyWrites;
488+
if (rejectNonStandardBodyWrites !== undefined) {
489+
validateBoolean(rejectNonStandardBodyWrites, 'options.rejectNonStandardBodyWrites');
490+
this.rejectNonStandardBodyWrites = rejectNonStandardBodyWrites;
491+
} else {
492+
this.rejectNonStandardBodyWrites = false;
493+
}
486494
}
487495

488496
function setupConnectionsTracking(server) {
@@ -1018,7 +1026,11 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
10181026
}
10191027
}
10201028

1021-
const res = new server[kServerResponse](req, { highWaterMark: socket.writableHighWaterMark });
1029+
const res = new server[kServerResponse](req,
1030+
{
1031+
highWaterMark: socket.writableHighWaterMark,
1032+
rejectNonStandardBodyWrites: server.rejectNonStandardBodyWrites,
1033+
});
10221034
res._keepAliveTimeout = server.keepAliveTimeout;
10231035
res._maxRequestsPerSocket = server.maxRequestsPerSocket;
10241036
res._onPendingData = updateOutgoingData.bind(undefined,

‎lib/http.js

+1
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ let maxHeaderSize;
5454
* maxHeaderSize?: number;
5555
* joinDuplicateHeaders?: boolean;
5656
* highWaterMark?: number;
57+
* rejectNonStandardBodyWrites?: boolean;
5758
* }} [opts]
5859
* @param {Function} [requestListener]
5960
* @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)
Please sign in to comment.