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 e52cc24

Browse filesBrowse files
ronagaddaleax
authored andcommitted
http: don't write error to socket
The state of the connection is unknown at this point and writing to it can corrupt client state before it is aware of an error. PR-URL: #34465 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent d8c5bda commit e52cc24
Copy full SHA for e52cc24

File tree

Expand file treeCollapse file tree

3 files changed

+34
-3
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+34
-3
lines changed
Open diff view settings
Collapse file

‎doc/api/http.md‎

Copy file name to clipboardExpand all lines: doc/api/http.md
+2-2Lines changed: 2 additions & 2 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -977,8 +977,8 @@ type other than {net.Socket}.
977977

978978
Default behavior is to try close the socket with a HTTP '400 Bad Request',
979979
or a HTTP '431 Request Header Fields Too Large' in the case of a
980-
[`HPE_HEADER_OVERFLOW`][] error. If the socket is not writable it is
981-
immediately destroyed.
980+
[`HPE_HEADER_OVERFLOW`][] error. If the socket is not writable or has already
981+
written data it is immediately destroyed.
982982

983983
`socket` is the [`net.Socket`][] object that the error originated from.
984984

Collapse file

‎lib/_http_server.js‎

Copy file name to clipboardExpand all lines: lib/_http_server.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,7 @@ function socketOnError(e) {
597597
this.on('error', noop);
598598

599599
if (!this.server.emit('clientError', e, this)) {
600-
if (this.writable) {
600+
if (this.writable && this.bytesWritten === 0) {
601601
const response = e.code === 'HPE_HEADER_OVERFLOW' ?
602602
requestHeaderFieldsTooLargeResponse : badRequestResponse;
603603
this.write(response);
Collapse file
+31Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
'use strict';
2+
const { mustCall } = require('../common');
3+
const assert = require('assert');
4+
const { createServer } = require('http');
5+
const { createConnection } = require('net');
6+
7+
const server = createServer();
8+
9+
server.on('request', mustCall((req, res) => {
10+
res.write('asd', () => {
11+
res.socket.emit('error', new Error('kaboom'));
12+
});
13+
}));
14+
15+
server.listen(0, mustCall(() => {
16+
const c = createConnection(server.address().port);
17+
let received = '';
18+
19+
c.on('connect', mustCall(() => {
20+
c.write('GET /blah HTTP/1.1\r\n\r\n');
21+
}));
22+
c.on('data', mustCall((data) => {
23+
received += data.toString();
24+
}));
25+
c.on('end', mustCall(() => {
26+
// Should not include anything else after asd.
27+
assert.strictEqual(received.indexOf('asd\r\n'), received.length - 5);
28+
c.end();
29+
}));
30+
c.on('close', mustCall(() => server.close()));
31+
}));

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.