From a9a0ba13ba63d348115457bf6073838655ccb3a5 Mon Sep 17 00:00:00 2001 From: CloakHQ Date: Sat, 18 Apr 2026 22:47:41 +0200 Subject: [PATCH] fix(proxy): auto URL-encode SOCKS5 credentials in string URLs (#157) Chromium's --proxy-server parser truncates passwords at '=' and other special chars, causing SOCKS5 auth to silently fail and fall back to direct connection. The dict path already encoded creds; now the string path does too. Idempotent: pre-encoded input stays encoded. --- bin/cloakserve | 4 +- cloakbrowser/browser.py | 94 +++++++++++++++++++++++++++++++++++------ js/src/proxy.ts | 92 +++++++++++++++++++++++++++++++++++++++- js/tests/proxy.test.ts | 63 ++++++++++++++++++++++++++- tests/test_proxy.py | 72 +++++++++++++++++++++++++++++++ 5 files changed, 306 insertions(+), 19 deletions(-) diff --git a/bin/cloakserve b/bin/cloakserve index 4d54833..c1e07a8 100755 --- a/bin/cloakserve +++ b/bin/cloakserve @@ -36,7 +36,7 @@ import aiohttp import websockets from aiohttp import web -from cloakbrowser.browser import build_args, maybe_resolve_geoip, _resolve_webrtc_args +from cloakbrowser.browser import build_args, maybe_resolve_geoip, _resolve_webrtc_args, _normalize_socks_string_url from cloakbrowser.download import ensure_binary logging.basicConfig( @@ -189,7 +189,7 @@ class ChromePool: if extra_args: fp_extra.extend(extra_args) if proxy: - fp_extra.append(f"--proxy-server={proxy}") + fp_extra.append(f"--proxy-server={_normalize_socks_string_url(proxy)}") # WebRTC IP spoofing: resolve auto, inject geoip exit IP fp_extra = _resolve_webrtc_args(fp_extra, proxy) diff --git a/cloakbrowser/browser.py b/cloakbrowser/browser.py index af00049..19a4b29 100644 --- a/cloakbrowser/browser.py +++ b/cloakbrowser/browser.py @@ -760,6 +760,37 @@ def _ensure_proxy_scheme(proxy_url: str) -> str: return proxy_url if "://" in proxy_url else f"http://{proxy_url}" +def _assemble_socks_url( + scheme: str, + host: str, + port: int | None, + enc_user: str, + enc_pass: str | None, + path: str = "", + params: str = "", + query: str = "", + fragment: str = "", +) -> str: + """Build a SOCKS URL from already-percent-encoded credentials and host parts. + + ``enc_pass is None`` means no password (no colon in userinfo). Empty string + means present-but-empty (colon preserved). This mirrors the distinction + urlparse makes between ``user@host`` and ``user:@host``. + """ + if ":" in host: # IPv6 literal — re-add brackets + host = f"[{host}]" + if enc_pass is not None: + userinfo = f"{enc_user}:{enc_pass}@" + elif enc_user: + userinfo = f"{enc_user}@" + else: + userinfo = "" + netloc = f"{userinfo}{host}" + if port is not None: + netloc += f":{port}" + return urlunparse((scheme, netloc, path, params, query, fragment)) + + def _reconstruct_socks_url(proxy: ProxySettings) -> str: """Reconstruct a SOCKS5 URL with inline credentials from a Playwright proxy dict.""" server = proxy.get("server", "") @@ -768,16 +799,47 @@ def _reconstruct_socks_url(proxy: ProxySettings) -> str: if not username: return server parsed = urlparse(server) - creds = quote(username, safe="") - if password: - creds += f":{quote(password, safe='')}" - host = parsed.hostname or "" - if ":" in host: # IPv6 literal — re-add brackets - host = f"[{host}]" - netloc = f"{creds}@{host}" - if parsed.port: - netloc += f":{parsed.port}" - return urlunparse((parsed.scheme, netloc, parsed.path, "", "", "")) + enc_user = quote(username, safe="") + # Dict convention: empty/missing password → no colon. + enc_pass = quote(password, safe="") if password else None + return _assemble_socks_url( + parsed.scheme, parsed.hostname or "", parsed.port, + enc_user, enc_pass, parsed.path, + ) + + +def _normalize_socks_string_url(url: str) -> str: + """Re-encode credentials in a SOCKS5 URL string so Chromium's parser doesn't + truncate them at special chars like '='. Idempotent: pre-encoded input stays + the same (decoded then re-encoded). + + On unparseable input (invalid port, broken IPv6 literal, etc.) logs a + warning and returns the original string — preserves pre-fix pass-through + behavior so Chromium's own error handling kicks in. + """ + try: + parsed = urlparse(url) + # Accessing .port raises ValueError on invalid port strings. + _ = parsed.port + except ValueError as e: + logger.warning("Malformed SOCKS5 proxy URL, passing through unchanged: %s", e) + return url + # Skip only if no credentials at all (username AND password both absent). + # urlparse returns None for absent components, "" for present-but-empty. + if parsed.username is None and parsed.password is None: + return url + enc_user = quote(unquote(parsed.username), safe="") if parsed.username else "" + # Preserve the colon separator when password component is present, even if + # empty, so `user:@host` stays `user:@host`. + if parsed.password is not None: + enc_pass = quote(unquote(parsed.password), safe="") if parsed.password else "" + else: + enc_pass = None + return _assemble_socks_url( + parsed.scheme, parsed.hostname or "", parsed.port, + enc_user, enc_pass, + parsed.path, parsed.params, parsed.query, parsed.fragment, + ) def _extract_proxy_url(proxy: str | ProxySettings | None) -> str | None: @@ -926,11 +988,14 @@ def build_args( def _parse_proxy_url(proxy: str) -> dict[str, Any]: - """Parse proxy URL, extracting credentials into separate Playwright fields. + """Parse HTTP(S) proxy URL, extracting credentials into separate Playwright fields. Handles: http://user:pass@host:port -> {server: "http://host:port", username: "user", password: "pass"} - Also handles: no credentials, URL-encoded special chars, socks5://, missing port, + Also handles: no credentials, URL-encoded special chars, missing port, and bare proxy strings without a scheme (e.g. 'user:pass@host:port' -> treated as http). + + SOCKS5 URLs are NOT handled here — they take a dedicated path via + ``_normalize_socks_string_url`` in ``_resolve_proxy_config``. """ # Bare format: "user:pass@host:port" — urlparse needs a scheme to extract credentials. normalized = proxy @@ -988,8 +1053,9 @@ def _resolve_proxy_config( if proxy.get("bypass"): extra_args.append(f"--proxy-bypass-list={proxy['bypass']}") return {}, extra_args - # String URL — pass as-is (Chrome handles user:pass@ in the URL) - return {}, [f"--proxy-server={proxy}"] + # String URL — re-encode creds to work around Chromium parser truncating + # passwords at '=' and other special chars (#157). + return {}, [f"--proxy-server={_normalize_socks_string_url(proxy)}"] # HTTP/HTTPS: use Playwright's proxy dict as before if isinstance(proxy, dict): diff --git a/js/src/proxy.ts b/js/src/proxy.ts index 7731c92..0b3ebe2 100644 --- a/js/src/proxy.ts +++ b/js/src/proxy.ts @@ -43,6 +43,40 @@ export function isSocksProxy(proxy: string | ProxyDict | undefined | null): bool return /^socks5h?:\/\//i.test(url); } +/** + * Build a SOCKS URL from already-percent-encoded credentials and a host suffix. + * + * `encPass === null` means no password (no colon in userinfo). Empty string + * means present-but-empty (colon preserved). + */ +function assembleSocksUrl( + scheme: string, + encUser: string, + encPass: string | null, + hostAndRest: string, +): string { + let userinfo: string; + if (encPass !== null) { + userinfo = `${encUser}:${encPass}@`; + } else if (encUser) { + userinfo = `${encUser}@`; + } else { + userinfo = ""; + } + return `${scheme}://${userinfo}${hostAndRest}`; +} + +/** + * Lenient percent-decode that handles malformed escapes gracefully, matching + * Python's ``urllib.parse.unquote``: valid ``%XX`` sequences are decoded, + * bare ``%`` not followed by two hex digits is left as a literal ``%``. + */ +function lenientDecodeURIComponent(s: string): string { + return s.replace(/%([0-9A-Fa-f]{2})|%/g, (match, hex) => + hex ? String.fromCharCode(parseInt(hex, 16)) : "%", + ); +} + /** * Reconstruct a SOCKS5 URL with inline credentials from a proxy dict. */ @@ -55,6 +89,60 @@ export function reconstructSocksUrl(proxy: ProxyDict): string { return url.href.replace(/\/$/, ""); } +/** + * Re-encode credentials in a SOCKS5 URL string so Chromium's parser doesn't + * truncate them at special chars like '='. Idempotent: pre-encoded input stays + * the same (decoded then re-encoded). + * + * Parsing is done manually rather than via `new URL` + setters, because WHATWG + * URL's username/password setters re-encode `%` on assignment, causing + * double-encoding when we round-trip decode-then-encode. + * + * On any unexpected failure, logs a warning and returns the original string + * so Chromium's own error handling can surface the real problem. + */ +export function normalizeSocksStringUrl(urlStr: string): string { + // Split userinfo from host at the LAST '@' (RFC 3986), so a raw '@' inside + // a password like `socks5://user:p@ss@host:1080` parses correctly. Matches + // Python urlparse's rpartition('@') behavior. + const schemeMatch = urlStr.match(/^([a-z][a-z0-9+\-.]*):\/\/(.*)$/i); + if (!schemeMatch) return urlStr; + const [, scheme, rest] = schemeMatch; + const hostStart = rest.search(/[/?#]/); + const authority = hostStart === -1 ? rest : rest.slice(0, hostStart); + const suffix = hostStart === -1 ? "" : rest.slice(hostStart); + const atIdx = authority.lastIndexOf("@"); + if (atIdx === -1) return urlStr; // no creds + const userinfo = authority.slice(0, atIdx); + const hostPart = authority.slice(atIdx + 1); + // Validate port (matches Python's urlparse().port ValueError guard). + // Extract port after last ':' — but skip IPv6 brackets (e.g. [::1]:1080). + const bracketEnd = hostPart.lastIndexOf("]"); + const portColonIdx = hostPart.indexOf(":", Math.max(bracketEnd, 0)); + if (portColonIdx !== -1) { + const portStr = hostPart.slice(portColonIdx + 1); + if (portStr && !/^\d+$/.test(portStr)) { + console.warn(`[cloakbrowser] Malformed SOCKS5 proxy URL, passing through unchanged: invalid port`); + return urlStr; + } + } + const hostAndRest = hostPart + suffix; + const colonIdx = userinfo.indexOf(":"); + const rawUserEnc = colonIdx === -1 ? userinfo : userinfo.slice(0, colonIdx); + const hasPassword = colonIdx !== -1; + const rawPassEnc = hasPassword ? userinfo.slice(colonIdx + 1) : ""; + try { + const encUser = rawUserEnc ? encodeURIComponent(lenientDecodeURIComponent(rawUserEnc)) : ""; + const encPass = hasPassword + ? (rawPassEnc ? encodeURIComponent(lenientDecodeURIComponent(rawPassEnc)) : "") + : null; + return assembleSocksUrl(scheme, encUser, encPass, hostAndRest); + } catch (e) { + console.warn(`[cloakbrowser] Could not normalize SOCKS5 proxy URL, passing through unchanged: ${(e as Error).message}`); + return urlStr; + } +} + /** * Resolve proxy into Playwright option and/or Chrome args. * @@ -67,7 +155,9 @@ export function resolveProxyConfig(proxy: string | ProxyDict | undefined): Proxy if (isSocksProxy(proxy)) { // SOCKS5: bypass Playwright, pass directly to Chrome via --proxy-server. if (typeof proxy === "string") { - return { proxyArgs: [`--proxy-server=${proxy}`] }; + // Re-encode creds to work around Chromium parser truncating passwords + // at '=' and other special chars (#157). + return { proxyArgs: [`--proxy-server=${normalizeSocksStringUrl(proxy)}`] }; } const socksUrl = reconstructSocksUrl(proxy); const args = [`--proxy-server=${socksUrl}`]; diff --git a/js/tests/proxy.test.ts b/js/tests/proxy.test.ts index 233a39c..d8aaade 100644 --- a/js/tests/proxy.test.ts +++ b/js/tests/proxy.test.ts @@ -191,8 +191,7 @@ describe("resolveProxyConfig", () => { password: "p@ss", }); expect(proxyOption).toBeUndefined(); - expect(proxyArgs.length).toBe(1); - expect(proxyArgs[0]).toContain("--proxy-server=socks5://user:p%40ss@host:1080"); + expect(proxyArgs).toEqual(["--proxy-server=socks5://user:p%40ss@host:1080"]); }); it("includes bypass for socks5 dict", () => { @@ -203,4 +202,64 @@ describe("resolveProxyConfig", () => { expect(proxyArgs).toContain("--proxy-server=socks5://host:1080"); expect(proxyArgs).toContain("--proxy-bypass-list=.example.com"); }); + + // Chromium's --proxy-server parser truncates passwords at '=' (#157). + // Wrapper must auto URL-encode before passing to Chrome. + it("encodes '=' in socks5 string password", () => { + const { proxyArgs } = resolveProxyConfig("socks5://user:pass=123@host:1080"); + expect(proxyArgs).toEqual(["--proxy-server=socks5://user:pass%3D123@host:1080"]); + }); + + it("encoding is idempotent for already-encoded socks5 string", () => { + const { proxyArgs } = resolveProxyConfig("socks5://user:pass%3D123@host:1080"); + expect(proxyArgs).toEqual(["--proxy-server=socks5://user:pass%3D123@host:1080"]); + }); + + it("leaves socks5 string without creds unchanged", () => { + const { proxyArgs } = resolveProxyConfig("socks5://host:1080"); + expect(proxyArgs).toEqual(["--proxy-server=socks5://host:1080"]); + }); + + it("encodes password even with empty username (password-only userinfo)", () => { + // Regression: empty-username bypass would skip encoding, leaving the + // Chromium truncation bug alive for this userinfo shape. + const { proxyArgs } = resolveProxyConfig("socks5://:pass=123@host:1080"); + expect(proxyArgs).toEqual(["--proxy-server=socks5://:pass%3D123@host:1080"]); + }); + + it("handles literal '%' in password without throwing (malformed escape)", () => { + // JS's decodeURIComponent throws on '%sure' (% not followed by 2 hex digits). + // Must fall back to treating '%' as literal and percent-encoding it. + const { proxyArgs } = resolveProxyConfig("socks5://user:100%sure@host:1080"); + expect(proxyArgs).toEqual(["--proxy-server=socks5://user:100%25sure@host:1080"]); + }); + + it("passes malformed SOCKS5 URLs through unchanged (no throw)", () => { + // Broken IPv6 bracket — wrapper must not throw; + // Chromium will surface its own error. + const { proxyArgs: a1 } = resolveProxyConfig("socks5://user:pass@[::1"); + expect(a1).toEqual(["--proxy-server=socks5://user:pass@[::1"]); + }); + + it("passes non-numeric port through unchanged", () => { + const { proxyArgs } = resolveProxyConfig("socks5://user:pass@host:abc"); + expect(proxyArgs).toEqual(["--proxy-server=socks5://user:pass@host:abc"]); + }); + + it("encodes special chars in IPv6 SOCKS5 string password", () => { + const { proxyArgs } = resolveProxyConfig("socks5://user:pass=eq@[::1]:1080"); + expect(proxyArgs).toEqual(["--proxy-server=socks5://user:pass%3Deq@[::1]:1080"]); + }); + + // Regression #157: userinfo must be split at the LAST '@' (RFC 3986), + // not the first, so raw '@' in a password parses correctly. + it("encodes raw '@' in socks5 string password (last-@ split)", () => { + const { proxyArgs } = resolveProxyConfig("socks5://user:p@ss@host:1080"); + expect(proxyArgs).toEqual(["--proxy-server=socks5://user:p%40ss@host:1080"]); + }); + + it("handles multiple raw '@' in password (splits at last)", () => { + const { proxyArgs } = resolveProxyConfig("socks5://user:a@b@c@host:1080"); + expect(proxyArgs).toEqual(["--proxy-server=socks5://user:a%40b%40c@host:1080"]); + }); }); diff --git a/tests/test_proxy.py b/tests/test_proxy.py index b3295ca..2e1fc9a 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -266,3 +266,75 @@ class TestResolveProxyConfig: assert kwargs == {} assert "--proxy-server=socks5://host:1080" in args assert "--proxy-bypass-list=.example.com" in args + + def test_socks5_string_encodes_equals_in_password(self): + # Chromium's --proxy-server parser truncates passwords at '=' (#157). + # Wrapper must auto URL-encode before passing to Chrome. + _, args = _resolve_proxy_config("socks5://user:pass=123@host:1080") + assert args == ["--proxy-server=socks5://user:pass%3D123@host:1080"] + + def test_socks5_string_encodes_at_in_password(self): + _, args = _resolve_proxy_config("socks5://user:p@ss@host:1080") + # Note: parsing "user:p@ss@host" — urlparse takes everything up to LAST @ + # as userinfo, so password = "p@ss". + assert args == ["--proxy-server=socks5://user:p%40ss@host:1080"] + + def test_socks5_string_encoding_idempotent(self): + # Already-encoded input should remain encoded (not double-encoded). + _, args = _resolve_proxy_config("socks5://user:pass%3D123@host:1080") + assert args == ["--proxy-server=socks5://user:pass%3D123@host:1080"] + + def test_socks5_string_no_creds_unchanged(self): + _, args = _resolve_proxy_config("socks5://host:1080") + assert args == ["--proxy-server=socks5://host:1080"] + + def test_socks5_string_password_only_still_encoded(self): + # Empty username with password: fix must still re-encode the password + # (regression test for empty-username bypass). + _, args = _resolve_proxy_config("socks5://:pass=123@host:1080") + assert args == ["--proxy-server=socks5://:pass%3D123@host:1080"] + + def test_socks5_string_empty_password_preserves_colon(self): + # `user:@host` (empty password) must NOT collapse to `user@host` — + # semantics differ between the two forms. + _, args = _resolve_proxy_config("socks5://user:@host:1080") + assert args == ["--proxy-server=socks5://user:@host:1080"] + + def test_socks5_string_literal_percent_in_password(self): + # Literal '%' not followed by 2 hex digits must be encoded as '%25' + # so Chrome decodes it back to '%'. Must not crash. + _, args = _resolve_proxy_config("socks5://user:100%sure@host:1080") + assert args == ["--proxy-server=socks5://user:100%25sure@host:1080"] + + def test_socks5_string_malformed_port_passes_through(self, caplog): + # Invalid port (non-numeric) raises in urlparse.port. Wrapper should + # log a warning and pass original through to Chromium. + import logging + with caplog.at_level(logging.WARNING, logger="cloakbrowser"): + _, args = _resolve_proxy_config("socks5://user:pass@host:abc") + assert args == ["--proxy-server=socks5://user:pass@host:abc"] + assert any("Malformed SOCKS5" in r.message for r in caplog.records) + + def test_socks5_string_malformed_ipv6_passes_through(self, caplog): + # Broken IPv6 bracket — must not crash, and must reach Chromium + # verbatim so its own error surfaces instead of a silent rewrite. + import logging + with caplog.at_level(logging.WARNING, logger="cloakbrowser"): + _, args = _resolve_proxy_config("socks5://user:pass@[::1") + assert args == ["--proxy-server=socks5://user:pass@[::1"] + + def test_socks5_string_preserves_path_and_query(self): + # Nonstandard for SOCKS5, but don't silently drop user-supplied suffixes. + # Matches JS behavior. + _, args = _resolve_proxy_config("socks5://user:pass@host:1080/p?x=1#f") + assert args[0] == "--proxy-server=socks5://user:pass@host:1080/p?x=1#f" + + def test_socks5_string_ipv6_with_special_char_password(self): + # IPv6 host + special char in password — both must be handled. + _, args = _resolve_proxy_config("socks5://user:pass=eq@[::1]:1080") + assert args[0] == "--proxy-server=socks5://user:pass%3Deq@[::1]:1080" + + def test_socks5_string_port_zero_preserved(self): + # Port 0 is an unusual but valid URL component; don't silently strip it. + _, args = _resolve_proxy_config("socks5://user:pass=1@host:0") + assert args[0] == "--proxy-server=socks5://user:pass%3D1@host:0"