diff --git a/src/client/index.js b/src/client/index.js index 94c8f799..1ad4ba17 100644 --- a/src/client/index.js +++ b/src/client/index.js @@ -195,10 +195,9 @@ function createSimpleClient(serverUrl) { try { // If it's a string, assume it's a file path and send directly // Otherwise it's a Buffer, so convert to base64 - const image = - typeof imageBuffer === 'string' - ? imageBuffer - : imageBuffer.toString('base64'); + let isFilePath = typeof imageBuffer === 'string'; + let image = isFilePath ? imageBuffer : imageBuffer.toString('base64'); + let type = isFilePath ? 'file-path' : 'base64'; const { status, json } = await httpPost( `${serverUrl}/screenshot`, @@ -206,6 +205,7 @@ function createSimpleClient(serverUrl) { buildId: getBuildId(), name, image, + type, properties: options, fullPage: options.fullPage || false, }, diff --git a/src/server/handlers/api-handler.js b/src/server/handlers/api-handler.js index 13b468e1..73c724e6 100644 --- a/src/server/handlers/api-handler.js +++ b/src/server/handlers/api-handler.js @@ -49,7 +49,13 @@ export const createApiHandler = ( let screenshotCount = 0; let uploadPromises = []; - const handleScreenshot = async (buildId, name, image, properties = {}) => { + const handleScreenshot = async ( + buildId, + name, + image, + properties = {}, + type + ) => { if (vizzlyDisabled) { output.debug('upload', `${name} (disabled)`); return { @@ -75,8 +81,12 @@ export const createApiHandler = ( } // Support both base64 encoded images and file paths + // Use explicit type from client if provided (fast path), otherwise detect (slow path) + // Only accept valid type values to prevent invalid types from bypassing detection let imageBuffer; - const inputType = detectImageInputType(image); + let validTypes = ['base64', 'file-path']; + const inputType = + type && validTypes.includes(type) ? type : detectImageInputType(image); if (inputType === 'file-path') { // It's a file path - resolve and read the file diff --git a/src/server/handlers/tdd-handler.js b/src/server/handlers/tdd-handler.js index 0d38be2f..099ae8d8 100644 --- a/src/server/handlers/tdd-handler.js +++ b/src/server/handlers/tdd-handler.js @@ -325,7 +325,13 @@ export const createTddHandler = ( } }; - const handleScreenshot = async (_buildId, name, image, properties = {}) => { + const handleScreenshot = async ( + _buildId, + name, + image, + properties = {}, + type + ) => { // Validate and sanitize screenshot name let sanitizedName; try { @@ -364,8 +370,12 @@ export const createTddHandler = ( // Support both base64 encoded images and file paths // Vitest browser mode returns file paths, so we need to handle both + // Use explicit type from client if provided (fast path), otherwise detect (slow path) + // Only accept valid type values to prevent invalid types from bypassing detection let imageBuffer; - const inputType = detectImageInputType(image); + let validTypes = ['base64', 'file-path']; + const inputType = + type && validTypes.includes(type) ? type : detectImageInputType(image); if (inputType === 'file-path') { // It's a file path - resolve and read the file diff --git a/src/server/routers/screenshot.js b/src/server/routers/screenshot.js index 804fb9ac..23c4954b 100644 --- a/src/server/routers/screenshot.js +++ b/src/server/routers/screenshot.js @@ -24,7 +24,7 @@ export function createScreenshotRouter({ screenshotHandler, defaultBuildId }) { if (pathname === '/screenshot') { try { const body = await parseJsonBody(req); - const { buildId, name, properties, image } = body; + const { buildId, name, properties, image, type } = body; if (!name || !image) { sendError(res, 400, 'name and image are required'); @@ -38,7 +38,8 @@ export function createScreenshotRouter({ screenshotHandler, defaultBuildId }) { effectiveBuildId, name, image, - properties + properties, + type ); sendJson(res, result.statusCode, result.body); diff --git a/src/utils/image-input-detector.js b/src/utils/image-input-detector.js index b5f893e6..e9ebc04f 100644 --- a/src/utils/image-input-detector.js +++ b/src/utils/image-input-detector.js @@ -82,42 +82,46 @@ export function looksLikeFilePath(str) { return false; } - // 0. Explicitly reject data URIs first (they contain : and / which would match path patterns) + // 0. Length check - file paths are short, base64 screenshots are huge + // Even the longest realistic file path is < 500 chars + // This makes detection O(1) for large base64 strings + // Use same threshold (1000) as detectImageInputType for consistency + if (str.length > 1000) { + return false; + } + + // 1. Explicitly reject data URIs (they contain : and / which would match path patterns) if (str.startsWith('data:')) { return false; } - // 1. Check for file:// URI scheme + // 2. Check for file:// URI scheme if (str.startsWith('file://')) { return true; } - // 2. Check for absolute paths (Unix or Windows) - // Unix: starts with / - // Windows: starts with drive letter like C:\ or C:/ - if (str.startsWith('/') || /^[A-Za-z]:[/\\]/.test(str)) { + // 3. Windows absolute paths (C:\ or C:/) - base64 never starts with drive letter + if (/^[A-Za-z]:[/\\]/.test(str)) { return true; } - // 3. Check for relative path indicators - // ./ or ../ or .\ or ..\ + // 4. Relative path indicators (./ or ../) - base64 never starts with dot if (/^\.\.?[/\\]/.test(str)) { return true; } - // 4. Check for path separators (forward or back slash) - // This catches paths like: subdirectory/file.png or subdirectory\file.png - if (/[/\\]/.test(str)) { - return true; - } - // 5. Check for common image file extensions - // This catches simple filenames like: screenshot.png - // Common extensions: png, jpg, jpeg, gif, webp, bmp, svg, tiff, ico + // This is the safest check - base64 never ends with .png/.jpg/etc + // Catches: /path/file.png, subdir/file.png, file.png if (/\.(png|jpe?g|gif|webp|bmp|svg|tiff?|ico)$/i.test(str)) { return true; } + // Note: We intentionally don't check for bare "/" prefix or "/" anywhere + // because JPEG base64 starts with "/9j/" which would false-positive + // File paths without extensions are rare for images and will fall through + // to base64 detection, which is acceptable for backwards compat + return false; } @@ -129,14 +133,13 @@ export function looksLikeFilePath(str) { * - 'file-path': A file path (relative or absolute) * - 'unknown': Cannot determine (ambiguous or invalid) * - * Strategy: - * 1. First check if it's valid base64 (can contain / which might look like paths) - * 2. Then check if it looks like a file path (more specific patterns) - * 3. Otherwise return 'unknown' + * Strategy (optimized for performance): + * 1. Check for data URI prefix first (O(1), definitive) + * 2. Check file path patterns (O(1) prefix/suffix checks) + * 3. For large non-path strings, assume base64 (skip expensive validation) + * 4. Only run full base64 validation on small ambiguous strings * - * This order prevents base64 strings (which can contain /) from being - * misidentified as file paths. Base64 validation is stricter and should - * be checked first. + * This avoids O(n) regex validation on large screenshot buffers. * * @param {string} str - String to detect * @returns {'base64' | 'file-path' | 'unknown'} Detected input type @@ -153,16 +156,27 @@ export function detectImageInputType(str) { return 'unknown'; } - // Check base64 FIRST - base64 strings can contain / which looks like paths - // Base64 validation is stricter and more deterministic - if (isBase64(str)) { + // 1. Data URIs are definitively base64 (O(1) check) + if (str.startsWith('data:')) { return 'base64'; } - // Then check file path - catch patterns that aren't valid base64 + // 2. Check file path patterns (O(1) prefix/suffix checks) if (looksLikeFilePath(str)) { return 'file-path'; } + // 3. For large strings that aren't file paths, assume base64 + // Screenshots are typically 100KB+ as base64, file paths are <1KB + // Skip expensive O(n) validation for large strings + if (str.length > 1000) { + return 'base64'; + } + + // 4. Full validation only for small ambiguous strings + if (isBase64(str)) { + return 'base64'; + } + return 'unknown'; } diff --git a/tests/server/handlers/api-handler.test.js b/tests/server/handlers/api-handler.test.js index 11b4b8c3..1c77e05e 100644 --- a/tests/server/handlers/api-handler.test.js +++ b/tests/server/handlers/api-handler.test.js @@ -166,6 +166,98 @@ describe('server/handlers/api-handler', () => { assert.ok(result.body.error.includes('Invalid image input')); }); + it('uses explicit base64 type parameter', async () => { + let mockClient = { request: async () => ({}) }; + let handler = createApiHandler(mockClient, { + uploadScreenshot: async () => ({ success: true }), + }); + + let pngHeader = Buffer.from([ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, + ]); + let base64Image = pngHeader.toString('base64'); + + // Pass explicit type='base64' + let result = await handler.handleScreenshot( + 'build-123', + 'test-with-type', + base64Image, + {}, + 'base64' + ); + + assert.strictEqual(result.statusCode, 200); + assert.strictEqual(result.body.success, true); + }); + + it('uses explicit file-path type parameter', async () => { + let _uploadedData = null; + let mockUploadScreenshot = async ( + _client, + buildId, + name, + buffer, + props + ) => { + _uploadedData = { buildId, name, buffer, props }; + return { success: true }; + }; + + let mockClient = { request: async () => ({}) }; + let handler = createApiHandler(mockClient, { + uploadScreenshot: mockUploadScreenshot, + }); + + // Create test image file + let { mkdtempSync, writeFileSync, rmSync } = await import('node:fs'); + let { tmpdir } = await import('node:os'); + let { join } = await import('node:path'); + let testDir = mkdtempSync(join(tmpdir(), 'api-handler-test-')); + + let imagePath = join(testDir, 'test.png'); + let imageData = Buffer.from([0x89, 0x50, 0x4e, 0x47]); + writeFileSync(imagePath, imageData); + + // Pass explicit type='file-path' + let result = await handler.handleScreenshot( + 'build-123', + 'file-screenshot', + `file://${imagePath}`, + {}, + 'file-path' + ); + + assert.strictEqual(result.statusCode, 200); + assert.strictEqual(result.body.success, true); + + await handler.flush(); + rmSync(testDir, { recursive: true }); + }); + + it('falls back to detection when type not provided', async () => { + let mockClient = { request: async () => ({}) }; + let handler = createApiHandler(mockClient, { + uploadScreenshot: async () => ({ success: true }), + }); + + let pngHeader = Buffer.from([ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, + ]); + let base64Image = pngHeader.toString('base64'); + + // No type parameter - relies on detection + let result = await handler.handleScreenshot( + 'build-123', + 'test-no-type', + base64Image, + {} + // No type parameter + ); + + assert.strictEqual(result.statusCode, 200); + assert.strictEqual(result.body.success, true); + }); + it('increments screenshot count', async () => { let mockClient = { request: async () => ({}) }; let handler = createApiHandler(mockClient, { diff --git a/tests/server/handlers/tdd-handler.test.js b/tests/server/handlers/tdd-handler.test.js index 3898c39a..b348bb32 100644 --- a/tests/server/handlers/tdd-handler.test.js +++ b/tests/server/handlers/tdd-handler.test.js @@ -736,6 +736,139 @@ describe('server/handlers/tdd-handler', () => { assert.ok(result.body.error.includes('not found')); }); + it('uses explicit type parameter instead of detection', async () => { + let detectCalled = false; + let deps = createMockDeps({ + detectImageInputType: () => { + detectCalled = true; + return 'base64'; + }, + tddServiceOverrides: { + compareScreenshot: name => ({ + id: `comp-${name}`, + name, + status: 'passed', + baseline: '/baselines/test.png', + current: '/current/test.png', + diff: null, + }), + }, + }); + let handler = createTddHandler({}, '/test', null, null, false, deps); + + // Pass explicit type='base64' - detection should not be called + let result = await handler.handleScreenshot( + 'build-1', + 'test', + 'base64imagedata', + {}, + 'base64' + ); + + assert.strictEqual(result.statusCode, 200); + assert.strictEqual(detectCalled, false); + }); + + it('uses explicit file-path type parameter', async () => { + let detectCalled = false; + let deps = createMockDeps({ + detectImageInputType: () => { + detectCalled = true; + return 'base64'; // Would return wrong type if called + }, + existsSync: () => true, + readFileSync: () => Buffer.from('fake-png-data'), + tddServiceOverrides: { + compareScreenshot: name => ({ + id: `comp-${name}`, + name, + status: 'passed', + baseline: '/baselines/test.png', + current: '/current/test.png', + diff: null, + }), + }, + }); + let handler = createTddHandler({}, '/test', null, null, false, deps); + + // Pass explicit type='file-path' - detection should not be called + let result = await handler.handleScreenshot( + 'build-1', + 'test', + 'file:///path/to/screenshot.png', + {}, + 'file-path' + ); + + assert.strictEqual(result.statusCode, 200); + assert.strictEqual(detectCalled, false); + }); + + it('falls back to detection when type not provided', async () => { + let detectCalled = false; + let deps = createMockDeps({ + detectImageInputType: () => { + detectCalled = true; + return 'base64'; + }, + tddServiceOverrides: { + compareScreenshot: name => ({ + id: `comp-${name}`, + name, + status: 'passed', + baseline: '/baselines/test.png', + current: '/current/test.png', + diff: null, + }), + }, + }); + let handler = createTddHandler({}, '/test', null, null, false, deps); + + // No type parameter - should call detection + let result = await handler.handleScreenshot( + 'build-1', + 'test', + 'base64imagedata', + {} + ); + + assert.strictEqual(result.statusCode, 200); + assert.strictEqual(detectCalled, true); + }); + + it('falls back to detection for invalid type values', async () => { + let detectCalled = false; + let deps = createMockDeps({ + detectImageInputType: () => { + detectCalled = true; + return 'base64'; + }, + tddServiceOverrides: { + compareScreenshot: name => ({ + id: `comp-${name}`, + name, + status: 'passed', + baseline: '/baselines/test.png', + current: '/current/test.png', + diff: null, + }), + }, + }); + let handler = createTddHandler({}, '/test', null, null, false, deps); + + // Invalid type value - should fall back to detection + let result = await handler.handleScreenshot( + 'build-1', + 'test', + 'base64imagedata', + {}, + 'invalid-type' + ); + + assert.strictEqual(result.statusCode, 200); + assert.strictEqual(detectCalled, true); + }); + it('returns 500 for error comparison', async () => { let deps = createMockDeps({ tddServiceOverrides: { diff --git a/tests/server/routers/screenshot.test.js b/tests/server/routers/screenshot.test.js index 0457ccaf..05876a09 100644 --- a/tests/server/routers/screenshot.test.js +++ b/tests/server/routers/screenshot.test.js @@ -190,6 +190,99 @@ describe('server/routers/screenshot', () => { assert.strictEqual(capturedBuildId, 'default-build'); }); + it('passes explicit type to handler when provided', async () => { + let capturedType; + let screenshotHandler = { + handleScreenshot: async ( + _buildId, + _name, + _image, + _properties, + type + ) => { + capturedType = type; + return { statusCode: 200, body: { success: true } }; + }, + }; + + let handler = createScreenshotRouter({ + screenshotHandler, + defaultBuildId: 'build-1', + }); + let req = createMockRequest('POST', { + name: 'test', + image: 'base64data', + type: 'base64', + }); + let res = createMockResponse(); + + await handler(req, res, '/screenshot'); + + assert.strictEqual(capturedType, 'base64'); + }); + + it('passes file-path type to handler', async () => { + let capturedType; + let screenshotHandler = { + handleScreenshot: async ( + _buildId, + _name, + _image, + _properties, + type + ) => { + capturedType = type; + return { statusCode: 200, body: { success: true } }; + }, + }; + + let handler = createScreenshotRouter({ + screenshotHandler, + defaultBuildId: 'build-1', + }); + let req = createMockRequest('POST', { + name: 'test', + image: '/path/to/file.png', + type: 'file-path', + }); + let res = createMockResponse(); + + await handler(req, res, '/screenshot'); + + assert.strictEqual(capturedType, 'file-path'); + }); + + it('passes undefined type when not provided (backwards compat)', async () => { + let capturedType = 'not-called'; + let screenshotHandler = { + handleScreenshot: async ( + _buildId, + _name, + _image, + _properties, + type + ) => { + capturedType = type; + return { statusCode: 200, body: { success: true } }; + }, + }; + + let handler = createScreenshotRouter({ + screenshotHandler, + defaultBuildId: 'build-1', + }); + let req = createMockRequest('POST', { + name: 'test', + image: 'base64data', + // No type field - simulating old client + }); + let res = createMockResponse(); + + await handler(req, res, '/screenshot'); + + assert.strictEqual(capturedType, undefined); + }); + it('returns 400 when name is missing', async () => { let handler = createScreenshotRouter({ screenshotHandler: createMockScreenshotHandler(), diff --git a/tests/utils/image-input-detector.test.js b/tests/utils/image-input-detector.test.js index a92ca869..2296060c 100644 --- a/tests/utils/image-input-detector.test.js +++ b/tests/utils/image-input-detector.test.js @@ -114,6 +114,18 @@ describe('utils/image-input-detector', () => { it('returns false for base64 strings', () => { assert.strictEqual(looksLikeFilePath('aGVsbG8='), false); }); + + it('returns false for large strings (length > 1000)', () => { + // Large strings are assumed to be base64, not file paths + let largeString = 'a'.repeat(1001); + assert.strictEqual(looksLikeFilePath(largeString), false); + }); + + it('returns false for JPEG base64 starting with /9j/', () => { + // JPEG base64 starts with /9j/ which could look like a path + // but should not be detected as file path + assert.strictEqual(looksLikeFilePath('/9j/4AAQSkZJRg=='), false); + }); }); describe('detectImageInputType', () => { @@ -151,5 +163,32 @@ describe('utils/image-input-detector', () => { it('returns unknown for ambiguous strings', () => { assert.strictEqual(detectImageInputType('invalid!!!'), 'unknown'); }); + + it('returns base64 for large strings (length > 1000) without file path patterns', () => { + // Large strings are assumed to be base64 without running expensive validation + let largeString = 'a'.repeat(1001); + assert.strictEqual(detectImageInputType(largeString), 'base64'); + }); + + it('returns base64 for JPEG base64 starting with /9j/', () => { + // JPEG base64 starts with /9j/ - should be detected as base64, not file-path + assert.strictEqual(detectImageInputType('/9j/4AAQSkZJRg=='), 'base64'); + }); + + it('returns file-path for absolute Unix path with extension', () => { + // Even though it starts with /, the .png extension makes it a file path + assert.strictEqual( + detectImageInputType('/path/to/image.png'), + 'file-path' + ); + }); + + it('handles data URI before checking file paths', () => { + // Data URIs should be detected as base64 immediately + assert.strictEqual( + detectImageInputType('data:image/png;base64,abc123'), + 'base64' + ); + }); }); });