fix(browse): a blocked navigation that fails at once leaves the tab blank and the next command clean

PR CI on 8d2d269 (free-suite 3): the redirect to [::ffff:169.254.170.2]
failed immediately on the runner, so its chrome-error page committed around
the guard's about:blank reset, and leaving the error page committed a
second about:blank after the reset resolved, interrupting the next three
tests' gotos. The guard now lets the blocked request settle (bounded at
1 s; a hanging target is still cut off by the reset), resets, and waits for
the main frame to stay quiet before returning. Regression: a redirect to a
blocked address on Chromium's unsafe port 1 fails at once (pre-fix: blank
check and the next goto fail, deterministically).
This commit is contained in:
garrytan committed 2026-10-04 05:02:06 +00:00
1 parent 8d2d269dc3
commit 65d94f2343
2 files changed
+25 -2

No files matched your search

+13 -2
View File
@@ -2119,8 +2119,19 @@ export class BrowserManager {
if (!reason || page.isClosed()) return; if (!reason || page.isClosed()) return;
guard.blocked = reason; guard.blocked = reason;
console.warn(`[browse] ${reason} (navigation from ${req.frame().url() || 'a new page'}; tab reset to about:blank)`); console.warn(`[browse] ${reason} (navigation from ${req.frame().url() || 'a new page'}; tab reset to about:blank)`);
// Commit is enough to blank the tab; a bounded reset never holds the command for the full navigation timeout. // A target that fails fast commits its error page around the reset, so
await page.goto('about:blank', { waitUntil: 'commit', timeout: 5_000 }).catch(() => {}); // let it settle (bounded; a hanging target is cut off by the reset),
// then blank the tab until it stays blank (about:blank loads at once).
await Promise.race([req.response().catch(() => null), new Promise((resolve) => setTimeout(resolve, 1_000))]);
// Leaving an error page can commit a second about:blank after the reset
// resolves; wait for the main frame to stay quiet so the next command's
// navigation is not interrupted by it.
for (let attempt = 0; attempt < 3 && !page.isClosed(); attempt++) {
await page.goto('about:blank', { waitUntil: 'load', timeout: 5_000 }).catch(() => {});
const late = await page.waitForEvent('framenavigated', { predicate: (frame) => frame === page.mainFrame(), timeout: 250 })
.then(() => true, () => false);
if (!late && page.url() === 'about:blank') break;
}
}).finally(() => { guard.pending.delete(check); }); }).finally(() => { guard.pending.delete(check); });
guard.pending.add(check); guard.pending.add(check);
}); });
+12
View File
@@ -13,6 +13,9 @@ let base: string;
const LINK_LOCAL = 'http://169.254.170.2/v2/credentials'; const LINK_LOCAL = 'http://169.254.170.2/v2/credentials';
const MAPPED = 'http://[::ffff:169.254.170.2]/v2/credentials'; const MAPPED = 'http://[::ffff:169.254.170.2]/v2/credentials';
// Port 1 is unsafe in Chromium, so this blocked target fails at once (ERR_UNSAFE_PORT)
// and commits its error page around the guard's reset, as CI's mapped-address hop did.
const FAST_FAIL = 'http://[::ffff:169.254.170.2]:1/v2/credentials';
beforeAll(async () => { beforeAll(async () => {
server = Bun.serve({ server = Bun.serve({
@@ -23,6 +26,7 @@ beforeAll(async () => {
const html = (body: string) => new Response(`<!doctype html><html><body>${body}</body></html>`, { headers: { 'Content-Type': 'text/html' } }); const html = (body: string) => new Response(`<!doctype html><html><body>${body}</body></html>`, { headers: { 'Content-Type': 'text/html' } });
if (url.pathname === '/to-link-local') return Response.redirect(LINK_LOCAL, 302); if (url.pathname === '/to-link-local') return Response.redirect(LINK_LOCAL, 302);
if (url.pathname === '/to-mapped') return Response.redirect(MAPPED, 302); if (url.pathname === '/to-mapped') return Response.redirect(MAPPED, 302);
if (url.pathname === '/to-fast-fail') return Response.redirect(FAST_FAIL, 302);
if (url.pathname === '/to-safe') return Response.redirect(`${base}/safe`, 302); if (url.pathname === '/to-safe') return Response.redirect(`${base}/safe`, 302);
if (url.pathname === '/link') return html(`<a id="go" href="${LINK_LOCAL}">go</a>`); if (url.pathname === '/link') return html(`<a id="go" href="${LINK_LOCAL}">go</a>`);
if (url.pathname === '/script') return html(`<button id="go" onclick="location.href='${LINK_LOCAL}'">go</button>`); if (url.pathname === '/script') return html(`<button id="go" onclick="location.href='${LINK_LOCAL}'">go</button>`);
@@ -55,6 +59,14 @@ describe('D2: navigation guard', () => {
expect(bm.getActiveSession().getPage().url()).toBe('about:blank'); expect(bm.getActiveSession().getPage().url()).toBe('about:blank');
}, 30_000); }, 30_000);
test('a blocked target that fails at once still leaves the tab blank for the next command', async () => {
await expect(run('goto', [`${base}/to-fast-fail`])).rejects.toThrow(/cloud metadata or link-local/);
expect(bm.getActiveSession().getPage().url()).toBe('about:blank');
const result = await run('goto', [`${base}/safe`]);
expect(result).toContain('Navigated to');
expect(bm.getActiveSession().getPage().url()).toBe(`${base}/safe`);
}, 30_000);
test('clicking a link to a link-local address is refused', async () => { test('clicking a link to a link-local address is refused', async () => {
await run('goto', [`${base}/link`]); await run('goto', [`${base}/link`]);
await expect(run('click', ['#go'])).rejects.toThrow(/cloud metadata or link-local/); await expect(run('click', ['#go'])).rejects.toThrow(/cloud metadata or link-local/);