Skip to content

dns: return localhost IP directly#63368

Closed
LiviaMedeiros wants to merge 1 commit into
nodejs:mainfrom
LiviaMedeiros:dns-local
Closed

dns: return localhost IP directly#63368
LiviaMedeiros wants to merge 1 commit into
nodejs:mainfrom
LiviaMedeiros:dns-local

Conversation

@LiviaMedeiros
Copy link
Copy Markdown
Member

Fixes: #44804
Fixes: #54111
Not fully fixes yet: #50871
Partially related to: #44003

As per RFC 6761 6.3,

  1. Application software MAY recognize localhost names as special, or
    MAY pass them to name resolution APIs as they would for other
    domain names.
  1. Name resolution APIs and libraries SHOULD recognize localhost
    names as special and SHOULD always return the IP loopback address
    for address queries and negative responses for all other query
    types. Name resolution APIs SHOULD NOT send queries for
    localhost names to their configured caching DNS server(s).

I think we should recognize localhost. zone as special and return immediately, without relying on getaddrinfo(3), /etc/hosts, etc..

Signed-off-by: LiviaMedeiros <livia@cirno.name>
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net
  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added dns Issues and PRs related to the dns subsystem. needs-ci PRs that need a full CI run. labels May 16, 2026
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

We can't return an ipv6 address if we don't have the matching interface.

Also, this can be customized by the host, and I think it would be better
to leave as is.

@LiviaMedeiros
Copy link
Copy Markdown
Member Author

We can't return an ipv6 address if we don't have the matching interface.

Agreed. I think something similar can also be done within net subsystem or libuv; if low-enough-cost check for IPv6 support is feasible there.

We can also make fast-path exclusively for cases where we must return only IPv4 (which would add inconsistency, i'd be -1 myself).

Also, this can be customized by the host, and I think it would be better
to leave as is.

Not sure how spec-compliant customizations are... But if it might break on some systems, probably not worth changing it.


Closing this. For future references, proper check here would be something closer to RegExpPrototypeExec(/^(?:.+\.)?localhost\.?$/i, hostname)

@codecov
Copy link
Copy Markdown

codecov Bot commented May 16, 2026

Codecov Report

❌ Patch coverage is 84.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.04%. Comparing base (a6696e2) to head (86d24c8).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
lib/dns.js 86.66% 1 Missing and 1 partial ⚠️
lib/internal/dns/promises.js 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63368      +/-   ##
==========================================
- Coverage   90.06%   90.04%   -0.02%     
==========================================
  Files         714      714              
  Lines      225522   225589      +67     
  Branches    42636    42662      +26     
==========================================
+ Hits       203110   203128      +18     
- Misses      14196    14259      +63     
+ Partials     8216     8202      -14     
Files with missing lines Coverage Δ
lib/dns.js 98.17% <86.66%> (-0.47%) ⬇️
lib/internal/dns/promises.js 97.85% <80.00%> (-0.44%) ⬇️

... and 45 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test-net-socket-connect-without-cb test fails Error running tests on HEAD

3 participants