Skip to content

Commit 65bee02

Browse files
authored
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 <[email protected]>
1 parent f551c31 commit 65bee02

File tree

3 files changed

+210
-5
lines changed

3 files changed

+210
-5
lines changed

doc/api/http.md

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1671,6 +1671,11 @@ per connection (in the case of HTTP Keep-Alive connections).
16711671
<!-- YAML
16721672
added: v0.1.94
16731673
changes:
1674+
- version: REPLACEME
1675+
pr-url: https://github.com/nodejs/node/pull/59824
1676+
description: Whether this event is fired can now be controlled by the
1677+
`shouldUpgradeCallback` and sockets will be destroyed
1678+
if upgraded while no event handler is listening.
16741679
- version: v10.0.0
16751680
pr-url: https://github.com/nodejs/node/pull/19981
16761681
description: Not listening to this event no longer causes the socket
@@ -1682,13 +1687,25 @@ changes:
16821687
* `socket` {stream.Duplex} Network socket between the server and client
16831688
* `head` {Buffer} The first packet of the upgraded stream (may be empty)
16841689

1685-
Emitted each time a client requests an HTTP upgrade. Listening to this event
1686-
is optional and clients cannot insist on a protocol change.
1690+
Emitted each time a client's HTTP upgrade request is accepted. By default
1691+
all HTTP upgrade requests are ignored (i.e. only regular `'request'` events
1692+
are emitted, sticking with the normal HTTP request/response flow) unless you
1693+
listen to this event, in which case they are all accepted (i.e. the `'upgrade'`
1694+
event is emitted instead, and future communication must handled directly
1695+
through the raw socket). You can control this more precisely by using the
1696+
server `shouldUpgradeCallback` option.
1697+
1698+
Listening to this event is optional and clients cannot insist on a protocol
1699+
change.
16871700

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

1705+
If an upgrade is accepted by `shouldUpgradeCallback` but no event handler
1706+
is registered then the socket is destroyed, resulting in an immediate
1707+
connection closure for the client.
1708+
16921709
This event is guaranteed to be passed an instance of the {net.Socket} class,
16931710
a subclass of {stream.Duplex}, unless the user specifies a socket
16941711
type other than {net.Socket}.
@@ -3537,6 +3554,9 @@ Found'`.
35373554
<!-- YAML
35383555
added: v0.1.13
35393556
changes:
3557+
- version: REPLACEME
3558+
pr-url: https://github.com/nodejs/node/pull/59824
3559+
description: The `shouldUpgradeCallback` option is now supported.
35403560
- version:
35413561
- v20.1.0
35423562
- v18.17.0
@@ -3626,6 +3646,13 @@ changes:
36263646
* `ServerResponse` {http.ServerResponse} Specifies the `ServerResponse` class
36273647
to be used. Useful for extending the original `ServerResponse`. **Default:**
36283648
`ServerResponse`.
3649+
* `shouldUpgradeCallback(request)` {Function} A callback which receives an
3650+
incoming request and returns a boolean, to control which upgrade attempts
3651+
should be accepted. Accepted upgrades will fire an `'upgrade'` event (or
3652+
their sockets will be destroyed, if no listener is registered) while
3653+
rejected upgrades will fire a `'request'` event like any non-upgrade
3654+
request. This options defaults to
3655+
`() => server.listenerCount('upgrade') > 0`.
36293656
* `uniqueHeaders` {Array} A list of response headers that should be sent only
36303657
once. If the header's value is an array, the items will be joined
36313658
using `; `.

lib/_http_server.js

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ const {
9191
validateBoolean,
9292
validateLinkHeaderValue,
9393
validateObject,
94+
validateFunction,
9495
} = require('internal/validators');
9596
const Buffer = require('buffer').Buffer;
9697
const { setInterval, clearInterval } = require('timers');
@@ -522,6 +523,16 @@ function storeHTTPOptions(options) {
522523
} else {
523524
this.rejectNonStandardBodyWrites = false;
524525
}
526+
527+
const shouldUpgradeCallback = options.shouldUpgradeCallback;
528+
if (shouldUpgradeCallback !== undefined) {
529+
validateFunction(shouldUpgradeCallback, 'options.shouldUpgradeCallback');
530+
this.shouldUpgradeCallback = shouldUpgradeCallback;
531+
} else {
532+
this.shouldUpgradeCallback = function() {
533+
return this.listenerCount('upgrade') > 0;
534+
};
535+
}
525536
}
526537

527538
function setupConnectionsTracking() {
@@ -957,15 +968,15 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
957968
parser = null;
958969

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

964975
socket.readableFlowing = null;
965976

966977
server.emit(eventName, req, socket, bodyHead);
967978
} else {
968-
// Got CONNECT method, but have no handler.
979+
// Got upgrade or CONNECT method, but have no handler.
969980
socket.destroy();
970981
}
971982
} else if (parser.incoming && parser.incoming.method === 'PRI') {
@@ -1064,7 +1075,7 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
10641075

10651076
if (req.upgrade) {
10661077
req.upgrade = req.method === 'CONNECT' ||
1067-
server.listenerCount('upgrade') > 0;
1078+
!!server.shouldUpgradeCallback(req);
10681079
if (req.upgrade)
10691080
return 2;
10701081
}
Lines 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)