fix(client): reject pipelined TLS altname errors

[marko1olo]

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

[mcollina]

lgtm

[codecov-commenter]

Codecov Report

❌ Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.24%. Comparing base (a8ea6f2) to head (bf90ab2).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/dispatcher/client.js	75.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5373      +/-   ##
==========================================
- Coverage   93.25%   93.24%   -0.01%     
==========================================
  Files         110      110              
  Lines       36738    36744       +6     
==========================================
+ Hits        34259    34262       +3     
- Misses       2479     2482       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[AliMahmoudDev]

makes sense. the original assert(client[kRunning] === 0) assumed no pipelining — splicing out the running requests and rejecting them before handling pending ones is the right approach. using splice for pending too avoids the index shift issue. test with 20 pipelined requests against a bad cert is a good coverage of the race.

[AliMahmoudDev]

Good catch — before this fix, running requests with a matching servername would stay in the queue and never get rejected when a TLS altname error hits. Using splice instead of incrementing the index is correct since errorRequest removes items from the queue. The test covering 20 pipelined requests all getting rejected with the right error code is solid.