Skip to content

Commit 939084a

Browse files
committed
fix: reset invalid HTTP/2 sessions
1 parent 81fdf87 commit 939084a

2 files changed

Lines changed: 203 additions & 9 deletions

File tree

lib/dispatcher/client-h2.js

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,29 @@ function getGoAwayError (session, errorCode) {
8181
: new SocketError(`HTTP/2: "GOAWAY" frame received with code ${errorCode}`, util.getSocketInfo(session[kSocket])))
8282
}
8383

84+
function resetHttp2Session (session, err) {
85+
const client = session[kClient]
86+
const socket = session[kSocket]
87+
88+
if (client[kHTTP2Session] === session) {
89+
client[kSocket] = null
90+
client[kHTTPContext] = null
91+
client[kHTTP2Session] = null
92+
}
93+
94+
if (socket != null && socket[kError] == null) {
95+
socket[kError] = err
96+
}
97+
98+
if (!session.closed && !session.destroyed) {
99+
try {
100+
session.destroy(err)
101+
} catch {}
102+
}
103+
104+
util.destroy(socket, err)
105+
}
106+
84107
function getGoAwayPendingIdx (client, lastStreamID) {
85108
const maxAcceptedStreamID = Number.isInteger(lastStreamID) ? lastStreamID : Number.MAX_SAFE_INTEGER
86109

@@ -122,6 +145,10 @@ function clearRequestStream (request) {
122145
cleanup?.(stream)
123146
}
124147

148+
function requeueUnsentRequest (client, request) {
149+
client[kQueue].splice(client[kPendingIdx] + 1, 0, request)
150+
}
151+
125152
function canRetryRequestAfterGoAway (request) {
126153
const { body } = request
127154

@@ -736,19 +763,29 @@ function writeH2 (client, request) {
736763
try {
737764
return session.request(headers, options)
738765
} catch (err) {
739-
if (err?.code !== 'ERR_HTTP2_INVALID_CONNECTION_HEADERS') {
740-
throw err
766+
if (err?.code === 'ERR_HTTP2_INVALID_CONNECTION_HEADERS') {
767+
const wrappedErr = new InformationalError(err.message, { cause: err })
768+
session[kError] = wrappedErr
769+
session[kSocket][kError] = wrappedErr
770+
771+
session.destroy(wrappedErr)
772+
util.destroy(session[kSocket], wrappedErr)
773+
abort(wrappedErr)
774+
775+
return null
741776
}
742777

743-
const wrappedErr = new InformationalError(err.message, { cause: err })
744-
session[kError] = wrappedErr
745-
session[kSocket][kError] = wrappedErr
778+
if (err?.code === 'ERR_HTTP2_INVALID_SESSION') {
779+
const wrappedErr = new SocketError(err.message, util.getSocketInfo(session[kSocket]))
780+
wrappedErr.cause = err
781+
session[kError] = wrappedErr
782+
resetHttp2Session(session, wrappedErr)
783+
requeueUnsentRequest(client, request)
746784

747-
session.destroy(wrappedErr)
748-
util.destroy(session[kSocket], wrappedErr)
749-
abort(wrappedErr)
785+
return null
786+
}
750787

751-
return null
788+
throw err
752789
}
753790
}
754791

test/http2-invalid-session.js

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
'use strict'
2+
3+
const { test, after } = require('node:test')
4+
const { EventEmitter, once } = require('node:events')
5+
const { createSecureServer } = require('node:http2')
6+
const { tspl } = require('@matteo.collina/tspl')
7+
8+
const pem = require('@metcoder95/https-pem')
9+
10+
const { Client } = require('..')
11+
12+
test('invalid HTTP/2 session stream creation is requeued on a fresh session', async (t) => {
13+
t = tspl(t, { plan: 5 })
14+
15+
const http2 = require('node:http2')
16+
const originalConnect = http2.connect
17+
18+
class FakeSession extends EventEmitter {
19+
constructor () {
20+
super()
21+
this.closed = false
22+
this.destroyed = false
23+
}
24+
25+
request () {
26+
const err = new Error('The session has been destroyed')
27+
err.code = 'ERR_HTTP2_INVALID_SESSION'
28+
throw err
29+
}
30+
31+
destroy () {
32+
if (this.destroyed) {
33+
return
34+
}
35+
36+
this.destroyed = true
37+
this.emit('close')
38+
}
39+
40+
ref () {}
41+
unref () {}
42+
}
43+
44+
const session = new FakeSession()
45+
let connectCalls = 0
46+
let streams = 0
47+
48+
http2.connect = function connectStub (...args) {
49+
connectCalls++
50+
51+
if (connectCalls === 1) {
52+
return session
53+
}
54+
55+
return originalConnect.apply(this, args)
56+
}
57+
58+
after(() => {
59+
http2.connect = originalConnect
60+
})
61+
62+
const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } }))
63+
server.on('stream', (stream) => {
64+
streams++
65+
stream.respond({ ':status': 200 })
66+
stream.end('ok')
67+
})
68+
69+
after(() => server.close())
70+
await once(server.listen(0), 'listening')
71+
72+
const client = new Client(`https://localhost:${server.address().port}`, {
73+
allowH2: true,
74+
connect: {
75+
rejectUnauthorized: false
76+
}
77+
})
78+
after(() => client.close())
79+
80+
const response = await client.request({ path: '/', method: 'GET' })
81+
const chunks = []
82+
response.body.on('data', chunk => {
83+
chunks.push(chunk)
84+
})
85+
await once(response.body, 'end')
86+
87+
t.strictEqual(response.statusCode, 200)
88+
t.strictEqual(Buffer.concat(chunks).toString(), 'ok')
89+
t.strictEqual(streams, 1)
90+
t.strictEqual(connectCalls, 2)
91+
t.strictEqual(session.destroyed, true)
92+
93+
await t.completed
94+
})
95+
96+
test('truncated HTTP/2 server session resets the client session', async (t) => {
97+
t = tspl(t, { plan: 4 })
98+
99+
const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } }))
100+
const serverSockets = []
101+
let streams = 0
102+
103+
server.on('secureConnection', (socket) => {
104+
socket.on('error', () => {})
105+
serverSockets.push(socket)
106+
})
107+
server.on('sessionError', () => {})
108+
server.on('tlsClientError', () => {})
109+
110+
server.on('stream', (stream) => {
111+
streams++
112+
113+
if (streams === 1) {
114+
stream.respond({ ':status': 200 })
115+
stream.write('partial', () => {
116+
setImmediate(() => {
117+
serverSockets[0].destroy()
118+
})
119+
})
120+
return
121+
}
122+
123+
stream.respond({ ':status': 200 })
124+
stream.end('ok')
125+
})
126+
127+
after(() => server.close())
128+
await once(server.listen(0), 'listening')
129+
130+
const client = new Client(`https://localhost:${server.address().port}`, {
131+
allowH2: true,
132+
connect: {
133+
rejectUnauthorized: false
134+
},
135+
maxConcurrentStreams: 1
136+
})
137+
after(() => client.close())
138+
139+
const first = await client.request({ path: '/', method: 'GET' })
140+
141+
first.body.resume()
142+
const [err] = await once(first.body, 'error')
143+
t.ok(err.code === 'UND_ERR_SOCKET' || err.code === 'ECONNRESET')
144+
145+
const response = await client.request({ path: '/second', method: 'GET' })
146+
const chunks = []
147+
response.body.on('data', chunk => {
148+
chunks.push(chunk)
149+
})
150+
await once(response.body, 'end')
151+
152+
t.strictEqual(response.statusCode, 200)
153+
t.strictEqual(Buffer.concat(chunks).toString(), 'ok')
154+
t.strictEqual(streams, 2)
155+
156+
await t.completed
157+
})

0 commit comments

Comments
 (0)