Skip to content

test(node): connect http2 tests via localhost for TLS cert match#211

Merged
pi0 merged 1 commit into
mainfrom
fix/node26-http2-tls-localhost
Jun 19, 2026
Merged

test(node): connect http2 tests via localhost for TLS cert match#211
pi0 merged 1 commit into
mainfrom
fix/node26-http2-tls-localhost

Conversation

@pi0

@pi0 pi0 commented Jun 19, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Tests
    • Updated internal test infrastructure to improve HTTP/2 certificate validation handling.

Node 26 no longer matches a bare IPv6 literal host (`::1` from
`server.url`) against the test cert's IP altnames, breaking the http2
TLS check. Connect via the `localhost` DNS altname instead, which works
across all Node versions.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b9a1d97e-958c-4cdf-b5f5-72d274abbdce

📥 Commits

Reviewing files that changed from the base of the PR and between e95ca51 and 8be3690.

📒 Files selected for processing (1)
  • test/node.test.ts

📝 Walkthrough

Walkthrough

In test/node.test.ts, a url(path) function is added to the harness object passed to addTests. For HTTP/2 runs, it parses server.url, replaces the hostname with localhost, and appends the test path; non-HTTP/2 runs continue using the original server.url string concatenation.

HTTP/2 TLS URL Fix

Layer / File(s) Summary
HTTP/2 test harness URL override
test/node.test.ts
A url(path) override is added to the addTests harness. When config.http2 is enabled, server.url is parsed and its hostname is replaced with localhost before appending the path, ensuring TLS certificate DNS alt name matching works. Non-HTTP/2 runs are unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A rabbit once tunneled through TLS with fear,
The hostname it brought was a number, oh dear!
Just swap it for localhost, the cert will agree,
HTTP/2 now hops along merrily free.
No more cert mismatch — what a fix, yippee! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: modifying HTTP/2 tests to connect via localhost for TLS certificate matching.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/node26-http2-tls-localhost

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@pkg-pr-new

pkg-pr-new Bot commented Jun 19, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/srvx@211

commit: 8be3690

@pi0 pi0 merged commit 0a32618 into main Jun 19, 2026
15 checks passed
@pi0 pi0 deleted the fix/node26-http2-tls-localhost branch June 19, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant