Skip to content

Commit 01e89e9

Browse files
authored
fix(client): reject pipelined TLS altname errors (#5373)
Drain already running requests before rejecting same-servername pending requests so TLS hostname failures reject the request promises instead of tripping the queue assertion. Fixes #5355 Signed-off-by: marko1olo <barsukdana@gmail.com>
1 parent d03fb24 commit 01e89e9

2 files changed

Lines changed: 52 additions & 2 deletions

File tree

lib/dispatcher/client.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -568,9 +568,15 @@ function handleConnectError (client, err, { host, hostname, protocol, port }) {
568568
}
569569

570570
if (err.code === 'ERR_TLS_CERT_ALTNAME_INVALID') {
571-
assert(client[kRunning] === 0)
571+
const running = client[kQueue].splice(client[kRunningIdx], client[kRunning])
572+
client[kPendingIdx] = client[kRunningIdx]
573+
574+
for (let i = 0; i < running.length; i++) {
575+
util.errorRequest(client, running[i], err)
576+
}
577+
572578
while (client[kPending] > 0 && client[kQueue][client[kPendingIdx]].servername === client[kServerName]) {
573-
const request = client[kQueue][client[kPendingIdx]++]
579+
const request = client[kQueue].splice(client[kPendingIdx], 1)[0]
574580
util.errorRequest(client, request, err)
575581
}
576582
} else {

test/node-test/client-tls.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
'use strict'
2+
3+
const { test } = require('node:test')
4+
const assert = require('node:assert')
5+
const { once } = require('node:events')
6+
const fs = require('node:fs')
7+
const https = require('node:https')
8+
const path = require('node:path')
9+
const { Pool } = require('../..')
10+
11+
test('TLS altname errors reject pipelined requests', async (t) => {
12+
const server = https.createServer({
13+
key: fs.readFileSync(path.join(__dirname, '..', 'fixtures', 'key.pem')),
14+
cert: fs.readFileSync(path.join(__dirname, '..', 'fixtures', 'cert.pem'))
15+
}, (req, res) => {
16+
res.end('ok')
17+
})
18+
19+
t.after(() => {
20+
server.close()
21+
})
22+
23+
server.listen(0, '127.0.0.1')
24+
await once(server, 'listening')
25+
26+
const pool = new Pool(`https://localhost:${server.address().port}`, {
27+
connections: 1,
28+
pipelining: 10,
29+
connect: {
30+
ca: fs.readFileSync(path.join(__dirname, '..', 'fixtures', 'ca.pem'))
31+
}
32+
})
33+
34+
t.after(async () => {
35+
await pool.close().catch(() => {})
36+
})
37+
38+
const settled = await Promise.allSettled(
39+
Array.from({ length: 20 }, () => pool.request({ path: '/', method: 'GET' }))
40+
)
41+
42+
assert.ok(settled.every(({ status }) => status === 'rejected'))
43+
assert.ok(settled.every(({ reason }) => reason.code === 'ERR_TLS_CERT_ALTNAME_INVALID'))
44+
})

0 commit comments

Comments
 (0)