Skip to content

fix(miniflare): skip SNI for Hyperdrive IP targets#13708

Merged
edmundhung merged 1 commit intomainfrom
edmundhung/fix-hyperdrive-sni-ip
Apr 28, 2026
Merged

fix(miniflare): skip SNI for Hyperdrive IP targets#13708
edmundhung merged 1 commit intomainfrom
edmundhung/fix-hyperdrive-sni-ip

Conversation

@edmundhung
Copy link
Copy Markdown
Member

@edmundhung edmundhung commented Apr 28, 2026

Fixes #13599 .

The hyperdrive test fails in Node 25 as it has a stricter validation on servername and fails if it is an IP address.

No changeset as the corresponding change is not yet released.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because: covered by existing test
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: bugfix

A picture of a cute animal (not mandatory, but encouraged)


Open in Devin Review

@edmundhung edmundhung added no-changeset-required test all node versions Run the tests on all Node versions for this PR labels Apr 28, 2026
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 28, 2026

⚠️ No Changeset found

Latest commit: e1564c3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk Apr 28, 2026
@workers-devprod workers-devprod requested review from a team and petebacondarwin and removed request for a team April 28, 2026 11:41
@workers-devprod
Copy link
Copy Markdown
Contributor

workers-devprod commented Apr 28, 2026

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 28, 2026

UnknownError: ProviderInitError

github run

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 28, 2026

@edmundhung Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 28, 2026

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@13708

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@13708

miniflare

npm i https://pkg.pr.new/miniflare@13708

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@13708

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@13708

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@13708

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@13708

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@13708

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@13708

wrangler

npm i https://pkg.pr.new/wrangler@13708

commit: e1564c3

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 28, 2026

The fix is correct. net.isIP() returns 0 for non-IP strings (hostnames), 4 for IPv4, and 6 for IPv6. The condition net.isIP(targetHost) === 0 properly gates servername to only be set for hostnames.

This aligns with the TLS/SNI specification (RFC 6066 section 3), which states that SNI HostName must be a DNS hostname, not an IP address. Node.js 25 enforces this more strictly, causing the test failure described in the linked issue.

The net module is already imported at line 2, so no new dependency is introduced. The change is minimal and well-targeted.

LGTM

github run

Copy link
Copy Markdown
Contributor

@workers-devprod workers-devprod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codeowners reviews satisfied

@edmundhung edmundhung merged commit 3f39c7e into main Apr 28, 2026
65 of 76 checks passed
@edmundhung edmundhung deleted the edmundhung/fix-hyperdrive-sni-ip branch April 28, 2026 12:04
@github-project-automation github-project-automation Bot moved this from Untriaged to Done in workers-sdk Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changeset-required test all node versions Run the tests on all Node versions for this PR

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants