[relay] Preserve non-standard port in WS dialer URL prep by lixmal · Pull Request #6061 · netbirdio/netbird

lixmal · 2026-05-04T09:31:24Z

Describe your changes

Rewrite prepareURL in the relay WS dialer to use net/url parsing instead of a strings.Replace based scheme rewrite, and add tests covering non-standard ports and other edge cases.

Parse the relay address with url.Parse, switch on the scheme, and set the path explicitly
Add table-driven tests for ports, IPs, hostnames starting with rel, and rejected schemes

Issue ticket number and link

#6057

Stack

Checklist

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

I added/updated documentation for this change
Documentation is not needed for this change (explain why)

Internal refactor with no user-visible API change.

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

Bug Fixes
- Improved WebSocket endpoint handling with stricter scheme validation, more reliable host/port/path construction, and clearer error reporting for invalid or unsupported inputs.
Tests
- Added comprehensive tests for URL conversion and validation, covering hostnames, ports, IPv4/IPv6 formats, and malformed or unsupported URLs.

…rd ports

coderabbitai · 2026-05-04T09:31:35Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 62d6c91f-a494-4d55-aacc-8698ead91ec5

📥 Commits

Reviewing files that changed from the base of the PR and between a61a04a and 67dc18e.

📒 Files selected for processing (1)

shared/relay/client/dialer/ws/ws.go

📝 Walkthrough

Walkthrough

Replaces string-based WebSocket URL rewriting with a url.Parse-based prepareURL that validates rel/rels, maps them to ws/wss, sets the relay path, and returns explicit errors; Dial now uses the produced URL directly. Adds table-driven tests for mappings and error cases.

Changes

WebSocket URL Preparation Refactoring

Layer / File(s)	Summary
Data Shape / Parsing `shared/relay/client/dialer/ws/ws.go`	`prepareURL` now uses `net/url` to parse input, validates `rel`/`rels` schemes, maps them to `ws`/`wss`, sets `parsed.Path = relay.WebSocketURLPath`, and returns explicit parse/unsupported-scheme errors. Removed `strings`-based logic.
Core Dial Flow `shared/relay/client/dialer/ws/ws.go`	`Dial` calls `prepareURL` and dials the returned `wsURL` directly (previous reparse-and-overwrite removed).
Tests / Verification `shared/relay/client/dialer/ws/ws_test.go`	Adds `TestPrepareURL` table-driven tests verifying scheme translation (`rel`→`ws`, `rels`→`wss`), host and port preservation (including IPv4/IPv6 and non-standard ports), `/relay` path presence, and error cases for unsupported or missing schemes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Unable to use Relay due to failing healthcheck for non-standard HTTP ports #6057 — Addresses the same prepareURL parsing/preserving-port behavior and scheme mapping.

Possibly related PRs

netbirdio/netbird#4872 — Related changes to WebSocket URL construction and rel/rels → ws/wss mapping.

Suggested reviewers

mlsmaycon

Poem

🐰 I parsed the paths with gentle care,
rels to wss, rel to ws — prepare,
Hosts and ports kept snug and tight,
Tests hop forward in morning light,
A rabbit cheers the dialer's flight.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
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 (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly describes the main change: rewriting prepareURL to preserve non-standard ports in WS dialer URL preparation, which aligns with the core objective.
Description check	✅ Passed	The description follows the template structure, includes a clear summary of changes, references issue `#6057`, and properly completes the checklist with explanations for documentation and test coverage.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ 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/relay-healthcheck-non-standard-port

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.}

github-actions · 2026-05-04T09:33:13Z

Release artifacts

Built for PR head 67dc18e in workflow run #14952.

Artifact	Link
All release artifacts	Download
Linux packages	Download
Windows packages	Download
macOS packages	Download
UI artifacts	Download
UI macOS artifacts	Download

GHCR images (amd64)

This comment is updated by the Release workflow. Artifact links expire according to the workflow retention policy.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@shared/relay/client/dialer/ws/ws.go`:
- Around line 57-70: After parsing the relay address in ws.go, ensure the parsed
URL contains an authority host before returning: after the scheme mapping (cases
for "rel"/"rels") add a check like if parsed.Host == "" (or parsed.Hostname() ==
"") return "", fmt.Errorf("missing host in relay address %q", address) so opaque
inputs like "rel:example.com:33080" are rejected; keep the rest of the logic
(set parsed.Path = relay.WebSocketURLPath and return parsed.String()) unchanged
and reference the existing parsed variable and the function that constructs the
WS URL.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1f91ec78-3f9d-4a38-bc9c-c772f5fd6695

📥 Commits

Reviewing files that changed from the base of the PR and between abccbde and a61a04a.

📒 Files selected for processing (1)

shared/relay/client/dialer/ws/ws.go

sonarqubecloud · 2026-05-06T09:36:28Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Rewrite relay WS dialer URL prep using net/url to preserve non-standa…

a409678

…rd ports

Add IPv6 cases to relay WS prepareURL tests

abccbde

Merge branch 'main' into fix/relay-healthcheck-non-standard-port

a61a04a

coderabbitai Bot reviewed May 6, 2026

View reviewed changes

Comment thread shared/relay/client/dialer/ws/ws.go

Reject relay address with missing host

67dc18e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[relay] Preserve non-standard port in WS dialer URL prep#6061

[relay] Preserve non-standard port in WS dialer URL prep#6061
lixmal wants to merge 4 commits intomainfrom
fix/relay-healthcheck-non-standard-port

lixmal commented May 4, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 4, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented May 4, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

sonarqubecloud Bot commented May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

lixmal commented May 4, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

Issue ticket number and link

Stack

Checklist

Documentation

Docs PR URL (required if "docs added" is checked)

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release artifacts

GHCR images (amd64)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud Bot commented May 6, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lixmal commented May 4, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 4, 2026 •

edited

Loading

github-actions Bot commented May 4, 2026 •

edited

Loading