net: fix SocketAddress.parse default port handling

[araujogui]

Fixes #62906

[Copilot]

Pull request overview

This PR fixes net.SocketAddress.parse() so it correctly preserves explicit default ports (notably :80) that are otherwise dropped by URL.parse() when using an http:// base, aligning behavior with expectations and documentation.

Changes:

• Update SocketAddress.parse() to derive the port from the raw input string rather than url.port.
• Simplify the parse return path to a single SocketAddress construction for both IPv4 and IPv6.
• Add regression tests covering IPv4/IPv6 with explicit ports 80 and 443.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
lib/internal/socketaddress.js	Fixes default-port handling by parsing the port from the raw input instead of URLParse(...).port.
test/parallel/test-socketaddress.js	Adds regression coverage ensuring explicit :80/:443 are preserved for both IPv4 and IPv6.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

❌ Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.14%. Comparing base (6a3d80f) to head (dc09dc4).

Files with missing lines	Patch %	Lines
lib/internal/socketaddress.js	93.75%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #62912      +/-   ##
==========================================
- Coverage   90.16%   90.14%   -0.03%     
==========================================
  Files         744      744              
  Lines      242518   242523       +5     
  Branches    45705    45691      -14     
==========================================
- Hits       218670   218623      -47     
- Misses      15357    15391      +34     
- Partials     8491     8509      +18     

Files with missing lines	Coverage Δ
lib/internal/socketaddress.js	98.51% <93.75%> (-0.47%)	⬇️

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

[nodejs-github-bot]

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

[cookesan]

I noticed one extra behavior change from parsing the port with lastIndexOf(':') and parseInt(): it can pick up a colon that belongs to userinfo rather than the host port.

For example, current main keeps these at port 0 because there is no host port:

SocketAddress.parse('user:80@5.6.7.8')
SocketAddress.parse('user:81@5.6.7.8')

With this patch they become ports 80 and 81 respectively. That may be acceptable if userinfo-shaped inputs are considered out of scope, but it is a behavior change beyond preserving explicit default host ports.

An alternative is to keep using the HTTP URL parse for the address and existing non-default ports, and only when url.port is empty ask a scheme with no default port for the port value. For example:

URLParse(`socket://${input}`).port

That preserves the existing URL host parsing while recovering explicit host ports like :80.

[araujogui]

I noticed one extra behavior change from parsing the port with lastIndexOf(':') and parseInt(): it can pick up a colon that belongs to userinfo rather than the host port.

For example, current main keeps these at port 0 because there is no host port:

SocketAddress.parse('user:80@5.6.7.8')
SocketAddress.parse('user:81@5.6.7.8')

With this patch they become ports 80 and 81 respectively. That may be acceptable if userinfo-shaped inputs are considered out of scope, but it is a behavior change beyond preserving explicit default host ports.

An alternative is to keep using the HTTP URL parse for the address and existing non-default ports, and only when url.port is empty ask a scheme with no default port for the port value. For example:

URLParse(`socket://${input}`).port

That preserves the existing URL host parsing while recovering explicit host ports like :80.

Great finding and solution, thank you @cookesan

[araujogui]

I noticed one extra behavior change from parsing the port with lastIndexOf(':') and parseInt(): it can pick up a colon that belongs to userinfo rather than the host port.
For example, current main keeps these at port 0 because there is no host port:

SocketAddress.parse('user:80@5.6.7.8')
SocketAddress.parse('user:81@5.6.7.8')

With this patch they become ports 80 and 81 respectively. That may be acceptable if userinfo-shaped inputs are considered out of scope, but it is a behavior change beyond preserving explicit default host ports.
An alternative is to keep using the HTTP URL parse for the address and existing non-default ports, and only when url.port is empty ask a scheme with no default port for the port value. For example:

URLParse(`socket://${input}`).port

That preserves the existing URL host parsing while recovering explicit host ports like :80.

Great finding and solution, thank you @cookesan

Actually, I did some benchmarks and noticed the double URL.parse solution is definitely simpler but ~25% slower than a string-based approach. So I pushed the string solution.

[github-actions]

Failed to start CI

- Validating Jenkins credentials
✔  Jenkins credentials valid
- Querying data for job/node-test-pull-request/73051/
[SyntaxError: Unexpected token '<', ..."    
  https://github.com/nodejs/node/actions/runs/30286474954

[Renegade334]

The purpose of this method is stated as Parse an "${ip}:${port}" formatted string into a SocketAddress. Surely we want to be rejecting arbitrary URL components anyway?

[araujogui]

Agreed. Reverted the userinfo handling and its tests.

[araujogui]

We can reject the other URL components in a follow-up semver-major PR.