From 7fce72a5def8c484889d0e5faf1a9b695b323ec0 Mon Sep 17 00:00:00 2001 From: MarkXian Date: Sat, 1 Aug 2026 16:50:31 +0800 Subject: [PATCH 1/3] fix: support bracketed IPv6 runtime URLs --- dotnet/src/Client.cs | 23 +++++++-- .../Unit/RuntimeConnectionUrlParsingTests.cs | 51 +++++++++++++++++++ go/client.go | 47 ++++++++++------- go/client_test.go | 27 ++++++++++ nodejs/src/client.ts | 41 +++++++++++---- nodejs/test/client.test.ts | 31 +++++++++++ python/copilot/client.py | 23 ++++----- python/test_client.py | 16 ++++++ 8 files changed, 216 insertions(+), 43 deletions(-) create mode 100644 dotnet/test/Unit/RuntimeConnectionUrlParsingTests.cs diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index 228b115601..3e085e4692 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -167,7 +167,7 @@ public CopilotClient(CopilotClientOptions? options = null) throw new ArgumentException("GitHubToken and UseLoggedInUser cannot be combined with RuntimeConnection.ForUri (the existing runtime manages its own auth).", nameof(options)); } var parsed = ParseRuntimeUrl(uri.Url); - _optionsHost = parsed.Host; + _optionsHost = parsed.Host.Trim('[', ']'); _optionsPort = parsed.Port; break; @@ -298,12 +298,18 @@ private static RuntimeConnection ResolveDefaultConnection(CopilotClientOptions o /// /// Parses a runtime URL into a URI with host and port. /// - /// The URL to parse. Supports formats: "port", "host:port", "http://host:port". + /// The URL to parse. Supports formats: "port", "host:port", "[ipv6]:port", "http://host:port". private static Uri ParseRuntimeUrl(string url) { + url = url.Trim(); + // If it's just a port number, treat as localhost if (int.TryParse(url, out var port)) { + if (port <= 0 || port > 65535) + { + throw new ArgumentException($"Invalid runtime URL port: {url}"); + } return new Uri($"http://localhost:{port}"); } @@ -314,7 +320,18 @@ private static Uri ParseRuntimeUrl(string url) url = "https://" + url; } - return new Uri(url); + if (!Uri.TryCreate(url, UriKind.Absolute, out var uri) || + string.IsNullOrEmpty(uri.Host) || + uri.Port <= 0 || + uri.Port > 65535 || + (!string.IsNullOrEmpty(uri.AbsolutePath) && uri.AbsolutePath != "/") || + !string.IsNullOrEmpty(uri.Query) || + !string.IsNullOrEmpty(uri.Fragment)) + { + throw new ArgumentException($"Invalid runtime URL: {url}"); + } + + return uri; } /// diff --git a/dotnet/test/Unit/RuntimeConnectionUrlParsingTests.cs b/dotnet/test/Unit/RuntimeConnectionUrlParsingTests.cs new file mode 100644 index 0000000000..2cf0050a3c --- /dev/null +++ b/dotnet/test/Unit/RuntimeConnectionUrlParsingTests.cs @@ -0,0 +1,51 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +using Xunit; +using System.Reflection; + +namespace GitHub.Copilot.Test.Unit; + +public class RuntimeConnectionUrlParsingTests +{ + [Fact] + public void ForUri_ParsesBracketedIpv6HostPort() + { + var client = new CopilotClient(new CopilotClientOptions + { + Connection = RuntimeConnection.ForUri("[::1]:9000") + }); + + Assert.Equal("::1", GetPrivateField(client, "_optionsHost")); + Assert.Equal(9000, GetPrivateField(client, "_optionsPort")); + } + + [Fact] + public void ForUri_ParsesHttpIpv6HostPort() + { + var client = new CopilotClient(new CopilotClientOptions + { + Connection = RuntimeConnection.ForUri("http://[::1]:7000") + }); + + Assert.Equal("::1", GetPrivateField(client, "_optionsHost")); + Assert.Equal(7000, GetPrivateField(client, "_optionsPort")); + } + + [Fact] + public void ForUri_RejectsUrlPath() + { + Assert.Throws(() => new CopilotClient(new CopilotClientOptions + { + Connection = RuntimeConnection.ForUri("http://localhost:8080/path") + })); + } + + private static T? GetPrivateField(object instance, string name) + { + var field = instance.GetType().GetField(name, BindingFlags.Instance | BindingFlags.NonPublic); + Assert.NotNull(field); + return (T?)field.GetValue(instance); + } +} diff --git a/go/client.go b/go/client.go index d2c43c26bd..10de530dc1 100644 --- a/go/client.go +++ b/go/client.go @@ -36,6 +36,7 @@ import ( "fmt" "log" "net" + neturl "net/url" "os" "os/exec" "regexp" @@ -372,35 +373,47 @@ func setEnvValue(env []string, key string, value string) []string { // parseCLIURL parses a CLI URL into host and port components. // -// Supports formats: "host:port", "http://host:port", "https://host:port", or just "port". +// Supports formats: "host:port", "[ipv6]:port", "http://host:port", "https://host:port", or just "port". // Panics if the URL format is invalid or the port is out of range. func parseCLIURL(url string) (string, int) { - // Remove protocol if present - cleanURL, _ := strings.CutPrefix(url, "https://") - cleanURL, _ = strings.CutPrefix(cleanURL, "http://") - - // Parse host:port or port format - var host string - var portStr string - if before, after, found := strings.Cut(cleanURL, ":"); found { - host = before - portStr = after - } else { - // Only port provided - portStr = before + cleanURL := strings.TrimSpace(url) + if cleanURL == "" { + panic(fmt.Sprintf("Invalid URIConnection format: %s", url)) + } + + if _, err := strconv.Atoi(cleanURL); err == nil { + port := parseCLIPort(url, cleanURL) + return "localhost", port + } + + parseURL := cleanURL + if !strings.Contains(parseURL, "://") { + parseURL = "tcp://" + parseURL + } + + parsed, err := neturl.Parse(parseURL) + if err != nil { + panic(fmt.Sprintf("Invalid URIConnection format: %s", url)) + } + if parsed.Host == "" || parsed.Port() == "" || parsed.RawQuery != "" || parsed.Fragment != "" || (parsed.Path != "" && parsed.Path != "/") { + panic(fmt.Sprintf("Invalid URIConnection format: %s", url)) } + port := parseCLIPort(url, parsed.Port()) + host := parsed.Hostname() if host == "" { host = "localhost" } - // Validate port + return host, port +} + +func parseCLIPort(url string, portStr string) int { port, err := strconv.Atoi(portStr) if err != nil || port <= 0 || port > 65535 { panic(fmt.Sprintf("Invalid port in URIConnection: %s", url)) } - - return host, port + return port } // Start starts the CLI server (if not using an external server) and establishes diff --git a/go/client_test.go b/go/client_test.go index b5274cfda0..2961ace6eb 100644 --- a/go/client_test.go +++ b/go/client_test.go @@ -49,6 +49,15 @@ func TestClient_URLParsing(t *testing.T) { } }) + t.Run("should parse bracketed IPv6 host:port URL format", func(t *testing.T) { + client := NewClient(&ClientOptions{ + Connection: URIConnection{URL: "[::1]:9000"}, + }) + if client.actualPort != 9000 || client.actualHost != "::1" { + t.Errorf("Expected [::1]:9000, got %s:%d", client.actualHost, client.actualPort) + } + }) + t.Run("should parse http://host:port URL format", func(t *testing.T) { client := NewClient(&ClientOptions{ Connection: URIConnection{URL: "http://localhost:7000"}, @@ -58,6 +67,15 @@ func TestClient_URLParsing(t *testing.T) { } }) + t.Run("should parse http://[ipv6]:port URL format", func(t *testing.T) { + client := NewClient(&ClientOptions{ + Connection: URIConnection{URL: "http://[::1]:7000"}, + }) + if client.actualPort != 7000 || client.actualHost != "::1" { + t.Errorf("Expected [::1]:7000, got %s:%d", client.actualHost, client.actualPort) + } + }) + t.Run("should parse https://host:port URL format", func(t *testing.T) { client := NewClient(&ClientOptions{ Connection: URIConnection{URL: "https://example.com:443"}, @@ -76,6 +94,15 @@ func TestClient_URLParsing(t *testing.T) { NewClient(&ClientOptions{Connection: URIConnection{URL: "invalid-url"}}) }) + t.Run("should panic for URL path", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("Expected panic for invalid URL path") + } + }() + NewClient(&ClientOptions{Connection: URIConnection{URL: "http://localhost:8080/path"}}) + }) + t.Run("should panic for invalid port - too high", func(t *testing.T) { defer func() { if r := recover(); r == nil { diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 4ed139be70..6cf215ae9b 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -765,27 +765,46 @@ export class CopilotClient { /** * Parse CLI URL into host and port - * Supports formats: "host:port", "http://host:port", "https://host:port", or just "port" + * Supports formats: "host:port", "[ipv6]:port", "http://host:port", "https://host:port", or just "port" */ private parseCliUrl(url: string): { host: string; port: number } { - // Remove protocol if present - let cleanUrl = url.replace(/^https?:\/\//, ""); + const trimmedUrl = url.trim(); // Check if it's just a port number - if (/^\d+$/.test(cleanUrl)) { - return { host: "localhost", port: parseInt(cleanUrl, 10) }; + if (/^\d+$/.test(trimmedUrl)) { + return { host: "localhost", port: parseInt(trimmedUrl, 10) }; } - // Parse host:port format - const parts = cleanUrl.split(":"); - if (parts.length !== 2) { + let parsed: URL; + try { + parsed = new URL( + /^[a-z][a-z0-9+.-]*:\/\//i.test(trimmedUrl) ? trimmedUrl : `tcp://${trimmedUrl}` + ); + } catch { + if (trimmedUrl.includes(":")) { + throw new Error(`Invalid port in cliUrl: ${url}`); + } + throw new Error( + `Invalid cliUrl format: ${url}. Expected "host:port", "[ipv6]:port", "http://host:port", or "port"` + ); + } + + const explicitPort = trimmedUrl.match(/:(\d+)(?:[/?#]|$)/)?.[1]; + const portString = parsed.port || explicitPort; + + if ( + !portString || + (parsed.pathname !== "" && parsed.pathname !== "/") || + parsed.search !== "" || + parsed.hash !== "" + ) { throw new Error( - `Invalid cliUrl format: ${url}. Expected "host:port", "http://host:port", or "port"` + `Invalid cliUrl format: ${url}. Expected "host:port", "[ipv6]:port", "http://host:port", or "port"` ); } - const host = parts[0] || "localhost"; - const port = parseInt(parts[1], 10); + const host = parsed.hostname.replace(/^\[(.*)\]$/, "$1") || "localhost"; + const port = parseInt(portString, 10); if (isNaN(port) || port <= 0 || port > 65535) { throw new Error(`Invalid port in cliUrl: ${url}`); diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 962d90970e..2c8713859d 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -2166,6 +2166,17 @@ describe("CopilotClient", () => { expect((client as any).isExternalServer).toBe(true); }); + it("should parse bracketed IPv6 host:port URL format", () => { + const client = new CopilotClient({ + connection: RuntimeConnection.forUri("[::1]:9000"), + logLevel: "error", + }); + + expect((client as any).runtimePort).toBe(9000); + expect((client as any).actualHost).toBe("::1"); + expect((client as any).isExternalServer).toBe(true); + }); + it("should parse http://host:port URL format", () => { const client = new CopilotClient({ connection: RuntimeConnection.forUri("http://localhost:7000"), @@ -2177,6 +2188,17 @@ describe("CopilotClient", () => { expect((client as any).isExternalServer).toBe(true); }); + it("should parse http://[ipv6]:port URL format", () => { + const client = new CopilotClient({ + connection: RuntimeConnection.forUri("http://[::1]:7000"), + logLevel: "error", + }); + + expect((client as any).runtimePort).toBe(7000); + expect((client as any).actualHost).toBe("::1"); + expect((client as any).isExternalServer).toBe(true); + }); + it("should parse https://host:port URL format", () => { const client = new CopilotClient({ connection: RuntimeConnection.forUri("https://example.com:443"), @@ -2197,6 +2219,15 @@ describe("CopilotClient", () => { }).toThrow(/Invalid cliUrl format/); }); + it("should throw error for URL path", () => { + expect(() => { + new CopilotClient({ + connection: RuntimeConnection.forUri("http://localhost:8080/path"), + logLevel: "error", + }); + }).toThrow(/Invalid cliUrl format/); + }); + it("should throw error for invalid port - too high", () => { expect(() => { new CopilotClient({ diff --git a/python/copilot/client.py b/python/copilot/client.py index 7c273bd010..96e09eda2d 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -30,6 +30,7 @@ from datetime import UTC, datetime from types import TracebackType from typing import Any, ClassVar, Literal, TypedDict, cast, overload +from urllib.parse import urlsplit from ._diagnostics import log_timing from ._ffi_runtime_host import FfiRuntimeHost @@ -1630,8 +1631,8 @@ def _parse_cli_url(self, url: str) -> tuple[str, int]: """ Parse CLI URL into host and port. - Supports formats: "host:port", "http://host:port", "https://host:port", - or just "port". + Supports formats: "host:port", "[ipv6]:port", "http://host:port", + "https://host:port", or just "port". Args: url: The CLI URL to parse. @@ -1642,10 +1643,7 @@ def _parse_cli_url(self, url: str) -> tuple[str, int]: Raises: ValueError: If the URL format is invalid or the port is out of range. """ - import re - - # Remove protocol if present - clean_url = re.sub(r"^https?://", "", url) + clean_url = url.strip() # Check if it's just a port number if clean_url.isdigit(): @@ -1654,21 +1652,22 @@ def _parse_cli_url(self, url: str) -> tuple[str, int]: raise ValueError(f"Invalid port in cli_url: {url}") return ("localhost", port) - # Parse host:port format - parts = clean_url.split(":") - if len(parts) != 2: + parsed = urlsplit(clean_url if "://" in clean_url else f"tcp://{clean_url}") + if parsed.path not in ("", "/") or parsed.query or parsed.fragment: raise ValueError(f"Invalid cli_url format: {url}") - host = parts[0] if parts[0] else "localhost" try: - port = int(parts[1]) + port = parsed.port except ValueError as e: raise ValueError(f"Invalid port in cli_url: {url}") from e + if port is None: + raise ValueError(f"Invalid cli_url format: {url}") + if port <= 0 or port > 65535: raise ValueError(f"Invalid port in cli_url: {url}") - return (host, port) + return (parsed.hostname or "localhost", port) async def __aenter__(self) -> CopilotClient: """ diff --git a/python/test_client.py b/python/test_client.py index f101fc3968..d0ba903053 100644 --- a/python/test_client.py +++ b/python/test_client.py @@ -1196,12 +1196,24 @@ def test_parse_host_port_url(self): assert client._actual_host == "127.0.0.1" assert client._is_external_server + def test_parse_bracketed_ipv6_host_port_url(self): + client = CopilotClient(connection=RuntimeConnection.for_uri("[::1]:9000")) + assert client._runtime_port == 9000 + assert client._actual_host == "::1" + assert client._is_external_server + def test_parse_http_url(self): client = CopilotClient(connection=RuntimeConnection.for_uri("http://localhost:7000")) assert client._runtime_port == 7000 assert client._actual_host == "localhost" assert client._is_external_server + def test_parse_http_ipv6_url(self): + client = CopilotClient(connection=RuntimeConnection.for_uri("http://[::1]:7000")) + assert client._runtime_port == 7000 + assert client._actual_host == "::1" + assert client._is_external_server + def test_parse_https_url(self): client = CopilotClient(connection=RuntimeConnection.for_uri("https://example.com:443")) assert client._runtime_port == 443 @@ -1212,6 +1224,10 @@ def test_invalid_url_format(self): with pytest.raises(ValueError, match="Invalid cli_url format"): CopilotClient(connection=RuntimeConnection.for_uri("invalid-url")) + def test_invalid_url_path(self): + with pytest.raises(ValueError, match="Invalid cli_url format"): + CopilotClient(connection=RuntimeConnection.for_uri("http://localhost:8080/path")) + def test_invalid_port_too_high(self): with pytest.raises(ValueError, match="Invalid port in cli_url"): CopilotClient(connection=RuntimeConnection.for_uri("localhost:99999")) From 1b0af6ebbf255ba87d757f69b35c7201e8380b18 Mon Sep 17 00:00:00 2001 From: MarkXian Date: Thu, 6 Aug 2026 18:15:03 +0800 Subject: [PATCH 2/3] fix: support bracketed IPv6 runtime URLs across SDKs --- dotnet/src/Client.cs | 19 +------ .../Unit/RuntimeConnectionUrlParsingTests.cs | 9 ---- go/client.go | 53 +++++++++---------- go/client_test.go | 9 ---- .../com/github/copilot/CopilotClient.java | 5 +- .../github/copilot/CliServerManagerTest.java | 7 +++ .../com/github/copilot/CopilotClientTest.java | 9 ++++ nodejs/src/client.ts | 40 ++++++-------- nodejs/test/client.test.ts | 9 ---- python/copilot/client.py | 34 ++++++------ python/test_client.py | 16 ++++-- 11 files changed, 94 insertions(+), 116 deletions(-) diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index 3e085e4692..16c56f63f4 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -301,15 +301,9 @@ private static RuntimeConnection ResolveDefaultConnection(CopilotClientOptions o /// The URL to parse. Supports formats: "port", "host:port", "[ipv6]:port", "http://host:port". private static Uri ParseRuntimeUrl(string url) { - url = url.Trim(); - // If it's just a port number, treat as localhost if (int.TryParse(url, out var port)) { - if (port <= 0 || port > 65535) - { - throw new ArgumentException($"Invalid runtime URL port: {url}"); - } return new Uri($"http://localhost:{port}"); } @@ -320,18 +314,7 @@ private static Uri ParseRuntimeUrl(string url) url = "https://" + url; } - if (!Uri.TryCreate(url, UriKind.Absolute, out var uri) || - string.IsNullOrEmpty(uri.Host) || - uri.Port <= 0 || - uri.Port > 65535 || - (!string.IsNullOrEmpty(uri.AbsolutePath) && uri.AbsolutePath != "/") || - !string.IsNullOrEmpty(uri.Query) || - !string.IsNullOrEmpty(uri.Fragment)) - { - throw new ArgumentException($"Invalid runtime URL: {url}"); - } - - return uri; + return new Uri(url); } /// diff --git a/dotnet/test/Unit/RuntimeConnectionUrlParsingTests.cs b/dotnet/test/Unit/RuntimeConnectionUrlParsingTests.cs index 2cf0050a3c..1b16456b0d 100644 --- a/dotnet/test/Unit/RuntimeConnectionUrlParsingTests.cs +++ b/dotnet/test/Unit/RuntimeConnectionUrlParsingTests.cs @@ -33,15 +33,6 @@ public void ForUri_ParsesHttpIpv6HostPort() Assert.Equal(7000, GetPrivateField(client, "_optionsPort")); } - [Fact] - public void ForUri_RejectsUrlPath() - { - Assert.Throws(() => new CopilotClient(new CopilotClientOptions - { - Connection = RuntimeConnection.ForUri("http://localhost:8080/path") - })); - } - private static T? GetPrivateField(object instance, string name) { var field = instance.GetType().GetField(name, BindingFlags.Instance | BindingFlags.NonPublic); diff --git a/go/client.go b/go/client.go index 10de530dc1..cdcca85f0f 100644 --- a/go/client.go +++ b/go/client.go @@ -36,7 +36,6 @@ import ( "fmt" "log" "net" - neturl "net/url" "os" "os/exec" "regexp" @@ -376,44 +375,44 @@ func setEnvValue(env []string, key string, value string) []string { // Supports formats: "host:port", "[ipv6]:port", "http://host:port", "https://host:port", or just "port". // Panics if the URL format is invalid or the port is out of range. func parseCLIURL(url string) (string, int) { - cleanURL := strings.TrimSpace(url) - if cleanURL == "" { - panic(fmt.Sprintf("Invalid URIConnection format: %s", url)) - } - - if _, err := strconv.Atoi(cleanURL); err == nil { - port := parseCLIPort(url, cleanURL) - return "localhost", port - } - - parseURL := cleanURL - if !strings.Contains(parseURL, "://") { - parseURL = "tcp://" + parseURL + // Remove protocol if present + cleanURL, _ := strings.CutPrefix(url, "https://") + cleanURL, _ = strings.CutPrefix(cleanURL, "http://") + + // Use the standard parser only for the bracketed IPv6 form. Keep the + // existing host:port parsing behavior for all other inputs. + if strings.HasPrefix(cleanURL, "[") { + host, portStr, err := net.SplitHostPort(cleanURL) + if err != nil { + panic(fmt.Sprintf("Invalid port in URIConnection: %s", url)) + } + port, err := strconv.Atoi(portStr) + if err != nil || port <= 0 || port > 65535 { + panic(fmt.Sprintf("Invalid port in URIConnection: %s", url)) + } + return host, port } - parsed, err := neturl.Parse(parseURL) - if err != nil { - panic(fmt.Sprintf("Invalid URIConnection format: %s", url)) - } - if parsed.Host == "" || parsed.Port() == "" || parsed.RawQuery != "" || parsed.Fragment != "" || (parsed.Path != "" && parsed.Path != "/") { - panic(fmt.Sprintf("Invalid URIConnection format: %s", url)) + // Parse host:port or port format + var host string + var portStr string + if before, after, found := strings.Cut(cleanURL, ":"); found { + host = before + portStr = after + } else { + portStr = cleanURL } - port := parseCLIPort(url, parsed.Port()) - host := parsed.Hostname() if host == "" { host = "localhost" } - return host, port -} - -func parseCLIPort(url string, portStr string) int { port, err := strconv.Atoi(portStr) if err != nil || port <= 0 || port > 65535 { panic(fmt.Sprintf("Invalid port in URIConnection: %s", url)) } - return port + + return host, port } // Start starts the CLI server (if not using an external server) and establishes diff --git a/go/client_test.go b/go/client_test.go index 2961ace6eb..f3a0e80535 100644 --- a/go/client_test.go +++ b/go/client_test.go @@ -94,15 +94,6 @@ func TestClient_URLParsing(t *testing.T) { NewClient(&ClientOptions{Connection: URIConnection{URL: "invalid-url"}}) }) - t.Run("should panic for URL path", func(t *testing.T) { - defer func() { - if r := recover(); r == nil { - t.Error("Expected panic for invalid URL path") - } - }() - NewClient(&ClientOptions{Connection: URIConnection{URL: "http://localhost:8080/path"}}) - }) - t.Run("should panic for invalid port - too high", func(t *testing.T) { defer func() { if r := recover(); r == nil { diff --git a/java/src/main/java/com/github/copilot/CopilotClient.java b/java/src/main/java/com/github/copilot/CopilotClient.java index 44878b87ec..5a4e6659ad 100644 --- a/java/src/main/java/com/github/copilot/CopilotClient.java +++ b/java/src/main/java/com/github/copilot/CopilotClient.java @@ -184,7 +184,10 @@ public CopilotClient(CopilotClientOptions options) { // Parse CliUrl if provided if (this.options.getCliUrl() != null && !this.options.getCliUrl().isEmpty()) { URI uri = CliServerManager.parseCliUrl(this.options.getCliUrl()); - this.optionsHost = uri.getHost(); + String host = uri.getHost(); + this.optionsHost = host != null && host.startsWith("[") && host.endsWith("]") + ? host.substring(1, host.length() - 1) + : host; this.optionsPort = uri.getPort(); } else { this.optionsHost = null; diff --git a/java/src/test/java/com/github/copilot/CliServerManagerTest.java b/java/src/test/java/com/github/copilot/CliServerManagerTest.java index b445d6153a..141e658e41 100644 --- a/java/src/test/java/com/github/copilot/CliServerManagerTest.java +++ b/java/src/test/java/com/github/copilot/CliServerManagerTest.java @@ -48,6 +48,13 @@ void parseCliUrlWithHttpsPrefix() { assertEquals("https://secure.host:443", uri.toString()); } + @Test + void parseCliUrlWithBracketedIpv6() { + URI uri = CliServerManager.parseCliUrl("[::1]:4321"); + assertNotNull(uri.getHost()); + assertEquals(4321, uri.getPort()); + } + @Test void parseCliUrlWithHostOnly() { URI uri = CliServerManager.parseCliUrl("copilot.example.com"); diff --git a/java/src/test/java/com/github/copilot/CopilotClientTest.java b/java/src/test/java/com/github/copilot/CopilotClientTest.java index d977563aeb..6bbb26c143 100644 --- a/java/src/test/java/com/github/copilot/CopilotClientTest.java +++ b/java/src/test/java/com/github/copilot/CopilotClientTest.java @@ -148,6 +148,15 @@ void testCliUrlOnlyConstruction() { client.close(); } + @Test + void testBracketedIpv6CliUrlNormalizesHost() throws Exception { + try (var client = new CopilotClient(new CopilotClientOptions().setCliUrl("[::1]:4321"))) { + Field hostField = CopilotClient.class.getDeclaredField("optionsHost"); + hostField.setAccessible(true); + assertEquals("::1", hostField.get(client)); + } + } + @Test void testCliUrlMutualExclusionWithCliPath() { var options = new CopilotClientOptions().setCliUrl("localhost:3000").setCliPath("/path/to/cli"); diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 6cf215ae9b..373558351a 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -768,43 +768,35 @@ export class CopilotClient { * Supports formats: "host:port", "[ipv6]:port", "http://host:port", "https://host:port", or just "port" */ private parseCliUrl(url: string): { host: string; port: number } { - const trimmedUrl = url.trim(); + // Remove protocol if present + const cleanUrl = url.replace(/^https?:\/\//, ""); // Check if it's just a port number - if (/^\d+$/.test(trimmedUrl)) { - return { host: "localhost", port: parseInt(trimmedUrl, 10) }; + if (/^\d+$/.test(cleanUrl)) { + return { host: "localhost", port: parseInt(cleanUrl, 10) }; } - let parsed: URL; - try { - parsed = new URL( - /^[a-z][a-z0-9+.-]*:\/\//i.test(trimmedUrl) ? trimmedUrl : `tcp://${trimmedUrl}` - ); - } catch { - if (trimmedUrl.includes(":")) { + // Handle the canonical bracketed IPv6 host:port form without changing + // the existing parser behavior for other inputs. + const ipv6Match = cleanUrl.match(/^\[([^\]]+)\]:(\d+)$/); + if (ipv6Match) { + const port = parseInt(ipv6Match[2], 10); + if (isNaN(port) || port <= 0 || port > 65535) { throw new Error(`Invalid port in cliUrl: ${url}`); } - throw new Error( - `Invalid cliUrl format: ${url}. Expected "host:port", "[ipv6]:port", "http://host:port", or "port"` - ); + return { host: ipv6Match[1], port }; } - const explicitPort = trimmedUrl.match(/:(\d+)(?:[/?#]|$)/)?.[1]; - const portString = parsed.port || explicitPort; - - if ( - !portString || - (parsed.pathname !== "" && parsed.pathname !== "/") || - parsed.search !== "" || - parsed.hash !== "" - ) { + // Parse host:port format + const parts = cleanUrl.split(":"); + if (parts.length !== 2) { throw new Error( `Invalid cliUrl format: ${url}. Expected "host:port", "[ipv6]:port", "http://host:port", or "port"` ); } - const host = parsed.hostname.replace(/^\[(.*)\]$/, "$1") || "localhost"; - const port = parseInt(portString, 10); + const host = parts[0] || "localhost"; + const port = parseInt(parts[1], 10); if (isNaN(port) || port <= 0 || port > 65535) { throw new Error(`Invalid port in cliUrl: ${url}`); diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 2c8713859d..077676f869 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -2219,15 +2219,6 @@ describe("CopilotClient", () => { }).toThrow(/Invalid cliUrl format/); }); - it("should throw error for URL path", () => { - expect(() => { - new CopilotClient({ - connection: RuntimeConnection.forUri("http://localhost:8080/path"), - logLevel: "error", - }); - }).toThrow(/Invalid cliUrl format/); - }); - it("should throw error for invalid port - too high", () => { expect(() => { new CopilotClient({ diff --git a/python/copilot/client.py b/python/copilot/client.py index 96e09eda2d..ce73108dc7 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -30,7 +30,6 @@ from datetime import UTC, datetime from types import TracebackType from typing import Any, ClassVar, Literal, TypedDict, cast, overload -from urllib.parse import urlsplit from ._diagnostics import log_timing from ._ffi_runtime_host import FfiRuntimeHost @@ -1643,7 +1642,7 @@ def _parse_cli_url(self, url: str) -> tuple[str, int]: Raises: ValueError: If the URL format is invalid or the port is out of range. """ - clean_url = url.strip() + clean_url = re.sub(r"^https?://", "", url) # Check if it's just a port number if clean_url.isdigit(): @@ -1652,22 +1651,27 @@ def _parse_cli_url(self, url: str) -> tuple[str, int]: raise ValueError(f"Invalid port in cli_url: {url}") return ("localhost", port) - parsed = urlsplit(clean_url if "://" in clean_url else f"tcp://{clean_url}") - if parsed.path not in ("", "/") or parsed.query or parsed.fragment: - raise ValueError(f"Invalid cli_url format: {url}") + ipv6_match = re.match(r"^\[([^\]]+)\]:(.*)$", clean_url) + if ipv6_match: + host = ipv6_match.group(1) + port_text = ipv6_match.group(2) + else: + # Parse host:port format + parts = clean_url.split(":") + if len(parts) != 2: + raise ValueError(f"Invalid cli_url format: {url}") + host = parts[0] if parts[0] else "localhost" + port_text = parts[1] try: - port = parsed.port + port = int(port_text) except ValueError as e: raise ValueError(f"Invalid port in cli_url: {url}") from e - if port is None: - raise ValueError(f"Invalid cli_url format: {url}") - if port <= 0 or port > 65535: raise ValueError(f"Invalid port in cli_url: {url}") - return (parsed.hostname or "localhost", port) + return (host, port) async def __aenter__(self) -> CopilotClient: """ @@ -4272,22 +4276,22 @@ async def _connect_via_tcp(self) -> None: if not self._runtime_port: raise RuntimeError("Server port not available") - # Create a TCP socket connection with timeout + # Create a TCP socket connection with timeout. create_connection resolves + # both IPv4 and IPv6 addresses instead of forcing AF_INET. import socket # Connection timeout constant TCP_CONNECTION_TIMEOUT = 10 # seconds - sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - sock.settimeout(TCP_CONNECTION_TIMEOUT) - try: tcp_connect_start = time.perf_counter() logger.info( "CopilotClient._connect_via_tcp connecting to CLI server", extra={"host": self._actual_host, "port": self._runtime_port}, ) - sock.connect((self._actual_host, self._runtime_port)) + sock = socket.create_connection( + (self._actual_host, self._runtime_port), timeout=TCP_CONNECTION_TIMEOUT + ) sock.settimeout(None) # Remove timeout after connection log_timing( logger, diff --git a/python/test_client.py b/python/test_client.py index d0ba903053..9f261dd489 100644 --- a/python/test_client.py +++ b/python/test_client.py @@ -1224,10 +1224,6 @@ def test_invalid_url_format(self): with pytest.raises(ValueError, match="Invalid cli_url format"): CopilotClient(connection=RuntimeConnection.for_uri("invalid-url")) - def test_invalid_url_path(self): - with pytest.raises(ValueError, match="Invalid cli_url format"): - CopilotClient(connection=RuntimeConnection.for_uri("http://localhost:8080/path")) - def test_invalid_port_too_high(self): with pytest.raises(ValueError, match="Invalid port in cli_url"): CopilotClient(connection=RuntimeConnection.for_uri("localhost:99999")) @@ -1244,6 +1240,18 @@ def test_is_external_server_true(self): client = CopilotClient(connection=RuntimeConnection.for_uri("localhost:8080")) assert client._is_external_server + @pytest.mark.asyncio + async def test_connect_via_tcp_uses_family_independent_resolution(self): + client = CopilotClient(connection=RuntimeConnection.for_uri("[::1]:9000")) + fake_socket = Mock() + fake_socket.makefile.return_value = Mock() + + with patch("socket.create_connection", return_value=fake_socket) as create_connection: + await client._connect_via_tcp() + + create_connection.assert_called_once_with(("::1", 9000), timeout=10) + client._process.terminate() + class TestSessionFsConfig: def test_missing_initial_cwd(self): From 298f34f800ea6ba388859708a5401b760fa702a9 Mon Sep 17 00:00:00 2001 From: MarkXian Date: Fri, 7 Aug 2026 12:28:58 +0800 Subject: [PATCH 3/3] fix: validate bracketed IPv6 runtime hosts --- go/client.go | 5 +++++ go/client_test.go | 9 +++++++++ nodejs/src/client.ts | 9 +++++++-- nodejs/test/client.test.ts | 9 +++++++++ python/copilot/client.py | 5 +++++ python/test_client.py | 4 ++++ 6 files changed, 39 insertions(+), 2 deletions(-) diff --git a/go/client.go b/go/client.go index cdcca85f0f..f6d746bc4e 100644 --- a/go/client.go +++ b/go/client.go @@ -36,6 +36,7 @@ import ( "fmt" "log" "net" + "net/netip" "os" "os/exec" "regexp" @@ -386,6 +387,10 @@ func parseCLIURL(url string) (string, int) { if err != nil { panic(fmt.Sprintf("Invalid port in URIConnection: %s", url)) } + addr, err := netip.ParseAddr(host) + if err != nil || !addr.Is6() { + panic(fmt.Sprintf("Invalid URIConnection format: %s", url)) + } port, err := strconv.Atoi(portStr) if err != nil || port <= 0 || port > 65535 { panic(fmt.Sprintf("Invalid port in URIConnection: %s", url)) diff --git a/go/client_test.go b/go/client_test.go index f3a0e80535..12112595f3 100644 --- a/go/client_test.go +++ b/go/client_test.go @@ -76,6 +76,15 @@ func TestClient_URLParsing(t *testing.T) { } }) + t.Run("should panic for bracketed non-IPv6 host", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("Expected panic for invalid bracketed host") + } + }() + NewClient(&ClientOptions{Connection: URIConnection{URL: "[not-ipv6]:1234"}}) + }) + t.Run("should parse https://host:port URL format", func(t *testing.T) { client := NewClient(&ClientOptions{ Connection: URIConnection{URL: "https://example.com:443"}, diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 373558351a..bc48bfbd5c 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -15,7 +15,7 @@ import { spawn, type ChildProcess } from "node:child_process"; import { randomUUID } from "node:crypto"; import { existsSync } from "node:fs"; import { createRequire } from "node:module"; -import { Socket } from "node:net"; +import { isIPv6, Socket } from "node:net"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { @@ -780,11 +780,16 @@ export class CopilotClient { // the existing parser behavior for other inputs. const ipv6Match = cleanUrl.match(/^\[([^\]]+)\]:(\d+)$/); if (ipv6Match) { + const host = ipv6Match[1]; + if (!isIPv6(host)) { + throw new Error(`Invalid cliUrl format: ${url}`); + } + const port = parseInt(ipv6Match[2], 10); if (isNaN(port) || port <= 0 || port > 65535) { throw new Error(`Invalid port in cliUrl: ${url}`); } - return { host: ipv6Match[1], port }; + return { host, port }; } // Parse host:port format diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 077676f869..a1dd590cc8 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -2199,6 +2199,15 @@ describe("CopilotClient", () => { expect((client as any).isExternalServer).toBe(true); }); + it("should reject a bracketed non-IPv6 host", () => { + expect(() => { + new CopilotClient({ + connection: RuntimeConnection.forUri("[not-ipv6]:1234"), + logLevel: "error", + }); + }).toThrow(/Invalid cliUrl format/); + }); + it("should parse https://host:port URL format", () => { const client = new CopilotClient({ connection: RuntimeConnection.forUri("https://example.com:443"), diff --git a/python/copilot/client.py b/python/copilot/client.py index ce73108dc7..a4383d218b 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -15,6 +15,7 @@ from __future__ import annotations import asyncio +import ipaddress import inspect import logging import os @@ -1655,6 +1656,10 @@ def _parse_cli_url(self, url: str) -> tuple[str, int]: if ipv6_match: host = ipv6_match.group(1) port_text = ipv6_match.group(2) + try: + ipaddress.IPv6Address(host) + except ValueError as e: + raise ValueError(f"Invalid cli_url format: {url}") from e else: # Parse host:port format parts = clean_url.split(":") diff --git a/python/test_client.py b/python/test_client.py index 9f261dd489..092a2d97fe 100644 --- a/python/test_client.py +++ b/python/test_client.py @@ -1214,6 +1214,10 @@ def test_parse_http_ipv6_url(self): assert client._actual_host == "::1" assert client._is_external_server + def test_reject_bracketed_non_ipv6_host(self): + with pytest.raises(ValueError, match="Invalid cli_url format"): + CopilotClient(connection=RuntimeConnection.for_uri("[not-ipv6]:1234")) + def test_parse_https_url(self): client = CopilotClient(connection=RuntimeConnection.for_uri("https://example.com:443")) assert client._runtime_port == 443