From ee0b11452d8931bbfa0fdf337db977bf96a7cd2c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 13 Mar 2026 09:49:41 -0700 Subject: [PATCH] fix: cookie import picker returns JSON instead of HTML jsonResponse() was defined at module scope but referenced `url` which only existed as a parameter of handleCookiePickerRoute(). Every API call crashed, the catch block also crashed, and Bun returned a default HTML page that the frontend couldn't parse as JSON. Thread port via corsOrigin() helper and options objects. Add route-level tests to prevent this class of bug from shipping again. Co-Authored-By: Claude Opus 4.6 --- browse/src/cookie-picker-routes.ts | 49 +++--- browse/test/cookie-picker-routes.test.ts | 205 +++++++++++++++++++++++ 2 files changed, 233 insertions(+), 21 deletions(-) create mode 100644 browse/test/cookie-picker-routes.test.ts diff --git a/browse/src/cookie-picker-routes.ts b/browse/src/cookie-picker-routes.ts index e185af0c..6a4a4319 100644 --- a/browse/src/cookie-picker-routes.ts +++ b/browse/src/cookie-picker-routes.ts @@ -26,18 +26,25 @@ const importedCounts = new Map(); // ─── JSON Helpers ─────────────────────────────────────────────── -function jsonResponse(data: any, status = 200): Response { +function corsOrigin(port: number): string { + return `http://127.0.0.1:${port}`; +} + +function jsonResponse(data: any, opts: { port: number; status?: number }): Response { return new Response(JSON.stringify(data), { - status, + status: opts.status ?? 200, headers: { 'Content-Type': 'application/json', - 'Access-Control-Allow-Origin': `http://127.0.0.1:${parseInt(url.port, 10) || 9400}`, + 'Access-Control-Allow-Origin': corsOrigin(opts.port), }, }); } -function errorResponse(message: string, code: string, status = 400, action?: string): Response { - return jsonResponse({ error: message, code, ...(action ? { action } : {}) }, status); +function errorResponse(message: string, code: string, opts: { port: number; status?: number; action?: string }): Response { + return jsonResponse( + { error: message, code, ...(opts.action ? { action: opts.action } : {}) }, + { port: opts.port, status: opts.status ?? 400 }, + ); } // ─── Route Handler ────────────────────────────────────────────── @@ -48,13 +55,14 @@ export async function handleCookiePickerRoute( bm: BrowserManager, ): Promise { const pathname = url.pathname; + const port = parseInt(url.port, 10) || 9400; // CORS preflight if (req.method === 'OPTIONS') { return new Response(null, { status: 204, headers: { - 'Access-Control-Allow-Origin': `http://127.0.0.1:${parseInt(url.port, 10) || 9400}`, + 'Access-Control-Allow-Origin': corsOrigin(port), 'Access-Control-Allow-Methods': 'GET, POST, OPTIONS', 'Access-Control-Allow-Headers': 'Content-Type', }, @@ -64,7 +72,6 @@ export async function handleCookiePickerRoute( try { // GET /cookie-picker — serve the picker UI if (pathname === '/cookie-picker' && req.method === 'GET') { - const port = parseInt(url.port, 10) || 9400; const html = getCookiePickerHTML(port); return new Response(html, { status: 200, @@ -80,20 +87,20 @@ export async function handleCookiePickerRoute( name: b.name, aliases: b.aliases, })), - }); + }, { port }); } // GET /cookie-picker/domains?browser= — list domains + counts if (pathname === '/cookie-picker/domains' && req.method === 'GET') { const browserName = url.searchParams.get('browser'); if (!browserName) { - return errorResponse("Missing 'browser' parameter", 'missing_param'); + return errorResponse("Missing 'browser' parameter", 'missing_param', { port }); } const result = listDomains(browserName); return jsonResponse({ browser: result.browser, domains: result.domains, - }); + }, { port }); } // POST /cookie-picker/import — decrypt + import to Playwright session @@ -102,13 +109,13 @@ export async function handleCookiePickerRoute( try { body = await req.json(); } catch { - return errorResponse('Invalid JSON body', 'bad_request'); + return errorResponse('Invalid JSON body', 'bad_request', { port }); } const { browser, domains } = body; - if (!browser) return errorResponse("Missing 'browser' field", 'missing_param'); + if (!browser) return errorResponse("Missing 'browser' field", 'missing_param', { port }); if (!domains || !Array.isArray(domains) || domains.length === 0) { - return errorResponse("Missing or empty 'domains' array", 'missing_param'); + return errorResponse("Missing or empty 'domains' array", 'missing_param', { port }); } // Decrypt cookies from the browser DB @@ -122,7 +129,7 @@ export async function handleCookiePickerRoute( message: result.failed > 0 ? `All ${result.failed} cookies failed to decrypt` : 'No cookies found for the specified domains', - }); + }, { port }); } // Add to Playwright context @@ -141,7 +148,7 @@ export async function handleCookiePickerRoute( imported: result.count, failed: result.failed, domainCounts: result.domainCounts, - }); + }, { port }); } // POST /cookie-picker/remove — clear cookies for domains @@ -150,12 +157,12 @@ export async function handleCookiePickerRoute( try { body = await req.json(); } catch { - return errorResponse('Invalid JSON body', 'bad_request'); + return errorResponse('Invalid JSON body', 'bad_request', { port }); } const { domains } = body; if (!domains || !Array.isArray(domains) || domains.length === 0) { - return errorResponse("Missing or empty 'domains' array", 'missing_param'); + return errorResponse("Missing or empty 'domains' array", 'missing_param', { port }); } const page = bm.getPage(); @@ -171,7 +178,7 @@ export async function handleCookiePickerRoute( return jsonResponse({ removed: domains.length, domains, - }); + }, { port }); } // GET /cookie-picker/imported — currently imported domains + counts @@ -186,15 +193,15 @@ export async function handleCookiePickerRoute( domains: entries, totalDomains: entries.length, totalCookies: entries.reduce((sum, e) => sum + e.count, 0), - }); + }, { port }); } return new Response('Not found', { status: 404 }); } catch (err: any) { if (err instanceof CookieImportError) { - return errorResponse(err.message, err.code, 400, err.action); + return errorResponse(err.message, err.code, { port, status: 400, action: err.action }); } console.error(`[cookie-picker] Error: ${err.message}`); - return errorResponse(err.message || 'Internal error', 'internal_error', 500); + return errorResponse(err.message || 'Internal error', 'internal_error', { port, status: 500 }); } } diff --git a/browse/test/cookie-picker-routes.test.ts b/browse/test/cookie-picker-routes.test.ts new file mode 100644 index 00000000..ca55c473 --- /dev/null +++ b/browse/test/cookie-picker-routes.test.ts @@ -0,0 +1,205 @@ +/** + * Tests for cookie-picker route handler + * + * Tests the HTTP glue layer directly with mock BrowserManager objects. + * Verifies that all routes return valid JSON (not HTML) with correct CORS headers. + */ + +import { describe, test, expect } from 'bun:test'; +import { handleCookiePickerRoute } from '../src/cookie-picker-routes'; + +// ─── Mock BrowserManager ────────────────────────────────────── + +function mockBrowserManager() { + const addedCookies: any[] = []; + const clearedDomains: string[] = []; + return { + bm: { + getPage: () => ({ + context: () => ({ + addCookies: (cookies: any[]) => { addedCookies.push(...cookies); }, + clearCookies: (opts: { domain: string }) => { clearedDomains.push(opts.domain); }, + }), + }), + } as any, + addedCookies, + clearedDomains, + }; +} + +function makeUrl(path: string, port = 9470): URL { + return new URL(`http://127.0.0.1:${port}${path}`); +} + +function makeReq(method: string, body?: any): Request { + const opts: RequestInit = { method }; + if (body) { + opts.body = JSON.stringify(body); + opts.headers = { 'Content-Type': 'application/json' }; + } + return new Request('http://127.0.0.1:9470', opts); +} + +// ─── Tests ────────────────────────────────────────────────────── + +describe('cookie-picker-routes', () => { + describe('CORS', () => { + test('OPTIONS returns 204 with correct CORS headers', async () => { + const { bm } = mockBrowserManager(); + const url = makeUrl('/cookie-picker/browsers'); + const req = new Request('http://127.0.0.1:9470', { method: 'OPTIONS' }); + + const res = await handleCookiePickerRoute(url, req, bm); + + expect(res.status).toBe(204); + expect(res.headers.get('Access-Control-Allow-Origin')).toBe('http://127.0.0.1:9470'); + expect(res.headers.get('Access-Control-Allow-Methods')).toContain('POST'); + }); + + test('JSON responses include correct CORS origin with port', async () => { + const { bm } = mockBrowserManager(); + const url = makeUrl('/cookie-picker/browsers', 9450); + const req = new Request('http://127.0.0.1:9450', { method: 'GET' }); + + const res = await handleCookiePickerRoute(url, req, bm); + + expect(res.headers.get('Access-Control-Allow-Origin')).toBe('http://127.0.0.1:9450'); + }); + }); + + describe('JSON responses (not HTML)', () => { + test('GET /cookie-picker/browsers returns JSON', async () => { + const { bm } = mockBrowserManager(); + const url = makeUrl('/cookie-picker/browsers'); + const req = new Request('http://127.0.0.1:9470', { method: 'GET' }); + + const res = await handleCookiePickerRoute(url, req, bm); + + expect(res.status).toBe(200); + expect(res.headers.get('Content-Type')).toBe('application/json'); + const body = await res.json(); + expect(body).toHaveProperty('browsers'); + expect(Array.isArray(body.browsers)).toBe(true); + }); + + test('GET /cookie-picker/domains without browser param returns JSON error', async () => { + const { bm } = mockBrowserManager(); + const url = makeUrl('/cookie-picker/domains'); + const req = new Request('http://127.0.0.1:9470', { method: 'GET' }); + + const res = await handleCookiePickerRoute(url, req, bm); + + expect(res.status).toBe(400); + expect(res.headers.get('Content-Type')).toBe('application/json'); + const body = await res.json(); + expect(body).toHaveProperty('error'); + expect(body).toHaveProperty('code', 'missing_param'); + }); + + test('POST /cookie-picker/import with invalid JSON returns JSON error', async () => { + const { bm } = mockBrowserManager(); + const url = makeUrl('/cookie-picker/import'); + const req = new Request('http://127.0.0.1:9470', { + method: 'POST', + body: 'not json', + headers: { 'Content-Type': 'application/json' }, + }); + + const res = await handleCookiePickerRoute(url, req, bm); + + expect(res.status).toBe(400); + expect(res.headers.get('Content-Type')).toBe('application/json'); + const body = await res.json(); + expect(body.code).toBe('bad_request'); + }); + + test('POST /cookie-picker/import missing browser field returns JSON error', async () => { + const { bm } = mockBrowserManager(); + const url = makeUrl('/cookie-picker/import'); + const req = makeReq('POST', { domains: ['.example.com'] }); + + const res = await handleCookiePickerRoute(url, req, bm); + + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.code).toBe('missing_param'); + }); + + test('POST /cookie-picker/import missing domains returns JSON error', async () => { + const { bm } = mockBrowserManager(); + const url = makeUrl('/cookie-picker/import'); + const req = makeReq('POST', { browser: 'Chrome' }); + + const res = await handleCookiePickerRoute(url, req, bm); + + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.code).toBe('missing_param'); + }); + + test('POST /cookie-picker/remove with invalid JSON returns JSON error', async () => { + const { bm } = mockBrowserManager(); + const url = makeUrl('/cookie-picker/remove'); + const req = new Request('http://127.0.0.1:9470', { + method: 'POST', + body: '{bad', + headers: { 'Content-Type': 'application/json' }, + }); + + const res = await handleCookiePickerRoute(url, req, bm); + + expect(res.status).toBe(400); + expect(res.headers.get('Content-Type')).toBe('application/json'); + }); + + test('POST /cookie-picker/remove missing domains returns JSON error', async () => { + const { bm } = mockBrowserManager(); + const url = makeUrl('/cookie-picker/remove'); + const req = makeReq('POST', {}); + + const res = await handleCookiePickerRoute(url, req, bm); + + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.code).toBe('missing_param'); + }); + + test('GET /cookie-picker/imported returns JSON with domain list', async () => { + const { bm } = mockBrowserManager(); + const url = makeUrl('/cookie-picker/imported'); + const req = new Request('http://127.0.0.1:9470', { method: 'GET' }); + + const res = await handleCookiePickerRoute(url, req, bm); + + expect(res.status).toBe(200); + expect(res.headers.get('Content-Type')).toBe('application/json'); + const body = await res.json(); + expect(body).toHaveProperty('domains'); + expect(body).toHaveProperty('totalDomains'); + expect(body).toHaveProperty('totalCookies'); + }); + }); + + describe('routing', () => { + test('GET /cookie-picker returns HTML', async () => { + const { bm } = mockBrowserManager(); + const url = makeUrl('/cookie-picker'); + const req = new Request('http://127.0.0.1:9470', { method: 'GET' }); + + const res = await handleCookiePickerRoute(url, req, bm); + + expect(res.status).toBe(200); + expect(res.headers.get('Content-Type')).toContain('text/html'); + }); + + test('unknown path returns 404', async () => { + const { bm } = mockBrowserManager(); + const url = makeUrl('/cookie-picker/nonexistent'); + const req = new Request('http://127.0.0.1:9470', { method: 'GET' }); + + const res = await handleCookiePickerRoute(url, req, bm); + + expect(res.status).toBe(404); + }); + }); +});