Skip to content

fix(fetch): keep proxy credentials as typed when they hold a stray % - #696

Open
goranai wants to merge 1 commit into
KnockOutEZ:mainfrom
goranai:fix/proxy-credentials-stray-percent
Open

goranai wants to merge 1 commit into
KnockOutEZ:mainfrom
goranai:fix/proxy-credentials-stray-percent

Conversation

@goranai

@goranai goranai commented Nov 11, 2026

Copy link
Copy Markdown

What & why

Fixes #695

playwrightProxyOption passed the proxy username and password to decodeURIComponent. That throws URIError on a % that is not a valid escape (50%off, p%, %2). The URL parser keeps such a % as it is, so the error reached both browser launch paths and every browser fetch failed while that proxy was set.

Changes

  • decodeUserinfo in src/fetch/proxy-credentials.ts decodes valid credentials exactly as before. If decodeURIComponent throws, it decodes each valid escape sequence on its own and keeps a stray % as typed. Decoding sequence by sequence matters because the URL parser has already escaped characters such as ", spaces and non-ASCII letters (pä%ss reaches us as p%C3%A4%ss). Returning that raw text would send the wrong password.
  • Behavior change: only inputs that used to throw are affected. Valid credentials decode as before (0 differences on 44059 valid inputs). If an escape sequence is not valid UTF-8 (for example %FF next to a stray %), it is kept as typed and not replaced with U+FFFD.
  • Tests: 3 cases in tests/unit/fetch/proxy-credentials.test.ts (2 stray-% cases, 1 valid-encoding guard) and 1 caller-level case in tests/unit/fetch/browser-pool.proxy.test.ts. The 3 stray-% cases fail on main with URIError and pass with the fix.
  • CHANGELOG line under Unreleased / Fixed.

Testing

  • npm test passes (not ticked: the full suite with a real browser has not run)
  • npm run lint passes
  • Added/updated tests for the new behavior

Local run 2026-11-12, Windows 11, Node 24.19.0, 2 workers: npm run lint is clean. npx vitest run tests/unit: 8129 tests, 8083 passed, 20 failed, 19 skipped. tests/unit/fetch: 74 files, 961 tests, all passed. The 20 failures are in 7 files outside fetch (cli/agents/skills, tui propagation, warmup json contract, plugins loader, repl shell, research synthesis-local). The same 20 tests fail on unchanged main on this Windows machine (same 7 files). Full npm test (integration and e2e with a real browser) was not run locally.

Checklist

  • Follows Conventional Commits
  • Focused change (no unrelated refactoring)
  • I have read CONTRIBUTING.md and agree to its contribution terms

Validated with Claude Opus 5.5.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 11, 2026

Copy link
Copy Markdown

Caution

Review failed

An error occurred during the review process. Please try again later.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Browser launch throws URIError when the proxy username or password contains a %

1 participant