diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index 46ce7ba80..dcc93a8cf 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 000000000..2cf0050a3 --- /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 292a5729e..63d8f489d 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 2301c990d..144abfef4 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 78290bf68..6f8b78825 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 124261527..43fd7b36b 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -1966,6 +1966,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"), @@ -1977,6 +1988,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"), @@ -1997,6 +2019,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 f7f0a4eb2..2a529240a 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 @@ -1629,8 +1630,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. @@ -1641,10 +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. """ - 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(): @@ -1653,21 +1651,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 ba353bde3..6261d856f 100644 --- a/python/test_client.py +++ b/python/test_client.py @@ -987,12 +987,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 @@ -1003,6 +1015,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"))