Skip to content

dns: default to verbatim=true in dns.lookup() - #39987

Closed
treysis wants to merge 18 commits into
nodejs:masterfrom
treysis:verbatim-true-v17
Closed

dns: default to verbatim=true in dns.lookup()#39987
treysis wants to merge 18 commits into
nodejs:masterfrom
treysis:verbatim-true-v17

Conversation

@treysis

@treysistreysis commented Sep 3, 2021

Copy link
Copy Markdown
Contributor

Switch the default from false (reorder the result so that IPv4 addresses
come before IPv6 addresses) to true (return them exactly as the resolver
sent them to us.)

After having the option to manually override the default behavior, this is another attempt at changing the default behavior.

Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reissue of #31567
Reissue of #37681
Reissue of #37931

@nodejs-github-botnodejs-github-bot added dns Issues and PRs related to the dns subsystem. needs-ci PRs that need a full CI run. labels Sep 3, 2021
@treysis
treysisforce-pushed the verbatim-true-v17 branch 4 times, most recently from 879d1ff to e37e2beCompareSeptember 3, 2021 20:40
bnoordhuisand others added 2 commits September 3, 2021 22:43
Switch the default from false (reorder the result so that IPv4 addresses
come before IPv6 addresses) to true (return them exactly as the resolver
sent them to us.)
Fixes: nodejs#31566
Refs: nodejs#6307
Refs: nodejs#20710
Refs: nodejs#38099
Reissue of nodejs#31567
Reissue of nodejs#37681
Reissue of nodejs#37931
Comment threadlib/internal/dns/utils.js Outdated
@aduh95aduh95 added the semver-major PRs that contain breaking changes and should be released in the next major version. label Sep 3, 2021
@treysis

Copy link
Copy Markdown
ContributorAuthor

Now again we have some failing tests. Do you think I should remove where appropriate to listen on 127.0.0.1 an just listen "generally"? Or force listening on 127.0.0.1 and force connect on 127.0.0.1?

@treysis

Copy link
Copy Markdown
ContributorAuthor

Oooh, my careless mistakes. Let's hope it turns green after this run so we can spin up a CI job.

@treysis

Copy link
Copy Markdown
ContributorAuthor

Alright, can we get a CI @aduh95 ?

@aduh95aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2021
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2021
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/39778/

@treysis

treysis commented Sep 5, 2021

Copy link
Copy Markdown
ContributorAuthor

Can we get a CI already before passing the other checks? Because I just need to see why some stuff is failing on CI @aduh95
I'll fix the linter errors before, though.

@BrandonGillisBrandonGillis mentioned this pull request Dec 5, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.dnsIssues and PRs related to the dns subsystem.semver-majorPRs that contain breaking changes and should be released in the next major version.tsc-agendaIssues and PRs to discuss during the meetings of the TSC.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dns: default to verbatim=true in dns.lookup()

9 participants

@treysis@nodejs-github-bot@aduh95@mcollina@starball5@jasnell@mhdawson@VoltrexKeyva@bnoordhuis