Skip to content

fix: support bracketed IPv6 runtime URLs - #2200

Draft
xianjianlf2 wants to merge 1 commit into
github:mainfrom
xianjianlf2:fix/cli-url-ipv6
Draft

fix: support bracketed IPv6 runtime URLs#2200
xianjianlf2 wants to merge 1 commit into
github:mainfrom
xianjianlf2:fix/cli-url-ipv6

Conversation

@xianjianlf2

Copy link
Copy Markdown
Contributor

Fixes #1137

Summary

  • support bracketed IPv6 runtime URLs across Node.js, Python, Go, and .NET
  • reject URI forms with path, query, or fragment components for runtime host/port parsing
  • add URL parsing regression tests for each supported SDK touched by the fix

Validation

  • cd go && go test .
  • cd python && uv run pytest test_client.py -q
  • cd nodejs && npx vitest run test/client.test.ts -t "URL parsing"
  • Not run: cd dotnet && dotnet test test/GitHub.Copilot.SDK.Test.csproj --filter RuntimeConnectionUrlParsingTests (dotnet is not installed in this environment: zsh:1: command not found: dotnet)

@xianjianlf2
xianjianlf2 requested a review from a team as a code owner August 1, 2026 08:50
@SteveSandersonMS

Copy link
Copy Markdown
Contributor

The underlying gap is worth fixing, but I think this PR should be narrower so it adds IPv6 without changing the existing endpoint grammar or taking on general URL-validation policy.

A minimal version would be:

  • Node: after the existing http(s):// removal, recognize the canonical [ipv6]:port form before falling back to the current host:port parser. Avoid moving all inputs through URL.
  • Python: add the same narrow bracketed-form branch and retain the existing parser for all other inputs. Also change _connect_via_tcp from an explicitly IPv4 socket.AF_INET socket to family-independent connection resolution (for example, socket.create_connection). At present this PR parses [::1]:4321, but start() still cannot connect to it because it constructs an IPv4-only socket.
  • Go: after removing the existing schemes, use net.SplitHostPort for bracket-prefixed inputs and otherwise retain the current behavior. net/url is not needed for this fix.
  • .NET: Uri already parses bracketed IPv6; normalize the parsed host to the unbracketed value if necessary and add regression coverage.
  • Java: confirm its existing URI path with a regression test and change production code only if the host passed to Socket remains bracketed.
  • Rust: no change; its external transport already accepts host and port as separate typed fields.

The intended new cases are simply:

RuntimeConnection.forUri("[::1]:4321");
RuntimeConnection.forUri("http://[2001:db8::1]:4321");

Tests should cover those forms in each URL-based SDK, plus one Python connector-level test proving the connection path is not forced to AF_INET. Parser-only assertions are insufficient there because they currently pass while the actual connection remains broken.

I would leave path/query/fragment rejection, whitespace normalization, arbitrary-scheme handling, userinfo policy, and unrelated port-validation changes out of this PR. Moving wholesale to general-purpose URL parsers changes which previously accepted inputs succeed or fail and introduces cross-language differences that then require us to define and maintain a much larger URL contract. A narrow bracketed-IPv6 branch fixes the demonstrated user problem, preserves compatibility, and avoids pulling those edge cases into scope.

@SteveSandersonMS

Copy link
Copy Markdown
Contributor

@xianjianlf2 Does that sound reasonable? I'll mark this PR as draft but please remark as ready to review as appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cliUrl / cli_url parser rejects IPv6 addresses (including bracketed [::1]:port form)

2 participants