fix(ext/node): fix dns.lookup result ordering #26264
Merged
+37
−40
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kt3k
force-pushed
the
fix-dns-resolt-order
branch
from
0814e2e
to
19f4385
Compare
kt3k
force-pushed
the
fix-dns-resolt-order
branch
from
a0d2ced
to
d65fee7
Compare
kt3k
force-pushed
the
fix-dns-resolt-order
branch
from
e51a8d1
to
9257b70
Compare
satyarohith
reviewed
Oct 16, 2024
tests/unit_node/http_test.ts
Outdated
| @@ -314,7 +314,7 @@ Deno.test("[node/http] IncomingRequest socket has remoteAddress + remotePort", a | |||
| remotePort = req.socket.remotePort; | |||
| res.end(); | |||
| }); | |||
| server.listen(async () => { | |||
| server.listen({ host: "0.0.0.0" }, async () => { | |||
There was a problem hiding this comment.
Can we remove the host and update the request instead to make sure ipv6 is working as expected?
There was a problem hiding this comment.
Sure. In that case we need to update assertion part instead, as server.listen (without host specified) listens on ipv6 (::1) on unix and ipv4 (0.0.0.0) on windows (I think those are the expected behaviors). See 7def448
kt3k
added a commit
to kt3k/deno
that referenced
this pull request
Oct 29, 2024
This reverts commit d59599f.
bartlomieju
pushed a commit
that referenced
this pull request
Oct 29, 2024
bartlomieju
pushed a commit
that referenced
this pull request
Oct 29, 2024
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
partially unblocks #25470
This PR aligns the resolution of
localhosthostname to Node.js behavior.In Node.js
dns.lookup("localhost", (_, addr) => console.log(addr))prints ipv6 address::1, but it prints ipv4 address127.0.0.1in Deno. That difference causes some errors in the work of enablingcreateConnectionoption inhttp.request(#25470). This PR fixes the issue by aligningdns.lookupbehavior to Node.js.This PR also changes the following behaviors (resolving TODOs):
http.createServernow listens on ipv6 address[::]by default on linux/macnet.createServernow listens on ipv6 address[::]by default on linux/macThese changes are also alignments to Node.js behaviors.
ref: nodejs/node#39987