refactor: decouple from site-specific CDN references #5
No files matched your search
@@ -1,23 +0,0 @@
|
|||||||
|
|
|||||||
{
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"permissions": {
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"allow": [
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(npm run *)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(node --check /home/kyle/Documents/goon_plugin/src/viewer/index.js)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(open -a \"Microsoft Edge\" --args --remote-debugging-port=9222 --profile-directory=Default)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(git checkout *)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(node --check src/viewer/index.js)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(node __tests__/run.mjs)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(node -e \"console.log\\(\\(require\\('./package.json'\\).scripts||{}\\).build || '\\(no build script\\)'\\)\")",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(git check-ignore *)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(node --check __tests__/simple_video.test.mjs)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(node --check __tests__/run.mjs)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"WebSearch",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(node *)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(python3 *)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(git stash *)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(npm -v)",
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
"Bash(npm install *)"
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
]
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
}
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
}
|
|
||||||
|
robot
commented
[SHOULD-FIX] F3 [SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
**Suggested fix:** Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
|
|||||||
@@ -2,4 +2,5 @@ node_modules/
|
|||||||
dist/
|
dist/
|
||||||
.vscode-insiders/
|
.vscode-insiders/
|
||||||
/icons
|
/icons
|
||||||
/icons/*
|
/icons/*
|
||||||
|
.claude/settings.local.json
|
||||||
@@ -193,8 +193,8 @@ describe('Data flow: popup -> storage -> viewer', () => {
|
|||||||
|
robot
commented
[FYI] F5 [FYI] F5
Consistent with the PR's goal ('no references in src/'), src/ is clean, but the NEW test files still reference the very providers being removed — phncdn.com in data_flow.test.mjs and referer_proxy.test.mjs, and thisvid.com in hls_qa.test.mjs / simple_video.test.mjs. These read as real-world fixtures, not logic coupling, so they do not reintroduce the coupling the PR removes; flagging for awareness / consistency.
**Suggested fix:** Optional: swap the real provider hostnames in test fixtures for neutral 'example.com' hosts to keep the spirit of the refactor consistent across the test suite.
robot
commented
[FYI] F5 [FYI] F5
Consistent with the PR's goal ('no references in src/'), src/ is clean, but the NEW test files still reference the very providers being removed — phncdn.com in data_flow.test.mjs and referer_proxy.test.mjs, and thisvid.com in hls_qa.test.mjs / simple_video.test.mjs. These read as real-world fixtures, not logic coupling, so they do not reintroduce the coupling the PR removes; flagging for awareness / consistency.
**Suggested fix:** Optional: swap the real provider hostnames in test fixtures for neutral 'example.com' hosts to keep the spirit of the refactor consistent across the test suite.
|
|||||||
// explicit property or storage-layer validation would reject the entry.
|
// explicit property or storage-layer validation would reject the entry.
|
||||||
const scrapedEntry = {
|
const scrapedEntry = {
|
||||||
url: 'https://pageA.example.com/watch',
|
url: 'https://pageA.example.com/watch',
|
||||||
src: 'https://hv-h.phncdn.com/hls/live/master.m3u8',
|
src: 'https://cdn.example.com/hls/live/master.m3u8',
|
||||||
|
robot
commented
[FYI] F5 [FYI] F5
Consistent with the PR's goal ('no references in src/'), src/ is clean, but the NEW test files still reference the very providers being removed — phncdn.com in data_flow.test.mjs and referer_proxy.test.mjs, and thisvid.com in hls_qa.test.mjs / simple_video.test.mjs. These read as real-world fixtures, not logic coupling, so they do not reintroduce the coupling the PR removes; flagging for awareness / consistency.
**Suggested fix:** Optional: swap the real provider hostnames in test fixtures for neutral 'example.com' hosts to keep the spirit of the refactor consistent across the test suite.
robot
commented
[FYI] F5 [FYI] F5
Consistent with the PR's goal ('no references in src/'), src/ is clean, but the NEW test files still reference the very providers being removed — phncdn.com in data_flow.test.mjs and referer_proxy.test.mjs, and thisvid.com in hls_qa.test.mjs / simple_video.test.mjs. These read as real-world fixtures, not logic coupling, so they do not reintroduce the coupling the PR removes; flagging for awareness / consistency.
**Suggested fix:** Optional: swap the real provider hostnames in test fixtures for neutral 'example.com' hosts to keep the spirit of the refactor consistent across the test suite.
|
|||||||
hlsManifest: 'https://hv-h.phncdn.com/hls/live/master.m3u8',
|
hlsManifest: 'https://cdn.example.com/hls/live/master.m3u8',
|
||||||
|
robot
commented
[FYI] F5 [FYI] F5
Consistent with the PR's goal ('no references in src/'), src/ is clean, but the NEW test files still reference the very providers being removed — phncdn.com in data_flow.test.mjs and referer_proxy.test.mjs, and thisvid.com in hls_qa.test.mjs / simple_video.test.mjs. These read as real-world fixtures, not logic coupling, so they do not reintroduce the coupling the PR removes; flagging for awareness / consistency.
**Suggested fix:** Optional: swap the real provider hostnames in test fixtures for neutral 'example.com' hosts to keep the spirit of the refactor consistent across the test suite.
robot
commented
[FYI] F5 [FYI] F5
Consistent with the PR's goal ('no references in src/'), src/ is clean, but the NEW test files still reference the very providers being removed — phncdn.com in data_flow.test.mjs and referer_proxy.test.mjs, and thisvid.com in hls_qa.test.mjs / simple_video.test.mjs. These read as real-world fixtures, not logic coupling, so they do not reintroduce the coupling the PR removes; flagging for awareness / consistency.
**Suggested fix:** Optional: swap the real provider hostnames in test fixtures for neutral 'example.com' hosts to keep the spirit of the refactor consistent across the test suite.
|
|||||||
mediaType: 'hls',
|
mediaType: 'hls',
|
||||||
poster: 'https://pageA.example.com/thumb.jpg',
|
poster: 'https://pageA.example.com/thumb.jpg',
|
||||||
tab: 'Referer-protected stream',
|
tab: 'Referer-protected stream',
|
||||||
|
|||||||
|
robot
commented
[FYI] F5 [FYI] F5
Consistent with the PR's goal ('no references in src/'), src/ is clean, but the NEW test files still reference the very providers being removed — phncdn.com in data_flow.test.mjs and referer_proxy.test.mjs, and thisvid.com in hls_qa.test.mjs / simple_video.test.mjs. These read as real-world fixtures, not logic coupling, so they do not reintroduce the coupling the PR removes; flagging for awareness / consistency.
**Suggested fix:** Optional: swap the real provider hostnames in test fixtures for neutral 'example.com' hosts to keep the spirit of the refactor consistent across the test suite.
robot
commented
[FYI] F5 [FYI] F5
Consistent with the PR's goal ('no references in src/'), src/ is clean, but the NEW test files still reference the very providers being removed — phncdn.com in data_flow.test.mjs and referer_proxy.test.mjs, and thisvid.com in hls_qa.test.mjs / simple_video.test.mjs. These read as real-world fixtures, not logic coupling, so they do not reintroduce the coupling the PR removes; flagging for awareness / consistency.
**Suggested fix:** Optional: swap the real provider hostnames in test fixtures for neutral 'example.com' hosts to keep the spirit of the refactor consistent across the test suite.
|
|||||||
@@ -1262,24 +1262,24 @@ describe('HLS QA: route-template / nav URL rejection', () => {
|
|||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Reproduces thisvid.com: pageContext.langUrls holds server-side route
|
// Reproduces a provider whose pageContext.langUrls holds server-side route
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
// TEMPLATES / nav links (e.g. /playlists/, /playlist/%1%/video/%2%/) that the
|
// TEMPLATES / nav links (e.g. /playlists/, /playlist/%1%/video/%2%/) that the
|
||||||
// old /playlist|/manifest substring check misclassified as "manifests".
|
// old /playlist|/manifest substring check misclassified as "manifests".
|
||||||
const TEMPLATE_URLS = [
|
const TEMPLATE_URLS = [
|
||||||
'https://thisvid.com/playlists/',
|
'https://example.com/playlists/',
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
'https://thisvid.com/playlists/liked/',
|
'https://example.com/playlists/liked/',
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
'https://thisvid.com/playlists/popular/',
|
'https://example.com/playlists/popular/',
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
'https://thisvid.com/playlists/commented/',
|
'https://example.com/playlists/commented/',
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
'https://thisvid.com/playlists/updated/',
|
'https://example.com/playlists/updated/',
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
'https://thisvid.com/playlists/largest/',
|
'https://example.com/playlists/largest/',
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
'https://thisvid.com/playlist/%1%/video/%2%/',
|
'https://example.com/playlist/%1%/video/%2%/',
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
'https://thisvid.com/playlists/?category=%DIR%',
|
'https://example.com/playlists/?category=%DIR%',
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
'https://thisvid.com/members/%ID%/playlists/',
|
'https://example.com/members/%ID%/playlists/',
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
'https://thisvid.com/my/playlists/%ID%/',
|
'https://example.com/my/playlists/%ID%/',
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
];
|
];
|
||||||
|
|
||||||
it('route templates / nav links are NOT reported as manifests', () => {
|
it('route templates / nav links are NOT reported as manifests', () => {
|
||||||
const result = runProbeWin('https://thisvid.com/videos/redneck/', {
|
const result = runProbeWin('https://example.com/videos/redneck/', {
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
pageContext: { langUrls: TEMPLATE_URLS },
|
pageContext: { langUrls: TEMPLATE_URLS },
|
||||||
kvsplayer: { config: { langUrls: TEMPLATE_URLS } },
|
kvsplayer: { config: { langUrls: TEMPLATE_URLS } },
|
||||||
__goon_plugin_data: { manifests: [], sources: [] },
|
__goon_plugin_data: { manifests: [], sources: [] },
|
||||||
@@ -1294,7 +1294,7 @@ describe('HLS QA: route-template / nav URL rejection', () => {
|
|||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
it('a real .m3u8 with a %2B token is still found (no over-rejection)', () => {
|
it('a real .m3u8 with a %2B token is still found (no over-rejection)', () => {
|
||||||
// %2B is a valid percent-encoded token, not a placeholder — it must survive.
|
// %2B is a valid percent-encoded token, not a placeholder — it must survive.
|
||||||
const real = 'https://cdn.example.com/live/master.m3u8?h=a%2Bb%2Fc&e=1234';
|
const real = 'https://cdn.example.com/live/master.m3u8?h=a%2Bb%2Fc&e=1234';
|
||||||
const result = runProbeWin('https://thisvid.com/videos/redneck/', {
|
const result = runProbeWin('https://example.com/videos/redneck/', {
|
||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
hls: { url: real },
|
hls: { url: real },
|
||||||
pageContext: { langUrls: TEMPLATE_URLS },
|
pageContext: { langUrls: TEMPLATE_URLS },
|
||||||
});
|
});
|
||||||
|
|||||||
|
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
robot
commented
[BLOCKING] F1 [BLOCKING] F1
This new suite hangs the entire `npm test` runner (>30s at 100% CPU, exit 124; reproduced in isolation). runProbeWin evaluates m3u8DeepProbeFn in a live JSDOM window; Strategy H (src/viewer/index.js:484) does `for (const key of Object.keys(window)) { walk(window[key], ...) }` and window/_parent/_top/frames/_globalObject/_globalProxy are circular Proxy objects. `walk` (index.js:161) has a depth cap of 5 but NO visited-set / cycle detection, so it recurses unboundedly over thousands of proxied window keys and never returns. Because __tests__/run.mjs loads hls_qa.test.mjs, `npm test` never completes -> the repo's canonical test command is broken.
**Suggested fix:** Give the walker a visited set (WeakSet) so each object is walked at most once: `const seen = new WeakSet(); function walk(obj,...){ if (obj && typeof obj==='object'){ if(seen.has(obj)) return; seen.add(obj);} ... }`. Additionally cap Strategy H's window-key scan (window has hundreds of keys; only the handful of genuine player globals matter) — e.g. skip built-in global keys or cap the scan. Alternatively, scope the new test so it stubs only the window globals under test (pageContext, kvsplayer, __goon_plugin_data) instead of a full JSDOM window, avoiding the circular globals entirely.
|
|||||||
@@ -252,26 +252,26 @@ describe('PR#5: per-host referer capture + proxy race', () => {
|
|||||||
});
|
});
|
||||||
await loadBackground(env);
|
await loadBackground(env);
|
||||||
|
|
||||||
// Capture the provider referer for hv-h.phncdn.com (one edge subdomain).
|
// Capture the provider referer for hv-h.example.net (one edge subdomain).
|
||||||
await capture(env, 'https://hv-h.phncdn.com/hls/live/seg-1.ts', 'https://provider.example.com/watch');
|
await capture(env, 'https://hv-h.example.net/hls/live/seg-1.ts', 'https://provider.example.com/watch');
|
||||||
|
|
||||||
// Proxy a SIBLING subdomain ev-h.phncdn.com with NO referer in the message:
|
// Proxy a SIBLING subdomain ev-h.example.net with NO referer in the message:
|
||||||
// the SW must resolve it from the domain-keyed store.
|
// the SW must resolve it from the domain-keyed store.
|
||||||
const handler = env.handlers.onMessage[env.handlers.onMessage.length - 1];
|
const handler = env.handlers.onMessage[env.handlers.onMessage.length - 1];
|
||||||
let resp;
|
let resp;
|
||||||
handler({ type: 'GOON_PROXY_FETCH', url: 'https://ev-h.phncdn.com/hls/live/seg-1.ts' }, {}, (r) => { resp = r; });
|
handler({ type: 'GOON_PROXY_FETCH', url: 'https://ev-h.example.net/hls/live/seg-1.ts' }, {}, (r) => { resp = r; });
|
||||||
await new Promise(r => setTimeout(r, 20));
|
await new Promise(r => setTimeout(r, 20));
|
||||||
|
|
||||||
if (!resp || !resp.ok) throw new Error('sibling proxy failed: ' + JSON.stringify(resp));
|
if (!resp || !resp.ok) throw new Error('sibling proxy failed: ' + JSON.stringify(resp));
|
||||||
const rules = env.getSessionRules();
|
const rules = env.getSessionRules();
|
||||||
const rule = Object.values(rules).find(r => r.condition?.requestDomains?.includes('phncdn.com'));
|
const rule = Object.values(rules).find(r => r.condition?.requestDomains?.includes('example.net'));
|
||||||
if (!rule) throw new Error('no domain-scoped DNR rule for phncdn.com; rules=' + JSON.stringify(rules));
|
if (!rule) throw new Error('no domain-scoped DNR rule for example.net; rules=' + JSON.stringify(rules));
|
||||||
const ref = rule.action.requestHeaders?.find(h => h.header.toLowerCase() === 'referer')?.value;
|
const ref = rule.action.requestHeaders?.find(h => h.header.toLowerCase() === 'referer')?.value;
|
||||||
if (ref !== 'https://provider.example.com/watch') {
|
if (ref !== 'https://provider.example.com/watch') {
|
||||||
throw new Error('sibling edge served wrong referer: ' + ref);
|
throw new Error('sibling edge served wrong referer: ' + ref);
|
||||||
}
|
}
|
||||||
// The rule is scoped to the registrable domain, NOT the exact subdomain.
|
// The rule is scoped to the registrable domain, NOT the exact subdomain.
|
||||||
if (rule.condition.requestDomains?.includes('ev-h.phncdn.com')) {
|
if (rule.condition.requestDomains?.includes('ev-h.example.net')) {
|
||||||
throw new Error('rule scoped to exact subdomain instead of registrable domain');
|
throw new Error('rule scoped to exact subdomain instead of registrable domain');
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
@@ -313,23 +313,23 @@ describe('PR#5: per-host referer capture + proxy race', () => {
|
|||||||
global.chrome = env.chrome;
|
global.chrome = env.chrome;
|
||||||
await loadBackground(env);
|
await loadBackground(env);
|
||||||
|
|
||||||
// Capture for hv-h.phncdn.com only.
|
// Capture for hv-h.example.net only.
|
||||||
await capture(env, 'https://hv-h.phncdn.com/hls/live/seg-1.ts', 'https://provider.example.com/watch');
|
await capture(env, 'https://hv-h.example.net/hls/live/seg-1.ts', 'https://provider.example.com/watch');
|
||||||
|
|
||||||
const handler = env.handlers.onMessage[env.handlers.onMessage.length - 1];
|
const handler = env.handlers.onMessage[env.handlers.onMessage.length - 1];
|
||||||
let resp;
|
let resp;
|
||||||
handler(
|
handler(
|
||||||
{ type: 'GOON_GET_REFERERS', hosts: ['ev-h.phncdn.com', 'hv-h.phncdn.com', 'cdnA.example.com'] },
|
{ type: 'GOON_GET_REFERERS', hosts: ['ev-h.example.net', 'hv-h.example.net', 'cdnA.example.com'] },
|
||||||
{},
|
{},
|
||||||
(r) => { resp = r; });
|
(r) => { resp = r; });
|
||||||
await new Promise(r => setTimeout(r, 15));
|
await new Promise(r => setTimeout(r, 15));
|
||||||
|
|
||||||
if (!resp || !resp.ok) throw new Error('GOON_GET_REFERERS failed: ' + JSON.stringify(resp));
|
if (!resp || !resp.ok) throw new Error('GOON_GET_REFERERS failed: ' + JSON.stringify(resp));
|
||||||
const referers = resp.referers || {};
|
const referers = resp.referers || {};
|
||||||
if (referers['ev-h.phncdn.com'] !== 'https://provider.example.com/watch') {
|
if (referers['ev-h.example.net'] !== 'https://provider.example.com/watch') {
|
||||||
throw new Error('sibling subdomain did not resolve the captured referer: ' + JSON.stringify(referers));
|
throw new Error('sibling subdomain did not resolve the captured referer: ' + JSON.stringify(referers));
|
||||||
}
|
}
|
||||||
if (referers['hv-h.phncdn.com'] !== 'https://provider.example.com/watch') {
|
if (referers['hv-h.example.net'] !== 'https://provider.example.com/watch') {
|
||||||
throw new Error('captured subdomain did not resolve its own referer: ' + JSON.stringify(referers));
|
throw new Error('captured subdomain did not resolve its own referer: ' + JSON.stringify(referers));
|
||||||
}
|
}
|
||||||
if ('cdnA.example.com' in referers) {
|
if ('cdnA.example.com' in referers) {
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
// Regression tests: simple <video> / direct-media detection.
|
// Regression tests: simple <video> / direct-media detection.
|
||||||
//
|
//
|
||||||
// The feature branch broke detection of pages that expose a plain <video>
|
// The feature branch broke detection of pages that expose a plain <video>
|
||||||
// (e.g. thisvid.com, which serves a direct MP4 with a TRAILING SLASH and a
|
// (e.g. a site that serves a direct MP4 with a TRAILING SLASH and a
|
||||||
// ?rnd= query: .../14644093/14644093.mp4/?rnd=...). Two site-agnostic fixes
|
// ?rnd= query: .../14644093/14644093.mp4/?rnd=...). Two site-agnostic fixes
|
||||||
// are guarded here:
|
// are guarded here:
|
||||||
// 1) The URL predicate (isValidManifestUrl) must let concrete media signals
|
// 1) The URL predicate (isValidManifestUrl) must let concrete media signals
|
||||||
@@ -9,7 +9,7 @@
|
|||||||
// slash rejections, so a real direct-media URL is not mistaken for a
|
// slash rejections, so a real direct-media URL is not mistaken for a
|
||||||
// route template.
|
// route template.
|
||||||
// 2) scrapeTab must fall back to an ISOLATED-world scrape when the MAIN-world
|
// 2) scrapeTab must fall back to an ISOLATED-world scrape when the MAIN-world
|
||||||
// scrape yields nothing — the runtime cause of "detects nothing" on thisvid
|
// scrape yields nothing — the runtime cause of "detects nothing" on such pages
|
||||||
// is a MAIN-world + allFrames rejection on its ad-iframe-heavy page.
|
// is a MAIN-world + allFrames rejection on its ad-iframe-heavy page.
|
||||||
import { describe, it } from './runner.mjs';
|
import { describe, it } from './runner.mjs';
|
||||||
import { JSDOM } from 'jsdom';
|
import { JSDOM } from 'jsdom';
|
||||||
@@ -40,12 +40,12 @@ function extractFn(src, name) {
|
|||||||
throw new Error('unbalanced: ' + name);
|
throw new Error('unbalanced: ' + name);
|
||||||
}
|
}
|
||||||
|
|
||||||
// The thisvid direct-media URL, exactly as served (note the trailing slash +
|
// The direct-media URL, shaped exactly like the rejected case (a trailing slash +
|
||||||
// ?rnd= query). This is the concrete case the predicate used to reject.
|
// ?rnd= query). This is the concrete case the predicate used to reject.
|
||||||
const THISVID_MP4 =
|
const DIRECT_MP4 =
|
||||||
'https://thisvid.com/get_file/5/6cae22f8b3ba9d74f10cc2f8d060af738c8d30ddf5/14644000/14644093/14644093.mp4/?rnd=1787811205406';
|
'https://example.com/get_file/5/6cae22f8b3ba9d74f10cc2f8d060af738c8d30ddf5/14644000/14644093/14644093.mp4/?rnd=1787811205406';
|
||||||
|
|
||||||
describe('HLS QA: simple <video> direct-media predicate (thisvid regression)', () => {
|
describe('HLS QA: simple <video> direct-media predicate', () => {
|
||||||
// Extract and evaluate the REAL isValidManifestUrl from index.js in a browser
|
// Extract and evaluate the REAL isValidManifestUrl from index.js in a browser
|
||||||
// window so its `new URL(...)` / string behavior matches production.
|
// window so its `new URL(...)` / string behavior matches production.
|
||||||
const isValidSrc = extractFn(indexSource, 'isValidManifestUrl');
|
const isValidSrc = extractFn(indexSource, 'isValidManifestUrl');
|
||||||
@@ -53,7 +53,7 @@ describe('HLS QA: simple <video> direct-media predicate (thisvid regression)', (
|
|||||||
function assertValid(url, expectValid, label) {
|
function assertValid(url, expectValid, label) {
|
||||||
const dom = new JSDOM(
|
const dom = new JSDOM(
|
||||||
'<!DOCTYPE html><html><body></body></html>',
|
'<!DOCTYPE html><html><body></body></html>',
|
||||||
{ url: 'https://thisvid.com/videos/x/', runScripts: 'dangerously' }
|
{ url: 'https://example.com/videos/x/', runScripts: 'dangerously' }
|
||||||
);
|
);
|
||||||
const got = dom.window.eval('(' + isValidSrc + ')(' + JSON.stringify(url) + ')');
|
const got = dom.window.eval('(' + isValidSrc + ')(' + JSON.stringify(url) + ')');
|
||||||
if (got !== expectValid) {
|
if (got !== expectValid) {
|
||||||
@@ -63,9 +63,9 @@ describe('HLS QA: simple <video> direct-media predicate (thisvid regression)', (
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
it('accepts thisvid\'s real direct-MP4 URL WITH a trailing slash + ?rnd=', () => {
|
it('accepts a real direct-MP4 URL WITH a trailing slash + ?rnd=', () => {
|
||||||
// Before the fix, `path.endsWith('/')` rejected this real media source.
|
// Before the fix, `path.endsWith('/')` rejected this real media source.
|
||||||
assertValid(THISVID_MP4, true, 'thisvid trailing-slash MP4');
|
assertValid(DIRECT_MP4, true, 'trailing-slash MP4');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('accepts a direct .mp4 with no trailing slash', () => {
|
it('accepts a direct .mp4 with no trailing slash', () => {
|
||||||
@@ -86,18 +86,18 @@ describe('HLS QA: simple <video> direct-media predicate (thisvid regression)', (
|
|||||||
assertValid('https://cdn.example.com/live/master.m3u8?h=a%2Bb%2Fc&e=1234', true, '%2B token');
|
assertValid('https://cdn.example.com/live/master.m3u8?h=a%2Bb%2Fc&e=1234', true, '%2B token');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('still rejects thisvid route-template / nav URLs (no over-acceptance)', () => {
|
it('still rejects route-template / nav URLs (no over-acceptance)', () => {
|
||||||
// The fix must not trade one regression for the other: the route templates
|
// The fix must not trade one regression for the other: the route templates
|
||||||
// thisvid\'s pageContext.langUrls leaks must still be rejected.
|
// a provider's pageContext.langUrls leaks must still be rejected.
|
||||||
assertValid('https://thisvid.com/playlists/', false, '/playlists/');
|
assertValid('https://example.com/playlists/', false, '/playlists/');
|
||||||
assertValid('https://thisvid.com/playlists/liked/', false, '/playlists/liked/');
|
assertValid('https://example.com/playlists/liked/', false, '/playlists/liked/');
|
||||||
assertValid('https://thisvid.com/playlist/%1%/video/%2%/', false, '/playlist/%1%/');
|
assertValid('https://example.com/playlist/%1%/video/%2%/', false, '/playlist/%1%/');
|
||||||
assertValid('https://thisvid.com/members/%ID%/playlists/', false, '/members/%ID%/playlists/');
|
assertValid('https://example.com/members/%ID%/playlists/', false, '/members/%ID%/playlists/');
|
||||||
assertValid('https://thisvid.com/videos/%DIR%/', false, '/videos/%DIR%/');
|
assertValid('https://example.com/videos/%DIR%/', false, '/videos/%DIR%/');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('HLS QA: scrapeTab isolated-world fallback (thisvid regression)', () => {
|
describe('HLS QA: scrapeTab isolated-world fallback', () => {
|
||||||
// Extract the whole scrapeTab function (balanced braces, indentation-agnostic).
|
// Extract the whole scrapeTab function (balanced braces, indentation-agnostic).
|
||||||
const scrapeSrc = extractFn(indexSource, 'scrapeTab');
|
const scrapeSrc = extractFn(indexSource, 'scrapeTab');
|
||||||
|
|
||||||
|
|||||||
@@ -7,9 +7,6 @@
|
|||||||
|
robot
commented
[FYI] F4 [FYI] F4
The PR also churns package-lock.json: it adds a top-level 'dependencies': {@rollup/rollup-darwin-arm64:'^4.60.4'} to the root '' entry and clears the 'dev':true/'optional':true flags on the darwin-arm64 package, while package.json now has an empty 'dependencies': {}. This looks like an npm-install side effect on the author's ARM-Mac that leaked in, unrelated to the decoupling refactor. Build still passes locally (copy-hlds.mjs copies the hls bundle), so it is not a functional break, but the lock churn is noise and a platform-specific optional dep should not be promoted to a hard dependency.
**Suggested fix:** Re-generate package-lock.json on a clean install and avoid forcing @rollup/rollup-darwin-arm64 into 'dependencies'; keep it as the platform-optional dev dep it already is, or drop it entirely.
robot
commented
[FYI] F4 [FYI] F4
The PR also churns package-lock.json: it adds a top-level 'dependencies': {@rollup/rollup-darwin-arm64:'^4.60.4'} to the root '' entry and clears the 'dev':true/'optional':true flags on the darwin-arm64 package, while package.json now has an empty 'dependencies': {}. This looks like an npm-install side effect on the author's ARM-Mac that leaked in, unrelated to the decoupling refactor. Build still passes locally (copy-hlds.mjs copies the hls bundle), so it is not a functional break, but the lock churn is noise and a platform-specific optional dep should not be promoted to a hard dependency.
**Suggested fix:** Re-generate package-lock.json on a clean install and avoid forcing @rollup/rollup-darwin-arm64 into 'dependencies'; keep it as the platform-optional dev dep it already is, or drop it entirely.
|
|||||||
"": {
|
"": {
|
||||||
"name": "vidplex",
|
"name": "vidplex",
|
||||||
"version": "0.1.0",
|
"version": "0.1.0",
|
||||||
"dependencies": {
|
|
||||||
|
robot
commented
[FYI] F4 [FYI] F4
The PR also churns package-lock.json: it adds a top-level 'dependencies': {@rollup/rollup-darwin-arm64:'^4.60.4'} to the root '' entry and clears the 'dev':true/'optional':true flags on the darwin-arm64 package, while package.json now has an empty 'dependencies': {}. This looks like an npm-install side effect on the author's ARM-Mac that leaked in, unrelated to the decoupling refactor. Build still passes locally (copy-hlds.mjs copies the hls bundle), so it is not a functional break, but the lock churn is noise and a platform-specific optional dep should not be promoted to a hard dependency.
**Suggested fix:** Re-generate package-lock.json on a clean install and avoid forcing @rollup/rollup-darwin-arm64 into 'dependencies'; keep it as the platform-optional dev dep it already is, or drop it entirely.
|
|||||||
"@rollup/rollup-darwin-arm64": "^4.60.4"
|
|
||||||
|
robot
commented
[FYI] F4 [FYI] F4
The PR also churns package-lock.json: it adds a top-level 'dependencies': {@rollup/rollup-darwin-arm64:'^4.60.4'} to the root '' entry and clears the 'dev':true/'optional':true flags on the darwin-arm64 package, while package.json now has an empty 'dependencies': {}. This looks like an npm-install side effect on the author's ARM-Mac that leaked in, unrelated to the decoupling refactor. Build still passes locally (copy-hlds.mjs copies the hls bundle), so it is not a functional break, but the lock churn is noise and a platform-specific optional dep should not be promoted to a hard dependency.
**Suggested fix:** Re-generate package-lock.json on a clean install and avoid forcing @rollup/rollup-darwin-arm64 into 'dependencies'; keep it as the platform-optional dev dep it already is, or drop it entirely.
|
|||||||
},
|
|
||||||
|
robot
commented
[FYI] F4 [FYI] F4
The PR also churns package-lock.json: it adds a top-level 'dependencies': {@rollup/rollup-darwin-arm64:'^4.60.4'} to the root '' entry and clears the 'dev':true/'optional':true flags on the darwin-arm64 package, while package.json now has an empty 'dependencies': {}. This looks like an npm-install side effect on the author's ARM-Mac that leaked in, unrelated to the decoupling refactor. Build still passes locally (copy-hlds.mjs copies the hls bundle), so it is not a functional break, but the lock churn is noise and a platform-specific optional dep should not be promoted to a hard dependency.
**Suggested fix:** Re-generate package-lock.json on a clean install and avoid forcing @rollup/rollup-darwin-arm64 into 'dependencies'; keep it as the platform-optional dev dep it already is, or drop it entirely.
|
|||||||
"devDependencies": {
|
"devDependencies": {
|
||||||
"ajv": "^8.17.1",
|
"ajv": "^8.17.1",
|
||||||
"ajv-formats": "^3.0.1",
|
"ajv-formats": "^3.0.1",
|
||||||
@@ -1344,7 +1341,9 @@
|
|||||||
|
robot
commented
[FYI] F4 [FYI] F4
The PR also churns package-lock.json: it adds a top-level 'dependencies': {@rollup/rollup-darwin-arm64:'^4.60.4'} to the root '' entry and clears the 'dev':true/'optional':true flags on the darwin-arm64 package, while package.json now has an empty 'dependencies': {}. This looks like an npm-install side effect on the author's ARM-Mac that leaked in, unrelated to the decoupling refactor. Build still passes locally (copy-hlds.mjs copies the hls bundle), so it is not a functional break, but the lock churn is noise and a platform-specific optional dep should not be promoted to a hard dependency.
**Suggested fix:** Re-generate package-lock.json on a clean install and avoid forcing @rollup/rollup-darwin-arm64 into 'dependencies'; keep it as the platform-optional dev dep it already is, or drop it entirely.
robot
commented
[FYI] F4 [FYI] F4
The PR also churns package-lock.json: it adds a top-level 'dependencies': {@rollup/rollup-darwin-arm64:'^4.60.4'} to the root '' entry and clears the 'dev':true/'optional':true flags on the darwin-arm64 package, while package.json now has an empty 'dependencies': {}. This looks like an npm-install side effect on the author's ARM-Mac that leaked in, unrelated to the decoupling refactor. Build still passes locally (copy-hlds.mjs copies the hls bundle), so it is not a functional break, but the lock churn is noise and a platform-specific optional dep should not be promoted to a hard dependency.
**Suggested fix:** Re-generate package-lock.json on a clean install and avoid forcing @rollup/rollup-darwin-arm64 into 'dependencies'; keep it as the platform-optional dev dep it already is, or drop it entirely.
|
|||||||
"cpu": [
|
"cpu": [
|
||||||
"arm64"
|
"arm64"
|
||||||
],
|
],
|
||||||
|
"dev": true,
|
||||||
|
robot
commented
[FYI] F4 [FYI] F4
The PR also churns package-lock.json: it adds a top-level 'dependencies': {@rollup/rollup-darwin-arm64:'^4.60.4'} to the root '' entry and clears the 'dev':true/'optional':true flags on the darwin-arm64 package, while package.json now has an empty 'dependencies': {}. This looks like an npm-install side effect on the author's ARM-Mac that leaked in, unrelated to the decoupling refactor. Build still passes locally (copy-hlds.mjs copies the hls bundle), so it is not a functional break, but the lock churn is noise and a platform-specific optional dep should not be promoted to a hard dependency.
**Suggested fix:** Re-generate package-lock.json on a clean install and avoid forcing @rollup/rollup-darwin-arm64 into 'dependencies'; keep it as the platform-optional dev dep it already is, or drop it entirely.
|
|||||||
"license": "MIT",
|
"license": "MIT",
|
||||||
|
"optional": true,
|
||||||
|
robot
commented
[FYI] F4 [FYI] F4
The PR also churns package-lock.json: it adds a top-level 'dependencies': {@rollup/rollup-darwin-arm64:'^4.60.4'} to the root '' entry and clears the 'dev':true/'optional':true flags on the darwin-arm64 package, while package.json now has an empty 'dependencies': {}. This looks like an npm-install side effect on the author's ARM-Mac that leaked in, unrelated to the decoupling refactor. Build still passes locally (copy-hlds.mjs copies the hls bundle), so it is not a functional break, but the lock churn is noise and a platform-specific optional dep should not be promoted to a hard dependency.
**Suggested fix:** Re-generate package-lock.json on a clean install and avoid forcing @rollup/rollup-darwin-arm64 into 'dependencies'; keep it as the platform-optional dev dep it already is, or drop it entirely.
|
|||||||
"os": [
|
"os": [
|
||||||
"darwin"
|
"darwin"
|
||||||
]
|
]
|
||||||
|
|||||||
|
robot
commented
[FYI] F4 [FYI] F4
The PR also churns package-lock.json: it adds a top-level 'dependencies': {@rollup/rollup-darwin-arm64:'^4.60.4'} to the root '' entry and clears the 'dev':true/'optional':true flags on the darwin-arm64 package, while package.json now has an empty 'dependencies': {}. This looks like an npm-install side effect on the author's ARM-Mac that leaked in, unrelated to the decoupling refactor. Build still passes locally (copy-hlds.mjs copies the hls bundle), so it is not a functional break, but the lock churn is noise and a platform-specific optional dep should not be promoted to a hard dependency.
**Suggested fix:** Re-generate package-lock.json on a clean install and avoid forcing @rollup/rollup-darwin-arm64 into 'dependencies'; keep it as the platform-optional dev dep it already is, or drop it entirely.
robot
commented
[FYI] F4 [FYI] F4
The PR also churns package-lock.json: it adds a top-level 'dependencies': {@rollup/rollup-darwin-arm64:'^4.60.4'} to the root '' entry and clears the 'dev':true/'optional':true flags on the darwin-arm64 package, while package.json now has an empty 'dependencies': {}. This looks like an npm-install side effect on the author's ARM-Mac that leaked in, unrelated to the decoupling refactor. Build still passes locally (copy-hlds.mjs copies the hls bundle), so it is not a functional break, but the lock churn is noise and a platform-specific optional dep should not be promoted to a hard dependency.
**Suggested fix:** Re-generate package-lock.json on a clean install and avoid forcing @rollup/rollup-darwin-arm64 into 'dependencies'; keep it as the platform-optional dev dep it already is, or drop it entirely.
|
|||||||
@@ -307,12 +307,19 @@ chrome.tabs.onRemoved.addListener(async (tabId) => {
|
|||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
//
|
//
|
||||||
// We cannot hardcode a target referer: the proxy is invoked from the viewer
|
// We cannot hardcode a target referer: the proxy is invoked from the viewer
|
||||||
// (a different origin than the source page), and a DNR rule cannot read a live
|
// (a different origin than the source page), and a DNR rule cannot read a live
|
||||||
// page's origin. So we capture the referer that the REAL page uses for its video
|
// page's origin. So we derive the provider referer from the source tab's own URL.
|
||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
// requests — via chrome.webRequest.onBeforeSendHeaders, which sees the request
|
// For a cross-origin video request the browser sends the referring page's URL as
|
||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
// headers the browser already built — and persist it per CDN host. The
|
// the Referer, and that page IS the tab currently loading the video — so the tab
|
||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
|
// URL is the referer we need. We read it via chrome.webRequest.onBeforeRequest
|
||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
|
// (no header access — it cannot read the request headers) plus chrome.tabs.get,
|
||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
|
// which returns the source tab's URL, and persist that per CDN host. The
|
||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
// GOON_PROXY_FETCH handler then installs a DNR rule that re-inserts that
|
// GOON_PROXY_FETCH handler then installs a DNR rule that re-inserts that
|
||||||
// captured referer for just that host, so the proxy's fetch() is accepted.
|
// captured referer for just that host, so the proxy's fetch() is accepted.
|
||||||
// This requires no knowledge of any particular site.
|
// This requires no knowledge of any particular site. Deriving the referer from
|
||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
|
// the tab URL (rather than reading the real request header via
|
||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
|
// onBeforeSendHeaders + extraHeaders) is intentional: extraInfoSpec header
|
||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
|
// parsing is rejected by some Chromium builds and fails the whole service worker,
|
||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
|
// and the tabs + host permissions already permit chrome.tabs.get.
|
||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
//
|
//
|
||||||
// Each CDN host gets its OWN declarativeNetRequest rule id so that concurrent
|
// Each CDN host gets its OWN declarativeNetRequest rule id so that concurrent
|
||||||
// proxy fetches for different hosts can't clobber each other's rule (a single
|
// proxy fetches for different hosts can't clobber each other's rule (a single
|
||||||
|
|||||||
|
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal. Blocking: 'goon_referers' is a host-keyed GLOBAL store meant to outlive its capture tab (see the docstring just above and the PR body: "persist it per CDN host ... even after this capture tab closes"). Adding it to the per-tab onRemoved cleanup means closing ANY tab wipes referers captured by other still-open source tabs, so the next proxy fetch for those hosts loses its referer and 412s. Remove 'goon_referers' from this array (keep the per-tab vr_requests_ / wr_manifests_ keys). If you want cleanup, key it per host with an expiry instead of on every tab removal.
robot
commented
Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue). Concurrent-proxy race: REFERER_RULE_ID is a single shared id and the idempotency check + updateSessionRules({removeRuleIds, addRules}) are not atomic. Two hosts proxied in parallel (common in the multi-panel grid) can interleave so the live rule ends up host B's while refererRuleInstalled records host A as installed -> host A's fetches get host B's referer -> 412. Now that the rule is per-host, use a distinct rule id per host (or add all hosts' rules in one updateSessionRules call) and make the install+record atomic (per-host async lock / queue).
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
robot
commented
[SHOULD-FIX] F2 [SHOULD-FIX] F2
Documentation/behavior mismatch. The PR body, commit 66c6218 message, and the code comments at :311 (:311 'via chrome.webRequest.onBeforeSendHeaders, which sees the request headers') describe an onBeforeSendHeaders + extraHeaders header-reading capture. The actual code uses onBeforeRequest (cannot read request headers) and derives the referer from the source TAB's URL instead (chrome.tabs.get). The `extraHeaders` permission is not declared in manifest.json (permissions: storage, tabs, scripting, webRequest, declarativeNetRequest). The commit message for :245-248 ('extraInfoSpec ... is rejected by some Chromium builds and fails the whole service worker') explains why, but the PR body / high-level comments were never updated to match, so a reviewer is misled about the mechanism.
**Suggested fix:** Reword the PR body and the :311/:308-314 comments to say the referer is derived from the source tab URL (via onBeforeRequest + chrome.tabs.get) rather than 'read from request headers via onBeforeSendHeaders'. The actual mechanism is sound and arguably safer than reading real headers; the mismatch is a documentation issue, not a correctness one.
|
|||||||
@@ -160,6 +160,17 @@ function m3u8DeepProbeFn() {
|
|||||||
|
|
||||||
function walk(obj, path, depth) {
|
function walk(obj, path, depth) {
|
||||||
if (depth > 5) return;
|
if (depth > 5) return;
|
||||||
|
// Cycle guard: the live window objects hold circular references
|
||||||
|
// (window/_parent/_top/frames/_globalObject/_globalProxy are mutually
|
||||||
|
// reachable), so a depth cap alone does NOT terminate — descending a cycle
|
||||||
|
// re-walks the same identities along exponentially many paths. Each object
|
||||||
|
// tree gets its own visited set so an identity is walked at most once per
|
||||||
|
// tree (trees are independent, so the set is declared per call).
|
||||||
|
const visited = new WeakSet();
|
||||||
|
if (obj && typeof obj === 'object') {
|
||||||
|
if (visited.has(obj)) return;
|
||||||
|
visited.add(obj);
|
||||||
|
}
|
||||||
if (typeof obj === 'string') {
|
if (typeof obj === 'string') {
|
||||||
const match = obj.match(/https?:\/\/[^"'`\s)]+?(?:\.m3u8|\.hlsvod|\/hls\/|\/playlist[^"'`\s]*|\/manifest[^"'`\s]*)[^"'`\s)]*/gi);
|
const match = obj.match(/https?:\/\/[^"'`\s)]+?(?:\.m3u8|\.hlsvod|\/hls\/|\/playlist[^"'`\s]*|\/manifest[^"'`\s]*)[^"'`\s)]*/gi);
|
||||||
if (match) {
|
if (match) {
|
||||||
|
|||||||
[SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
Suggested fix: Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.
[SHOULD-FIX] F3
A tool-local settings file is committed to the shared repo and now carries absolute local paths to the author's machine: 'Bash(node /Users/kyle/.claude/jobs/1462df8d/tmp/transform2.mjs)' and 'Bash(open -a "Microsoft Edge" ...)'. These are machine-specific and should not live in the shared codebase; .claude/settings.local.json is typically user-scoped and gitignored.
Suggested fix: Drop .claude/settings.local.json from the PR (it is unrelated to the refactor) and/or add it to .gitignore so per-machine tool settings never get committed.