Skip to content

net: fix SocketAddress.parse default port handling#62912

Open
araujogui wants to merge 3 commits intonodejs:mainfrom
araujogui:fix-socketaddress-port
Open

net: fix SocketAddress.parse default port handling#62912
araujogui wants to merge 3 commits intonodejs:mainfrom
araujogui:fix-socketaddress-port

Conversation

@araujogui
Copy link
Copy Markdown
Member

Fixes #62906

Copilot AI review requested due to automatic review settings April 23, 2026 14:47
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem. labels Apr 23, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

❌ Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.63%. Comparing base (1178926) to head (b274a64).
⚠️ Report is 2 commits behind head on main.

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   89.62%   89.63%   +0.01%     
==========================================
  Files         706      706              
  Lines      219203   219208       +5     
  Branches    41996    42003       +7     
==========================================
+ Hits       196454   196486      +32     
+ Misses      14658    14615      -43     
- Partials     8091     8107      +16     
Files with missing lines Coverage Δ
lib/internal/socketaddress.js 98.51% <93.75%> (-0.47%) ⬇️

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

needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SocketAddress parses port 80 as 0

3 participants