Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit dc318f2

Browse filesBrowse files
gerrard00MoLow
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 4b2aa3d commit dc318f2
Copy full SHA for dc318f2

File tree

Expand file treeCollapse file tree

8 files changed

+142
-12
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+142
-12
lines changed
Open diff view settings
Collapse file

‎doc/api/errors.md‎

Copy file name to clipboardExpand all lines: doc/api/errors.md
+5Lines changed: 5 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
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.
Collapse file

‎doc/api/http.md‎

Copy file name to clipboardExpand all lines: doc/api/http.md
+4-3Lines changed: 4 additions & 3 deletions
  • Display the source diff
  • Display the rich diff
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.
Collapse file

‎lib/_http_outgoing.js‎

Copy file name to clipboardExpand all lines: lib/_http_outgoing.js
+14-7Lines changed: 14 additions & 7 deletions
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);
Collapse file

‎lib/_http_server.js‎

Copy file name to clipboardExpand all lines: lib/_http_server.js
+13-1Lines changed: 13 additions & 1 deletion
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,
Collapse file

‎lib/http.js‎

Copy file name to clipboardExpand all lines: lib/http.js
+1Lines changed: 1 addition & 0 deletions
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}
Collapse file

‎lib/internal/errors.js‎

Copy file name to clipboardExpand all lines: lib/internal/errors.js
+2Lines changed: 2 additions & 0 deletions
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',
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-http-head-response-has-no-body-end.js
+1-1Lines changed: 1 addition & 1 deletion
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

Collapse file
+102Lines changed: 102 additions & 0 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.