diff --git a/clients/static-site/src/screenshot.js b/clients/static-site/src/screenshot.js index 0b233446..85551d64 100644 --- a/clients/static-site/src/screenshot.js +++ b/clients/static-site/src/screenshot.js @@ -16,15 +16,13 @@ try { } /** - * Generate screenshot name from page and viewport - * Format: "path/to/page@viewportName" + * Generate screenshot name from page path + * Viewport info goes in properties for grouping * @param {Object} page - Page object with path property - * @param {Object} viewport - Viewport object with name * @returns {string} Screenshot name */ -export function generateScreenshotName(page, viewport) { +export function generateScreenshotName(page) { let { path } = page; - let viewportName = viewport.name; // Remove leading slash for cleaner names let cleanPath = path.startsWith('/') ? path.slice(1) : path; @@ -34,7 +32,27 @@ export function generateScreenshotName(page, viewport) { cleanPath = 'index'; } - return `${cleanPath}@${viewportName}`; + // Replace slashes and backslashes with hyphens to create valid filename + cleanPath = cleanPath.replace(/[/\\]/g, '-'); + + // Replace double dots (path traversal sequences) with single dots + cleanPath = cleanPath.replace(/\.\./g, '.'); + + return cleanPath; +} + +/** + * Generate screenshot properties from viewport + * Properties are used by Vizzly for grouping and identification + * @param {Object} viewport - Viewport object with name, width, height + * @returns {Object} Screenshot properties + */ +export function generateScreenshotProperties(viewport) { + return { + viewport: viewport.name, + viewportWidth: viewport.width, + viewportHeight: viewport.height, + }; } /** @@ -70,8 +88,9 @@ export async function captureAndSendScreenshot( viewport, screenshotOptions = {} ) { - let name = generateScreenshotName(pageObj, viewport); + let name = generateScreenshotName(pageObj); + let properties = generateScreenshotProperties(viewport); let screenshot = await captureScreenshot(page, screenshotOptions); - await vizzlyScreenshot(name, screenshot); + await vizzlyScreenshot(name, screenshot, { properties }); } diff --git a/clients/static-site/tests/screenshot.spec.js b/clients/static-site/tests/screenshot.spec.js index f1de3bde..463ea562 100644 --- a/clients/static-site/tests/screenshot.spec.js +++ b/clients/static-site/tests/screenshot.spec.js @@ -3,62 +3,132 @@ */ import { describe, it, expect } from 'vitest'; -import { generateScreenshotName } from '../src/screenshot.js'; +import { + generateScreenshotName, + generateScreenshotProperties, +} from '../src/screenshot.js'; describe('generateScreenshotName', () => { - it('should generate correct screenshot name', () => { + it('should generate name from page path', () => { let page = { path: '/about' }; - let viewport = { name: 'mobile' }; - - let name = generateScreenshotName(page, viewport); - - expect(name).toBe('about@mobile'); + let name = generateScreenshotName(page); + expect(name).toBe('about'); }); it('should handle root path', () => { let page = { path: '/' }; - let viewport = { name: 'desktop' }; - - let name = generateScreenshotName(page, viewport); - - expect(name).toBe('index@desktop'); + let name = generateScreenshotName(page); + expect(name).toBe('index'); }); - it('should handle nested paths', () => { + it('should replace slashes with hyphens', () => { let page = { path: '/blog/post-1' }; - let viewport = { name: 'tablet' }; + let name = generateScreenshotName(page); + expect(name).toBe('blog-post-1'); + expect(name).not.toContain('/'); + }); - let name = generateScreenshotName(page, viewport); + it('should handle nested paths', () => { + let page = { path: '/configuration/billing' }; + let name = generateScreenshotName(page); + expect(name).toBe('configuration-billing'); + }); - expect(name).toBe('blog/post-1@tablet'); + it('should handle deeply nested paths', () => { + let page = { path: '/api/v1/users/settings' }; + let name = generateScreenshotName(page); + expect(name).toBe('api-v1-users-settings'); }); it('should handle paths without leading slash', () => { let page = { path: 'docs/guide' }; - let viewport = { name: 'mobile' }; - - let name = generateScreenshotName(page, viewport); - - expect(name).toBe('docs/guide@mobile'); + let name = generateScreenshotName(page); + expect(name).toBe('docs-guide'); }); it('should handle empty path', () => { let page = { path: '' }; - let viewport = { name: 'desktop' }; + let name = generateScreenshotName(page); + expect(name).toBe('index'); + }); - let name = generateScreenshotName(page, viewport); + it('should handle backslashes', () => { + let page = { path: 'foo\\bar' }; + let name = generateScreenshotName(page); + expect(name).toBe('foo-bar'); + expect(name).not.toContain('\\'); + }); + + it('should handle path traversal attempts', () => { + let testCases = [ + { path: '../../etc/passwd', expected: '.-.-etc-passwd' }, // .. becomes . + { path: '../../../sensitive', expected: '.-.-.-sensitive' }, // .. becomes ., extra / becomes - + { path: '/path/../secret', expected: 'path-.-secret' }, // .. becomes . + ]; + + for (let testCase of testCases) { + let page = { path: testCase.path }; + let name = generateScreenshotName(page); + expect(name).toBe(testCase.expected); + // Ensure no path traversal sequences remain + expect(name).not.toContain('..'); + // Ensure no unescaped slashes remain + expect(name).not.toContain('/'); + expect(name).not.toContain('\\'); + } + }); - expect(name).toBe('index@desktop'); + it('should handle triple dots', () => { + let page = { path: '/normal/.../path' }; + let name = generateScreenshotName(page); + // Triple dots contain .., which gets replaced: ... -> . + expect(name).toBe('normal-..-path'); + expect(name).not.toContain('...'); + }); + + it('should handle trailing slashes', () => { + let page = { path: '/about/' }; + let name = generateScreenshotName(page); + expect(name).toBe('about-'); + }); +}); + +describe('generateScreenshotProperties', () => { + it('should generate properties with viewport info', () => { + let viewport = { name: 'mobile', width: 375, height: 667 }; + let properties = generateScreenshotProperties(viewport); + + expect(properties).toEqual({ + viewport: 'mobile', + viewportWidth: 375, + viewportHeight: 667, + }); + }); + + it('should include viewport dimensions', () => { + let viewport1 = { name: 'mobile', width: 375, height: 667 }; + let viewport2 = { name: 'desktop', width: 1920, height: 1080 }; + let viewport3 = { name: 'tablet', width: 768, height: 1024 }; + + let props1 = generateScreenshotProperties(viewport1); + let props2 = generateScreenshotProperties(viewport2); + let props3 = generateScreenshotProperties(viewport3); + + expect(props1.viewportWidth).toBe(375); + expect(props1.viewportHeight).toBe(667); + + expect(props2.viewportWidth).toBe(1920); + expect(props2.viewportHeight).toBe(1080); + + expect(props3.viewportWidth).toBe(768); + expect(props3.viewportHeight).toBe(1024); }); it('should handle different viewport names', () => { - let page = { path: '/pricing' }; - let viewport1 = { name: 'mobile' }; - let viewport2 = { name: 'desktop' }; - let viewport3 = { name: 'tablet' }; - - expect(generateScreenshotName(page, viewport1)).toBe('pricing@mobile'); - expect(generateScreenshotName(page, viewport2)).toBe('pricing@desktop'); - expect(generateScreenshotName(page, viewport3)).toBe('pricing@tablet'); + let viewport1 = { name: 'mobile', width: 375, height: 667 }; + let viewport2 = { name: 'desktop', width: 1920, height: 1080 }; + + expect(generateScreenshotProperties(viewport1).viewport).toBe('mobile'); + expect(generateScreenshotProperties(viewport2).viewport).toBe('desktop'); }); });