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 af8b5fa

Browse filesBrowse files
pimterryaduh95
pimterry
authored and
aduh95
committed
http: add shouldUpgradeCallback to let servers control HTTP upgrades
Previously, you could either add no 'upgrade' event handler, in which case all upgrades were ignored, or add an 'upgrade' handler and all upgrade attempts would effectively succeed and skip normal request handling. This change adds a new shouldUpgradeCallback option to HTTP servers, which receives the request details and returns a boolean that controls whether the request should be upgraded. PR-URL: #59824 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent c42c120 commit af8b5fa
Copy full SHA for af8b5fa

File tree

Expand file treeCollapse file tree

3 files changed

+210
-5
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+210
-5
lines changed
Open diff view settings
Collapse file

‎doc/api/http.md‎

Copy file name to clipboardExpand all lines: doc/api/http.md
+29-2Lines changed: 29 additions & 2 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -1649,6 +1649,11 @@ per connection (in the case of HTTP Keep-Alive connections).
16491649
<!-- YAML
16501650
added: v0.1.94
16511651
changes:
1652+
- version: REPLACEME
1653+
pr-url: https://github.com/nodejs/node/pull/59824
1654+
description: Whether this event is fired can now be controlled by the
1655+
`shouldUpgradeCallback` and sockets will be destroyed
1656+
if upgraded while no event handler is listening.
16521657
- version: v10.0.0
16531658
pr-url: https://github.com/nodejs/node/pull/19981
16541659
description: Not listening to this event no longer causes the socket
@@ -1660,13 +1665,25 @@ changes:
16601665
* `socket` {stream.Duplex} Network socket between the server and client
16611666
* `head` {Buffer} The first packet of the upgraded stream (may be empty)
16621667

1663-
Emitted each time a client requests an HTTP upgrade. Listening to this event
1664-
is optional and clients cannot insist on a protocol change.
1668+
Emitted each time a client's HTTP upgrade request is accepted. By default
1669+
all HTTP upgrade requests are ignored (i.e. only regular `'request'` events
1670+
are emitted, sticking with the normal HTTP request/response flow) unless you
1671+
listen to this event, in which case they are all accepted (i.e. the `'upgrade'`
1672+
event is emitted instead, and future communication must handled directly
1673+
through the raw socket). You can control this more precisely by using the
1674+
server `shouldUpgradeCallback` option.
1675+
1676+
Listening to this event is optional and clients cannot insist on a protocol
1677+
change.
16651678

16661679
After this event is emitted, the request's socket will not have a `'data'`
16671680
event listener, meaning it will need to be bound in order to handle data
16681681
sent to the server on that socket.
16691682

1683+
If an upgrade is accepted by `shouldUpgradeCallback` but no event handler
1684+
is registered then the socket is destroyed, resulting in an immediate
1685+
connection closure for the client.
1686+
16701687
This event is guaranteed to be passed an instance of the {net.Socket} class,
16711688
a subclass of {stream.Duplex}, unless the user specifies a socket
16721689
type other than {net.Socket}.
@@ -3511,6 +3528,9 @@ Found'`.
35113528
<!-- YAML
35123529
added: v0.1.13
35133530
changes:
3531+
- version: REPLACEME
3532+
pr-url: https://github.com/nodejs/node/pull/59824
3533+
description: The `shouldUpgradeCallback` option is now supported.
35143534
- version:
35153535
- v20.1.0
35163536
- v18.17.0
@@ -3600,6 +3620,13 @@ changes:
36003620
* `ServerResponse` {http.ServerResponse} Specifies the `ServerResponse` class
36013621
to be used. Useful for extending the original `ServerResponse`. **Default:**
36023622
`ServerResponse`.
3623+
* `shouldUpgradeCallback(request)` {Function} A callback which receives an
3624+
incoming request and returns a boolean, to control which upgrade attempts
3625+
should be accepted. Accepted upgrades will fire an `'upgrade'` event (or
3626+
their sockets will be destroyed, if no listener is registered) while
3627+
rejected upgrades will fire a `'request'` event like any non-upgrade
3628+
request. This options defaults to
3629+
`() => server.listenerCount('upgrade') > 0`.
36033630
* `uniqueHeaders` {Array} A list of response headers that should be sent only
36043631
once. If the header's value is an array, the items will be joined
36053632
using `; `.
Collapse file

‎lib/_http_server.js‎

Copy file name to clipboardExpand all lines: lib/_http_server.js
+14-3Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ const {
9090
validateBoolean,
9191
validateLinkHeaderValue,
9292
validateObject,
93+
validateFunction,
9394
} = require('internal/validators');
9495
const Buffer = require('buffer').Buffer;
9596
const { setInterval, clearInterval } = require('timers');
@@ -520,6 +521,16 @@ function storeHTTPOptions(options) {
520521
} else {
521522
this.rejectNonStandardBodyWrites = false;
522523
}
524+
525+
const shouldUpgradeCallback = options.shouldUpgradeCallback;
526+
if (shouldUpgradeCallback !== undefined) {
527+
validateFunction(shouldUpgradeCallback, 'options.shouldUpgradeCallback');
528+
this.shouldUpgradeCallback = shouldUpgradeCallback;
529+
} else {
530+
this.shouldUpgradeCallback = function() {
531+
return this.listenerCount('upgrade') > 0;
532+
};
533+
}
523534
}
524535

525536
function setupConnectionsTracking() {
@@ -955,15 +966,15 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
955966
parser = null;
956967

957968
const eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade';
958-
if (eventName === 'upgrade' || server.listenerCount(eventName) > 0) {
969+
if (server.listenerCount(eventName) > 0) {
959970
debug('SERVER have listener for %s', eventName);
960971
const bodyHead = d.slice(ret, d.length);
961972

962973
socket.readableFlowing = null;
963974

964975
server.emit(eventName, req, socket, bodyHead);
965976
} else {
966-
// Got CONNECT method, but have no handler.
977+
// Got upgrade or CONNECT method, but have no handler.
967978
socket.destroy();
968979
}
969980
} else if (parser.incoming && parser.incoming.method === 'PRI') {
@@ -1062,7 +1073,7 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
10621073

10631074
if (req.upgrade) {
10641075
req.upgrade = req.method === 'CONNECT' ||
1065-
server.listenerCount('upgrade') > 0;
1076+
!!server.shouldUpgradeCallback(req);
10661077
if (req.upgrade)
10671078
return 2;
10681079
}
Collapse file

‎test/parallel/test-http-upgrade-server-callback.js‎

Copy file name to clipboard
+167Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
6+
const net = require('net');
7+
const http = require('http');
8+
9+
function testUpgradeCallbackTrue() {
10+
const server = http.createServer({
11+
shouldUpgradeCallback: common.mustCall((req) => {
12+
assert.strictEqual(req.url, '/websocket');
13+
assert.strictEqual(req.headers.upgrade, 'websocket');
14+
15+
return true;
16+
})
17+
});
18+
19+
server.on('upgrade', function(req, socket, upgradeHead) {
20+
assert.strictEqual(req.url, '/websocket');
21+
assert.strictEqual(req.headers.upgrade, 'websocket');
22+
assert.ok(socket instanceof net.Socket);
23+
assert.ok(upgradeHead instanceof Buffer);
24+
25+
socket.write('HTTP/1.1 101 Web Socket Protocol Handshake\r\n' +
26+
'Upgrade: WebSocket\r\n' +
27+
'Connection: Upgrade\r\n' +
28+
'\r\n\r\n');
29+
});
30+
31+
server.on('request', common.mustNotCall());
32+
33+
server.listen(0, common.mustCall(() => {
34+
const req = http.request({
35+
port: server.address().port,
36+
path: '/websocket',
37+
headers: {
38+
'Upgrade': 'websocket',
39+
'Connection': 'Upgrade'
40+
}
41+
});
42+
43+
req.on('upgrade', common.mustCall((res, socket, upgradeHead) => {
44+
assert.strictEqual(res.statusCode, 101);
45+
assert.ok(socket instanceof net.Socket);
46+
assert.ok(upgradeHead instanceof Buffer);
47+
socket.end();
48+
server.close();
49+
50+
testUpgradeCallbackFalse();
51+
}));
52+
53+
req.on('response', common.mustNotCall());
54+
req.end();
55+
}));
56+
}
57+
58+
59+
function testUpgradeCallbackFalse() {
60+
const server = http.createServer({
61+
shouldUpgradeCallback: common.mustCall(() => {
62+
return false;
63+
})
64+
});
65+
66+
server.on('upgrade', common.mustNotCall());
67+
68+
server.on('request', common.mustCall((req, res) => {
69+
res.writeHead(200, { 'Content-Type': 'text/plain' });
70+
res.write('received but not upgraded');
71+
res.end();
72+
}));
73+
74+
server.listen(0, common.mustCall(() => {
75+
const req = http.request({
76+
port: server.address().port,
77+
path: '/websocket',
78+
headers: {
79+
'Upgrade': 'websocket',
80+
'Connection': 'Upgrade'
81+
}
82+
});
83+
84+
req.on('upgrade', common.mustNotCall());
85+
86+
req.on('response', common.mustCall((res) => {
87+
assert.strictEqual(res.statusCode, 200);
88+
let data = '';
89+
res.on('data', (chunk) => { data += chunk; });
90+
res.on('end', common.mustCall(() => {
91+
assert.strictEqual(data, 'received but not upgraded');
92+
server.close();
93+
94+
testUpgradeCallbackTrueWithoutHandler();
95+
}));
96+
}));
97+
req.end();
98+
}));
99+
}
100+
101+
102+
function testUpgradeCallbackTrueWithoutHandler() {
103+
const server = http.createServer({
104+
shouldUpgradeCallback: common.mustCall(() => {
105+
return true;
106+
})
107+
});
108+
109+
// N.b: no 'upgrade' handler
110+
server.on('request', common.mustNotCall());
111+
112+
server.listen(0, common.mustCall(() => {
113+
const req = http.request({
114+
port: server.address().port,
115+
path: '/websocket',
116+
headers: {
117+
'Upgrade': 'websocket',
118+
'Connection': 'Upgrade'
119+
}
120+
});
121+
122+
req.on('upgrade', common.mustNotCall());
123+
req.on('response', common.mustNotCall());
124+
125+
req.on('error', common.mustCall((e) => {
126+
assert.strictEqual(e.code, 'ECONNRESET');
127+
server.close();
128+
129+
testUpgradeCallbackError();
130+
}));
131+
req.end();
132+
}));
133+
}
134+
135+
136+
function testUpgradeCallbackError() {
137+
const server = http.createServer({
138+
shouldUpgradeCallback: common.mustCall(() => {
139+
throw new Error('should upgrade callback failed');
140+
})
141+
});
142+
143+
server.on('upgrade', common.mustNotCall());
144+
server.on('request', common.mustNotCall());
145+
146+
server.listen(0, common.mustCall(() => {
147+
const req = http.request({
148+
port: server.address().port,
149+
path: '/websocket',
150+
headers: {
151+
'Upgrade': 'websocket',
152+
'Connection': 'Upgrade'
153+
}
154+
});
155+
156+
req.on('upgrade', common.mustNotCall());
157+
req.on('response', common.mustNotCall());
158+
159+
process.on('uncaughtException', common.mustCall(() => {
160+
process.exit(0);
161+
}));
162+
163+
req.end();
164+
}));
165+
}
166+
167+
testUpgradeCallbackTrue();

0 commit comments

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