Skip to content

fix(ext/node): support binding to localAddress #28779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

littledivy
Copy link
Member

@littledivy littledivy commented Apr 7, 2025

Fixes #24153

Enables parallel/test-http-localaddress.js and parallel/test-https-localaddress.js on Linux CI. These tests were skip-passing earlier.

$ /target/debug/deno test --config tests/config/deno.json -A tests/node_compat/test.ts -- parallel/test-http-localaddress.js
running 1 test from ./tests/node_compat/test.ts
Node.js compatibility ...
  Node.js compatibility "parallel/test-http-localaddress.js" ...
------- output -------
Connect from: 127.0.0.2

----- output end -----
  Node.js compatibility "parallel/test-http-localaddress.js" ... ok (75ms)
Node.js compatibility ... ok (81ms)

@littledivy littledivy requested a review from Copilot April 7, 2025 11:40
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

tests/node_compat/test/common/index.js:488

  • Ensure 'isLinux' is defined in this scope or imported, as it is used to determine multi-localhost support.
return isLinux;

ext/net/ops.rs:459

  • Using the remote port as a fallback for local_port may unintentionally cause port conflicts; please verify if this fallback is intended.
resolve_addr(&local_addr, addr.local_port.unwrap_or(addr.port))

Comment on lines +351 to 352
// bind is defered during socket creation. See #connect().

Copy link
Preview

Copilot AI Apr 7, 2025

Choose a reason for hiding this comment

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

Typo in comment: 'defered' should be 'deferred'.

Suggested change
// bind is defered during socket creation. See #connect().
// bind is deferred during socket creation. See #connect().

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

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.

Node:http localAddress not supported in compat
1 participant