Skip to content

Commit 384612e

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

2 files changed

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

0 commit comments

Comments
 (0)