Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a web request statistics service and enhances the auto-switch proxy mode by providing real-time detection feedback in the UI. Key changes include the addition of a WebRequestStatsService to track failed requests and active profiles per tab, a new AutoModeActionBar component for the popup, and a pacSimulator to evaluate PAC rules within the extension context. Additionally, the build system was updated with a new test mode, and several dependencies were upgraded. Feedback focuses on removing dead mocking code, improving the robustness of the shExpMatch regex pattern matching, and adding error handling to asynchronous indicator updates to prevent background worker crashes.
| // mocking | ||
| // tabStats.value = { | ||
| // failedRequests: [], | ||
| // currentProfile: { | ||
| // profile: { | ||
| // profileID: "profile1", | ||
| // profileName: "Profile 1", | ||
| // proxyType: "proxy", | ||
| // color: "#000000", | ||
| // proxyRules: { | ||
| // default: { | ||
| // host: "127.0.0.1", | ||
| // port: 8080, | ||
| // }, | ||
| // bypassList: [], | ||
| // }, | ||
| // }, | ||
| // isConfident: true, | ||
| // }, | ||
| // tabID: props.currentTab.id, | ||
| // }; | ||
| // console.log(tabStats.value); |
| export function shExpMatch(url: string, pattern: string) { | ||
| pattern = pattern.replace(/\./g, "\\."); | ||
| pattern = pattern.replace(/\*/g, ".*"); | ||
| pattern = pattern.replace(/\?/g, "."); | ||
| var newRe = new RegExp("^" + pattern + "$"); | ||
|
|
||
| return newRe.test(url); | ||
| } |
There was a problem hiding this comment.
The current implementation of shExpMatch is vulnerable to incorrect matching if the pattern contains regex special characters other than . and * (e.g., +, (, ), [, ], etc.). It is safer to escape all regex special characters before transforming the glob wildcards.
| export function shExpMatch(url: string, pattern: string) { | |
| pattern = pattern.replace(/\./g, "\\."); | |
| pattern = pattern.replace(/\*/g, ".*"); | |
| pattern = pattern.replace(/\?/g, "."); | |
| var newRe = new RegExp("^" + pattern + "$"); | |
| return newRe.test(url); | |
| } | |
| export function shExpMatch(url: string, pattern: string) { | |
| const escaped = pattern | |
| .replace(/[\\^$.+|()[\]{}]/g, "\\$&") | |
| .replace(/\*/g, ".*") | |
| .replace(/\?/g, "."); | |
| const newRe = new RegExp("^" + escaped + "$"); | |
| return newRe.test(url); | |
| } |
|
|
||
| if (ret.profile && ret.isConfident) { | ||
| const profile = ret.profile.profile; | ||
| setTimeout(() => setIndicator(profile, details.tabId), 50); // Sometimes the indicator doesn't update properly in Chrome, so we need to wait a bit. |
There was a problem hiding this comment.
The setIndicator call inside setTimeout should handle potential errors. If the tab is closed during the 50ms delay, the browser API will throw an error (e.g., 'No tab with id'), which could lead to an uncaught exception in the background service worker.
| setTimeout(() => setIndicator(profile, details.tabId), 50); // Sometimes the indicator doesn't update properly in Chrome, so we need to wait a bit. | |
| setTimeout(() => setIndicator(profile, details.tabId).catch(() => {}), 50); // Sometimes the indicator doesn't update properly in Chrome, so we need to wait a bit. |
- Fix critical `in` operator bug in refreshProxy() — `proxyType in ["system", "direct"]` always returned false; replaced with `["system", "direct"].includes(proxyType)` - Fix service worker crash: wrap `new URL(details.url)` in try/catch in background.ts - Fix document.body null access in preference.ts using optional chaining - Replace deepClone() JSON.parse/JSON.stringify with structuredClone() for correctness - Fix window.open() called with import.meta.url as target name; use '_blank' instead - Replace .map() with .forEach() for side-effect-only iterations (profile.ts, auth.ts) - Remove leftover debug console.log calls from ThemeSwitcher, AutoSwitchInput, background, PopupPage - Fix i18n typo: "Advance Config" → "Advanced Config" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
structuredClone() throws a DataCloneError on JavaScript Proxy objects, including Vue reactive() and ref() wrappers. The JSON.parse/JSON.stringify approach correctly serializes through the Proxy traps, producing a plain object safe for chrome.storage. Extend the comment to document why structuredClone() is intentionally avoided here, to prevent this mistake in future. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Provides build/test commands, architecture overview (adapter layer, PAC-via-AST proxy engine, profile system), and critical gotchas like the deepClone JSON round-trip requirement. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Work around Rolldown panic on nested member expressions of JSON imports by using createRequire for manifest.json. Fix newProxyString using the unnormalized cfg.scheme instead of the defaulted scheme variable. Clean up dead code (commented-out mocks, redundant comments, unused getter, console.debug). Add tests for deepClone, LRUCache, shExpMatch, isPlainHostName, newProxyString, and strengthen findProfile assertions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove @sentry/vue and @sentry/vite-plugin dependencies, Sentry.init() from main.ts, the vite plugin config, env variables, CI workflow refs, and the .env.sentry-build-plugin file. Reduces main.js bundle by ~32%. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`npm ci` on CI (Ubuntu) fails because `@emnapi/core@1.9.2` and `@emnapi/runtime@1.9.2` (transitive deps of the optional `@rolldown/binding-wasm32-wasi` package) were not resolved in the lockfile. This happened because macOS skips the wasm32-wasi optional package entirely during resolution. Regenerating the lockfile from scratch ensures all transitive dependencies are present. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- actions/checkout v4 → v6 - actions/setup-node v4 → v6 - actions/upload-artifact v4 → v7 - actions/download-artifact v4 → v8 - softprops/action-gh-release v2 → v3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
No description provided.