Skip to content
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

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

Closed
wants to merge 3 commits into from

Conversation

bnoordhuis
Copy link
Member

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.)

This is intended to be released in Node.js 11.

@bnoordhuis bnoordhuis added the semver-major PRs that contain breaking changes and should be released in the next major version. label May 14, 2018
@nodejs-github-bot nodejs-github-bot added the dns Issues and PRs related to the dns subsystem. label May 14, 2018
**Default:** currently `false` (addresses are reordered) but this is
expected to change in the not too distant future.
New code should use `{ verbatim: true }`.
IPv4 addresses are placed before IPv6 addresses. **Default:** `true`
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt May 14, 2018

Choose a reason for hiding this comment

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

Nit: a period after `true` for consistency with the previous option.

Copy link
Member

Choose a reason for hiding this comment

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

I guess that can also be done why landing the PR.

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 15, 2018
@BridgeAR
Copy link
Member

BridgeAR commented May 15, 2018

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

Please address the nit while landing.

@BridgeAR BridgeAR removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 18, 2018
@BridgeAR
Copy link
Member

There are lots of related CI failures.

@bnoordhuis
Copy link
Member Author

I kinda expected that, I was pretty sure the test suite would be full of assumptions about localhost. I fixed them, PTAL. I also removed common.localhostIPv4, see the commit log for rationale.

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

Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

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

LGTM if CI is green.

@BridgeAR BridgeAR added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. wip Issues and PRs that are still a work in progress. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels May 18, 2018
@jasnell
Copy link
Member

jasnell commented May 23, 2018

Needs a rebase and another CI run after.

@BridgeAR
Copy link
Member

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 29, 2018
@bnoordhuis
Copy link
Member Author

Thanks, Ruben. Looks like there are still some tests to fix up...

@BridgeAR BridgeAR removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 29, 2018
@jasnell
Copy link
Member

jasnell commented Aug 12, 2018

ping @bnoordhuis

@jasnell jasnell added the stalled Issues and PRs that are stalled. label Oct 17, 2018
@jasnell
Copy link
Member

jasnell commented Oct 17, 2018

ping @bnoordhuis

@Trott Trott added this to the 12.0.0 milestone Nov 29, 2018
treysis pushed a commit to treysis/io.js that referenced this pull request Mar 10, 2021
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
Reissue of nodejs#31567
treysis pushed a commit to treysis/io.js that referenced this pull request Mar 13, 2021
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
Reissue of nodejs#31567
treysis pushed a commit to treysis/io.js that referenced this pull request Mar 26, 2021
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
Reissue of nodejs#31567
Reissue of nodejs#37681
treysis pushed a commit to treysis/io.js that referenced this pull request Sep 3, 2021
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
Reissue of nodejs#31567
Reissue of nodejs#37681
Reissue of nodejs#37931
treysis pushed a commit to treysis/io.js that referenced this pull request Sep 3, 2021
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
Reissue of nodejs#31567
Reissue of nodejs#37681
Reissue of nodejs#37931
treysis pushed a commit to treysis/io.js that referenced this pull request Sep 3, 2021
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
Reissue of nodejs#31567
Reissue of nodejs#37681
Reissue of nodejs#37931
treysis pushed a commit to treysis/io.js that referenced this pull request Sep 3, 2021
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
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
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: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reissue of #31567
Reissue of #37681
Reissue of #37931

PR-URL: #39987
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2021
Co-authored-by: Antoine du Hamel <[email protected]>

PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
aduh95 pushed a commit that referenced this pull request Sep 12, 2021
Switch the default from `ipv4first` to `verbatim` (return them exactly
as the resolver sent them to us).

PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
aduh95 pushed a commit that referenced this pull request Sep 12, 2021
Switch the default from `ipv4first` to `verbatim` (return them exactly
as the resolver sent them to us).

PR-URL: #39987
Fixes: #31566
Refs: #6307
Refs: #20710
Refs: #38099
Co-authored-by: treysis <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dns Issues and PRs related to the dns subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. stalled Issues and PRs that are stalled. wip Issues and PRs that are still a work in progress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.