Skip to content

Commit

Permalink
http: fixed socket.setEncoding fatal error
Browse files Browse the repository at this point in the history
Applied updates from previous pull-requests to disallow
socket.setEncoding before a http connection is parsed.
Wrapped `socket.setEncoding` to throw an error.
This previously resulted in a fatal error.

PR-URL: nodejs#33405
Fixes: nodejs#18118
Refs: nodejs#18178
Refs: nodejs#19344
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
  • Loading branch information
iSkore authored and BridgeAR committed May 23, 2020
1 parent 10596b6 commit 32b641e
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 0 deletions.
6 changes: 6 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,11 @@ An invalid HTTP header value was specified.

Status code was outside the regular status code range (100-999).

<a id="ERR_HTTP_SOCKET_ENCODING"></a>
### `ERR_HTTP_SOCKET_ENCODING`

Changing the socket encoding is not allowed per [RFC 7230 Section 3][].

<a id="ERR_HTTP_TRAILER_INVALID"></a>
### `ERR_HTTP_TRAILER_INVALID`

Expand Down Expand Up @@ -2613,6 +2618,7 @@ such as `process.stdout.on('data')`.
[exports]: esm.html#esm_package_entry_points
[file descriptors]: https://en.wikipedia.org/wiki/File_descriptor
[policy]: policy.html
[RFC 7230 Section 3]: https://tools.ietf.org/html/rfc7230#section-3
[stream-based]: stream.html
[syscall]: http://man7.org/linux/man-pages/man2/syscalls.2.html
[Subresource Integrity specification]: https://www.w3.org/TR/SRI/#the-integrity-attribute
Expand Down
6 changes: 6 additions & 0 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const {
const {
ERR_HTTP_HEADERS_SENT,
ERR_HTTP_INVALID_STATUS_CODE,
ERR_HTTP_SOCKET_ENCODING,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_CHAR
} = codes;
Expand Down Expand Up @@ -476,6 +477,7 @@ function connectionListenerInternal(server, socket) {
socket.on = generateSocketListenerWrapper('on');
socket.addListener = generateSocketListenerWrapper('addListener');
socket.prependListener = generateSocketListenerWrapper('prependListener');
socket.setEncoding = socketSetEncoding;

// We only consume the socket if it has never been consumed before.
if (socket._handle && socket._handle.isStreamBase &&
Expand All @@ -493,6 +495,10 @@ function connectionListenerInternal(server, socket) {
socket._paused = false;
}

function socketSetEncoding() {
throw new ERR_HTTP_SOCKET_ENCODING();
}

function updateOutgoingData(socket, state, delta) {
state.outgoingData += delta;
socketOnDrain(socket, state);
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,8 @@ E('ERR_HTTP_HEADERS_SENT',
E('ERR_HTTP_INVALID_HEADER_VALUE',
'Invalid value "%s" for header "%s"', TypeError);
E('ERR_HTTP_INVALID_STATUS_CODE', 'Invalid status code: %s', RangeError);
E('ERR_HTTP_SOCKET_ENCODING',
'Changing the socket encoding is not allowed per RFC7230 Section 3.', Error);
E('ERR_HTTP_TRAILER_INVALID',
'Trailers are invalid with this transfer encoding', Error);
E('ERR_INCOMPATIBLE_OPTION_PAIR',
Expand Down
29 changes: 29 additions & 0 deletions test/parallel/test-http-socket-encoding-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const http = require('http');

const server = http.createServer().listen(0, connectToServer);

server.on('connection', common.mustCall((socket) => {
assert.throws(
() => {
socket.setEncoding('');
},
{
code: 'ERR_HTTP_SOCKET_ENCODING',
name: 'Error',
message: 'Changing the socket encoding is not ' +
'allowed per RFC7230 Section 3.'
}
);

socket.end();
}));

function connectToServer() {
const client = new http.Agent().createConnection(this.address().port, () => {
client.end();
}).on('end', () => server.close());
}

0 comments on commit 32b641e

Please sign in to comment.