net: enforce strict socket address formats in SocketAddress.parse

[araujogui]

SocketAddress.parse() no longer builds a http://${input} URL and reads the hostname and port back out of the Ada WHATWG URL parser. The input is now split in C++ into host and port, and the host is validated by uv_inet_pton() — the same check the SocketAddress constructor already used.

The Ada URL parser is deliberately lenient about hosts, so parse() accepted much that is not a socket address: legacy IPv4 forms (0177.0.0.1, 0x7f.0.0.1, 2130706433, 127.1), URL syntax (user@1.2.3.4:80, 1.2.3.4:80/foo), and input it rewrites first, such as embedded tabs and non-ASCII digits that IDNA maps to ASCII. Those legacy forms are the ambiguity behind CVE-2021-29923 and CVE-2021-29922. It also disagreed with the constructor, which rejects 0177.0.0.1 with ERR_INVALID_ADDRESS.

Separately, SocketAddress.parse('1.2.3.4:80') returned port 0, because the URL parser drops a port equal to the scheme's default. It now returns 80, fixes #62906.

The accepted grammar is documented in doc/api/net.md. I left IPv6 zone ids out of it: sin6_scope_id isn't exposed on the JS object, so a scoped address can't round trip through its own JSON, and resolving an interface name would mean the result depends on which machine did the parsing.

[nodejs-github-bot]

Review requested:

• @nodejs/net

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Renegade334]

This would make the port issue much simpler with this additional validation. If the string ends in /:(\d+)$/, then it can just be sliced off, and if the rest of the string gets parsed as a valid hostname by URLParse then the port can be obtained from the digit string with NumberParseInt.

[codecov]

Codecov Report

❌ Patch coverage is 95.37037% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.16%. Comparing base (0de4fcc) to head (45c3368).

Files with missing lines	Patch %	Lines
src/node_sockaddr.cc	76.47%	0 Missing and 4 partials ⚠️
src/node_sockaddr_parser.cc	98.63%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #64832      +/-   ##
==========================================
- Coverage   90.17%   90.16%   -0.01%     
==========================================
  Files         771      772       +1     
  Lines      265470   265542      +72     
  Branches    50463    50487      +24     
==========================================
+ Hits       239383   239426      +43     
+ Misses      17055    17048       -7     
- Partials     9032     9068      +36     

Files with missing lines	Coverage Δ
lib/internal/socketaddress.js	98.88% <100.00%> (-0.11%)	⬇️
src/node_sockaddr.h	51.28% <ø> (ø)
src/node_sockaddr_parser.cc	98.63% <98.63%> (ø)
src/node_sockaddr.cc	74.84% <76.47%> (+0.03%)	⬆️

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

[Renegade334]

LGTM!

[nodejs-github-bot]

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

[araujogui]

@Renegade334 maybe we should include semver-major tag?

[nodejs-github-bot]

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

[jasnell]

I'm not yet convinced this is the best way to approach this (with the regex). Won't block for now but there's probably a more performant / reliable way.

[araujogui]

I'm not yet convinced this is the best way to approach this (with the regex). Won't block for now but there's probably a more performant / reliable way.

@jasnell I implemented a new strict socket address parser replacing Ada, it fixes #62906 and is faster. Benchmark results compared to main:

                                                           confidence improvement accuracy (*)   (**)  (***)
net/net-socketaddress-parse.js input='ipv4-port' n=1000000        ***     52.37 %       ±0.63% ±0.84% ±1.10%
net/net-socketaddress-parse.js input='ipv4' n=1000000             ***     43.38 %       ±0.64% ±0.86% ±1.12%
net/net-socketaddress-parse.js input='ipv6-port' n=1000000        ***     59.56 %       ±0.49% ±0.65% ±0.85%
net/net-socketaddress-parse.js input='ipv6' n=1000000             ***     47.35 %       ±0.50% ±0.66% ±0.86%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 4 comparisons, you can thus expect the following amount of false-positive results:
  0.20 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.04 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

[github-actions]

Benchmark GHA (net / net-socketaddress-parse): https://github.com/nodejs/node/actions/runs/33605572788

Results

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

Benchmark results:

                                                           confidence improvement accuracy (*)    (**)   (***)
net/net-socketaddress-parse.js input='ipv4-port' n=1000000        ***     29.96 %      ±10.23% ±13.49% ±17.31%
net/net-socketaddress-parse.js input='ipv4' n=1000000             ***     34.36 %      ±10.45% ±13.78% ±17.68%
net/net-socketaddress-parse.js input='ipv6-port' n=1000000        ***     28.40 %      ±10.07% ±13.27% ±17.03%
net/net-socketaddress-parse.js input='ipv6' n=1000000             ***     31.02 %      ±10.04% ±13.24% ±16.99%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 4 comparisons, you can thus
expect the following amount of false-positive results:
  0.20 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.04 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.