Skip to content

Fix handling default ports from URL#1490

Closed
w666 wants to merge 1 commit intomasterfrom
fix-1488-url-port-parse
Closed

Fix handling default ports from URL#1490
w666 wants to merge 1 commit intomasterfrom
fix-1488-url-port-parse

Conversation

@w666
Copy link
Copy Markdown
Collaborator

@w666 w666 commented May 8, 2026

Close #1488

Summary by CodeRabbit

  • Bug Fixes

    • Fixed HTTP client to exclude default ports (80 for HTTP, 443 for HTTPS) from Host headers when not explicitly specified in URLs
    • Explicit port numbers are properly preserved in Host headers when included
  • Tests

    • Added test coverage for Host header formatting behavior

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: bcfad726-ba85-4a5e-9373-4786f143ecaa

📥 Commits

Reviewing files that changed from the base of the PR and between 2105141 and ce75134.

📒 Files selected for processing (2)
  • src/http.ts
  • test/client-customHttp-test.js

📝 Walkthrough

Walkthrough

The HTTP client's Host header construction is refactored to explicitly extract numeric ports from raw URL strings and exclude protocol defaults (80/443). The port-extraction helper is rewritten to parse the URL authority directly with IPv6 bracket support, then integrated into request building and validated with test cases.

Changes

Host Header Port Handling

Layer / File(s) Summary
Port Extraction Utility
src/http.ts
getPortFromUrl accepts a raw URL string, parses the authority section directly, handles bracketed IPv6 literals, and returns only explicit numeric ports or an empty string instead of inferring protocol defaults.
Host Header Construction
src/http.ts
HttpClient.buildRequest is updated to compute the Host header by combining the parsed hostname with the explicit port extracted from the raw rurl via the refactored getPortFromUrl, removing reliance on protocol-default port inference.
Test Coverage
test/client-customHttp-test.js
New unit test should not append default ports to Host header verifies that both https://example.com and http://example.com produce Host: example.com, while https://example.com:8443 preserves the explicit port.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A port by any other name should not default,
Explicit numbers shine, the implicit fault.
IPv6 brackets hugged, headers now clean,
The finest Host header ever seen!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The addition of index.js (a new SOAP client test script) is out of scope relative to the linked issue #1488, which targets fixing URL port handling in the HTTP client. Remove the index.js file or provide justification for its inclusion. The HTTP port handling fix should be the sole focus of this PR.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix handling default ports from URL' directly relates to the main change: rewriting port extraction logic in src/http.ts to handle explicit ports correctly.
Linked Issues check ✅ Passed The PR addresses the regression in issue #1488 by fixing URL port parsing in src/http.ts to extract explicit ports correctly and avoid appending default ports, which resolves the 'Invalid WSDL URL' error.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-1488-url-port-parse

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@w666 w666 force-pushed the fix-1488-url-port-parse branch from 2105141 to ce75134 Compare May 8, 2026 21:34
@w666 w666 closed this May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid WSDL URL when createClientAsync

2 participants