Skip to content

dns: validate address type in lookupService - #64878

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
lazerg:fix/issue-64877-lookupservice-buffer-abort
Aug 21, 2026
Merged

dns: validate address type in lookupService#64878
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
lazerg:fix/issue-64877-lookupservice-buffer-abort

Conversation

@lazerg

@lazerg lazerg commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

dns.lookupService() and dns.promises.lookupService() abort the process with a native assertion when address is a non-string that still coerces to a valid IP, like a Buffer. isIP() stringifies its argument, so the existing guard passes and the raw value reaches the C++ binding, where it trips CHECK(args[1]->IsString()). Validating that address is a string first, as dgram already does, makes a bad type throw ERR_INVALID_ARG_TYPE instead of crashing.

Fixes: #64877

Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net

@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 Jul 31, 2026
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.17%. Comparing base (67af6d1) to head (f6b29e6).
⚠️ Report is 352 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #64878   +/-   ##
=======================================
  Coverage   90.16%   90.17%           
=======================================
  Files         746      746           
  Lines      242763   242768    +5     
  Branches    45761    45768    +7     
=======================================
+ Hits       218898   218921   +23     
+ Misses      15352    15321   -31     
- Partials     8513     8526   +13     
Files with missing lines Coverage Δ
lib/dns.js 98.66% <100.00%> (+0.01%) ⬆️
lib/internal/dns/promises.js 98.07% <100.00%> (+<0.01%) ⬆️

... and 35 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.

@theanarkh theanarkh added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 1, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 1, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikr trivikr added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 21, 2026
@nodejs-github-bot
nodejs-github-bot merged commit da2d617 into nodejs:main Aug 21, 2026
80 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in da2d617

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 21, 2026
aduh95 pushed a commit that referenced this pull request Aug 25, 2026
Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com>
PR-URL: #64878
Fixes: #64877
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 25, 2026
Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com>
PR-URL: #64878
Fixes: #64877
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
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.

dns.lookupService triggers a SIGABRT when passed a Buffer

5 participants