Skip to content

Commit 3c99a4d

Browse files
pimterrydanielleadams
authored andcommitted
http2: handle existing socket data when creating HTTP/2 server sessions
When emitting a 'connection' event to manually inject connections into a server, it's common for the provided stream to already contain readable data, e.g. after sniffing a connection to detect HTTP/2 from the initial bytes. Previously this was supported only for outgoing HTTP/2 sessions created with http2.connect(). This change ensures that HTTP/2 over existing streams is supported on both outgoing and incoming sessions. PR-URL: #41185 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent f3b0dfc commit 3c99a4d

File tree

2 files changed

+87
-15
lines changed

2 files changed

+87
-15
lines changed

lib/internal/http2/core.js

+15-15
Original file line numberDiff line numberDiff line change
@@ -1257,6 +1257,21 @@ class Http2Session extends EventEmitter {
12571257
this.on('newListener', sessionListenerAdded);
12581258
this.on('removeListener', sessionListenerRemoved);
12591259

1260+
// Process data on the next tick - a remoteSettings handler may be attached.
1261+
// https://github.com/nodejs/node/issues/35981
1262+
process.nextTick(() => {
1263+
// Socket already has some buffered data - emulate receiving it
1264+
// https://github.com/nodejs/node/issues/35475
1265+
// https://github.com/nodejs/node/issues/34532
1266+
if (socket.readableLength) {
1267+
let buf;
1268+
while ((buf = socket.read()) !== null) {
1269+
debugSession(type, `${buf.length} bytes already in buffer`);
1270+
this[kHandle].receive(buf);
1271+
}
1272+
}
1273+
});
1274+
12601275
debugSession(type, 'created');
12611276
}
12621277

@@ -3268,21 +3283,6 @@ function connect(authority, options, listener) {
32683283
if (typeof listener === 'function')
32693284
session.once('connect', listener);
32703285

3271-
// Process data on the next tick - a remoteSettings handler may be attached.
3272-
// https://github.com/nodejs/node/issues/35981
3273-
process.nextTick(() => {
3274-
debug('Http2Session connect', options.createConnection);
3275-
// Socket already has some buffered data - emulate receiving it
3276-
// https://github.com/nodejs/node/issues/35475
3277-
if (socket && socket.readableLength) {
3278-
let buf;
3279-
while ((buf = socket.read()) !== null) {
3280-
debug(`Http2Session connect: ${buf.length} bytes already in buffer`);
3281-
session[kHandle].receive(buf);
3282-
}
3283-
}
3284-
});
3285-
32863286
return session;
32873287
}
32883288

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
'use strict';
2+
const common = require('../common');
3+
if (!common.hasCrypto)
4+
common.skip('missing crypto');
5+
6+
const assert = require('assert');
7+
const net = require('net');
8+
const http = require('http');
9+
const http2 = require('http2');
10+
11+
// Example test for HTTP/1 vs HTTP/2 protocol autoselection.
12+
// Refs: https://github.com/nodejs/node/issues/34532
13+
14+
const h1Server = http.createServer(common.mustCall((req, res) => {
15+
res.end('HTTP/1 Response');
16+
}));
17+
18+
const h2Server = http2.createServer(common.mustCall((req, res) => {
19+
res.end('HTTP/2 Response');
20+
}));
21+
22+
const rawServer = net.createServer(common.mustCall(function listener(socket) {
23+
const data = socket.read(3);
24+
25+
if (!data) { // Repeat until data is available
26+
socket.once('readable', () => listener(socket));
27+
return;
28+
}
29+
30+
// Put the data back, so the real server can handle it:
31+
socket.unshift(data);
32+
33+
if (data.toString('ascii') === 'PRI') { // Very dumb preface check
34+
h2Server.emit('connection', socket);
35+
} else {
36+
h1Server.emit('connection', socket);
37+
}
38+
}, 2));
39+
40+
rawServer.listen(common.mustCall(() => {
41+
const { port } = rawServer.address();
42+
43+
let done = 0;
44+
{
45+
// HTTP/2 Request
46+
const client = http2.connect(`http://localhost:${port}`);
47+
const req = client.request({ ':path': '/' });
48+
req.end();
49+
50+
let content = '';
51+
req.setEncoding('utf8');
52+
req.on('data', (chunk) => content += chunk);
53+
req.on('end', common.mustCall(() => {
54+
assert.strictEqual(content, 'HTTP/2 Response');
55+
if (++done === 2) rawServer.close();
56+
client.close();
57+
}));
58+
}
59+
60+
{
61+
// HTTP/1 Request
62+
http.get(`http://localhost:${port}`, common.mustCall((res) => {
63+
let content = '';
64+
res.setEncoding('utf8');
65+
res.on('data', (chunk) => content += chunk);
66+
res.on('end', common.mustCall(() => {
67+
assert.strictEqual(content, 'HTTP/1 Response');
68+
if (++done === 2) rawServer.close();
69+
}));
70+
}));
71+
}
72+
}));

0 commit comments

Comments
 (0)