fix(client): reject pipelined TLS altname errors by marko1olo · Pull Request #5373 · nodejs/undici (original) (raw)

This relates to...

Fixes #5355

Rationale

ERR_TLS_CERT_ALTNAME_INVALID had a special connect-error path that asserted client[kRunning] === 0 before rejecting pending requests for the current server name. With pipelining > 1, requests can already be in the running segment when the TLS hostname validation error is reported, so the assertion can crash the process instead of rejecting the affected request promises.

Changes

Features

N/A

Bug Fixes

• drain and reject already-running requests when a TLS altname error is reported
• remove same-servername pending requests from the queue while rejecting them, preserving later pending requests for other server names
• add a regression test using a local HTTPS server and pipelined requests

Breaking Changes and Deprecations

N/A

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• Documented - not applicable; this fixes internal error handling
• Review ready
• In review
• Merge ready

Tested

• node --test test\node-test\client-tls.js
• NODE_OPTIONS=--expose-gc node --test test\tls-cert-leak.js
• node --test test\client-connect.js
• node --test --test-name-pattern servername test\node-test\client-dispatch.js
• eslint lib\dispatcher\client.js test\node-test\client-tls.js
• git diff --check