diff --git a/source/constructs/lib/v8/functions/dit-header-normalization.js b/source/constructs/lib/v8/functions/dit-header-normalization.js index d553e37c6..7b9c8c5fb 100644 --- a/source/constructs/lib/v8/functions/dit-header-normalization.js +++ b/source/constructs/lib/v8/functions/dit-header-normalization.js @@ -1,8 +1,18 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 + + + +//const FORMAT_PRIORITY = ['webp', 'avif', 'jpeg', 'png', 'heif', 'tiff', 'raw', 'gif']; +// Priority: WebP first to avoid Chromium AVIF top-level-navigation download bug +// (Chrome/Edge fail to render AVIF on direct URL nav even with Content-Disposition: inline). +// Reverts the Issue #5 reorder. AVIF is still served when client explicitly accepts only AVIF. const FORMAT_PRIORITY = ['webp', 'avif', 'jpeg', 'png', 'heif', 'tiff', 'raw', 'gif']; + + + const FORMAT_MAPPING = { 'image/webp': 'webp', 'image/avif': 'avif', @@ -16,7 +26,39 @@ const FORMAT_MAPPING = { 'image/gif': 'gif' }; -function normalizeAcceptHeader(acceptHeader) { +const NON_ALPHA_FORMATS = ['jpeg']; + +// Accept-header wildcard tokens that mean "any image type" +const WILDCARD_TOKENS = ['*/*', 'image/*']; + +// User-Agent substrings that confirm WebP support. +// Verified support matrices: +// - Chrome 32+ (Jan 2014), Edge 18+, Firefox 65+, Opera 19+: WebP full support +// - Safari 14+ (macOS 11/iOS 14, Sep 2020): WebP full support +// - Mobile WebViews (Android Chrome, iOS WKWebView 14+): WebP full support +// References: https://caniuse.com/webp +const WEBP_CAPABLE_UA_PATTERNS = [ + /Chrome\/\d+/i, // Chrome, Edge (Chromium), Opera, modern Android browsers + /Firefox\/(6[5-9]|[7-9]\d|\d{3,})/i, // Firefox 65+ + /Version\/(1[4-9]|[2-9]\d|\d{3,})[\d.]*\s.*Safari\//i, // Safari 14+ desktop and iOS (Mobile token may appear between Version and Safari) + // Safari 14+ desktop and iOS (Mobile token may appear between Version and Safari) + // Safari 14+ (Version token, Safari trailing) + /CriOS\/\d+/i, // Chrome on iOS + /FxiOS\/\d+/i, // Firefox on iOS (uses iOS WebView, WebP support tied to OS) + /EdgiOS\/\d+/i // Edge on iOS +]; + +function isWebpCapableUserAgent(ua) { + if (!ua) return false; + for (var i = 0; i < WEBP_CAPABLE_UA_PATTERNS.length; i++) { + if (WEBP_CAPABLE_UA_PATTERNS[i].test(ua)) { + return true; + } + } + return false; +} + +function normalizeAcceptHeader(acceptHeader, userAgent, excludeFormats) { if (!acceptHeader) return null; const mimeTypes = acceptHeader @@ -24,21 +66,41 @@ function normalizeAcceptHeader(acceptHeader) { .map(part => part.split(';')[0].trim().toLowerCase()); var supportedFormats = []; + var hasWildcard = false; for (var i = 0; i < mimeTypes.length; i++) { + if (WILDCARD_TOKENS.indexOf(mimeTypes[i]) !== -1) { + hasWildcard = true; + continue; + } if (FORMAT_MAPPING[mimeTypes[i]]) { supportedFormats.push(FORMAT_MAPPING[mimeTypes[i]]); } } + // Issue #2: drop alpha-incompatible formats (e.g. JPEG) when source has alpha + if (excludeFormats && excludeFormats.length > 0) { + supportedFormats = supportedFormats.filter(function (fmt) { + return excludeFormats.indexOf(fmt) === -1; + }); + } + + // Specific MIME types win over wildcard for (var j = 0; j < FORMAT_PRIORITY.length; j++) { if (supportedFormats.indexOf(FORMAT_PRIORITY[j]) !== -1) { return 'image/' + FORMAT_PRIORITY[j]; } } + // Wildcard fallback: only set dit-accept if UA confirms WebP support. + // Unknown UAs fall through to null -> no dit-accept -> source format served (status quo). + if (hasWildcard && isWebpCapableUserAgent(userAgent)) { + return 'image/webp'; + } + return null; } + async function handler(event) { var request = event.request; var headers = request.headers; @@ -81,6 +143,15 @@ async function handler(event) { request.headers[ditViewportWidthHeader] = { value: normalizedWidth.toString() }; } + // Issue #2: detect alpha-capable source via URL extension to prevent + // transparent PNG/GIF/WebP being converted to JPEG (which loses alpha). + var excludeFormats = []; + var uri = request.uri ? request.uri.toLowerCase() : ''; + if (uri.endsWith('.png') || uri.endsWith('.gif') || uri.endsWith('.webp')) { + excludeFormats = NON_ALPHA_FORMATS; + } + + // Map standard headers to DIT headers for cache key optimization if (headers["host"] && ditHostHeader) { request.headers[ditHostHeader] = { value: headers["host"]["value"] }; @@ -88,7 +159,8 @@ async function handler(event) { // Only set dit-accept if format parameter is not present in query string if (headers["accept"] && ditAcceptHeader && !(request.querystring && request.querystring.format)) { - const normalizedFormat = normalizeAcceptHeader(headers["accept"]["value"]); + const userAgent = headers["user-agent"] && headers["user-agent"]["value"]; + const normalizedFormat = normalizeAcceptHeader(headers["accept"]["value"], userAgent, excludeFormats); if (normalizedFormat) { request.headers[ditAcceptHeader] = { value: normalizedFormat }; } diff --git a/source/constructs/lib/v8/test/unit/dit-function.test.ts b/source/constructs/lib/v8/test/unit/dit-function.test.ts index 12797445f..bfaba2ccf 100644 --- a/source/constructs/lib/v8/test/unit/dit-function.test.ts +++ b/source/constructs/lib/v8/test/unit/dit-function.test.ts @@ -7,8 +7,11 @@ import * as path from "path"; // CloudFront Function types interface CloudFrontRequest { headers: Record; + uri?: string; + querystring?: Record; } + interface CloudFrontEvent { request: CloudFrontRequest; } @@ -162,6 +165,175 @@ describe("DIT CloudFront Function", () => { }); describe("Accept header normalization", () => { + test("should fallback to webp for */* with modern Chrome UA", async () => { + const event: CloudFrontEvent = { + request: { + headers: { + host: { value: "test.com" }, + accept: { value: "*/*" }, + "user-agent": { value: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36" } + }, + querystring: {} + } + }; + const result = await handler(event); + expect(result.headers["dit-accept"]?.value).toBe("image/webp"); + }); + + test("should fallback to webp for image/* with modern Safari UA", async () => { + const event: CloudFrontEvent = { + request: { + headers: { + host: { value: "test.com" }, + accept: { value: "image/*,*/*;q=0.8" }, + "user-agent": { value: "Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Mobile/15E148 Safari/604.1" } + }, + querystring: {} + } + }; + const result = await handler(event); + expect(result.headers["dit-accept"]?.value).toBe("image/webp"); + }); + + test("should NOT set dit-accept for */* with unknown/legacy UA", async () => { + const event: CloudFrontEvent = { + request: { + headers: { + host: { value: "test.com" }, + accept: { value: "*/*" }, + "user-agent": { value: "curl/7.79.1" } + }, + querystring: {} + } + }; + const result = await handler(event); + expect(result.headers["dit-accept"]).toBeUndefined(); + }); + + test("should NOT set dit-accept for */* when User-Agent is missing", async () => { + const event: CloudFrontEvent = { + request: { + headers: { + host: { value: "test.com" }, + accept: { value: "*/*" } + }, + querystring: {} + } + }; + const result = await handler(event); + expect(result.headers["dit-accept"]).toBeUndefined(); + }); + + test("specific MIME types still win over wildcard fallback", async () => { + const event: CloudFrontEvent = { + request: { + headers: { + host: { value: "test.com" }, + accept: { value: "image/avif,image/webp,*/*" }, + "user-agent": { value: "Mozilla/5.0 ... Chrome/120.0.0.0 ..." } + }, + querystring: {} + } + }; + const result = await handler(event); + expect(result.headers["dit-accept"]?.value).toBe("image/webp"); // WebP first per post-revert priority + }); + + test("should exclude JPEG when source URL is .png (Issue #2 — preserve alpha)", async () => { + const event: CloudFrontEvent = { + request: { + uri: "/cdn-test/media/foo.png", + headers: { + host: { value: "test.com" }, + accept: { value: "image/png, image/jpeg, image/gif, image/*" } + }, + querystring: {} + } + }; + const result = await handler(event); + expect(result.headers["dit-accept"]?.value).toBe("image/png"); + }); + + test("should exclude JPEG when source URL is .gif", async () => { + const event: CloudFrontEvent = { + request: { + uri: "/foo.gif", + headers: { + host: { value: "test.com" }, + accept: { value: "image/jpeg, image/gif" } + }, + querystring: {} + } + }; + const result = await handler(event); + expect(result.headers["dit-accept"]?.value).toBe("image/gif"); + }); + + test("should exclude JPEG when source URL is .webp", async () => { + const event: CloudFrontEvent = { + request: { + uri: "/foo.webp", + headers: { + host: { value: "test.com" }, + accept: { value: "image/jpeg, image/webp" } + }, + querystring: {} + } + }; + const result = await handler(event); + expect(result.headers["dit-accept"]?.value).toBe("image/webp"); + }); + + //test("should still pick AVIF over WebP when source is .png and both accepted (priority preserved)", async () => { + test("should pick WebP over AVIF when source is .png and both accepted (post-revert priority)", async () => { + const event: CloudFrontEvent = { + request: { + uri: "/foo.png", + headers: { + host: { value: "test.com" }, + accept: { value: "image/avif, image/webp, image/jpeg" } + }, + querystring: {} + } + }; + const result = await handler(event); + expect(result.headers["dit-accept"]?.value).toBe("image/webp"); + + }); + + test("should NOT exclude JPEG when source URL is .jpg (no alpha to lose)", async () => { + const event: CloudFrontEvent = { + request: { + uri: "/foo.jpg", + headers: { + host: { value: "test.com" }, + accept: { value: "image/jpeg, image/webp" } + }, + querystring: {} + } + }; + const result = await handler(event); + // webp wins via priority; jpeg not excluded because source is jpg + expect(result.headers["dit-accept"]?.value).toBe("image/webp"); + }); + + test("should handle uppercase PNG extension", async () => { + const event: CloudFrontEvent = { + request: { + uri: "/Foo.PNG", + headers: { + host: { value: "test.com" }, + accept: { value: "image/png, image/jpeg" } + }, + querystring: {} + } + }; + const result = await handler(event); + expect(result.headers["dit-accept"]?.value).toBe("image/png"); + }); + + + test("should select highest priority format from Accept header", async () => { const testCases = [ { input: "image/avif,image/webp,image/png", expected: "image/webp" }, diff --git a/source/container/src/routes/image.ts b/source/container/src/routes/image.ts index b5beaec8e..c3d7e12ae 100644 --- a/source/container/src/routes/image.ts +++ b/source/container/src/routes/image.ts @@ -64,8 +64,13 @@ router.get('*', async (req: Request, res: Response) => { res.set('Access-Control-Allow-Origin', CORS_ORIGIN); } res.type(imageRequest.response.contentType || 'image/jpeg'); + // Force inline rendering. Without this, browsers fall back to URL-extension + // heuristics; when the URL ends in .jpeg/.png but DIT serves AVIF/WebP + // (per Issue #5 priority), Chrome treats the mismatch as "download." + res.set('Content-Disposition', 'inline'); res.send(processedImage); + console.log(JSON.stringify({ requestId: imageRequest.requestId, component: 'ImageRouter', diff --git a/source/container/src/services/image-processing/image-processor.service.test.ts b/source/container/src/services/image-processing/image-processor.service.test.ts index fe0ddb81d..04e2aea58 100644 --- a/source/container/src/services/image-processing/image-processor.service.test.ts +++ b/source/container/src/services/image-processing/image-processor.service.test.ts @@ -10,6 +10,10 @@ import sharp from 'sharp'; let TEST_JPEG_BUFFER: Buffer; let TEST_GIF_BUFFER: Buffer; +let TEST_ANIMATED_WEBP_BUFFER: Buffer; +let TEST_STILL_WEBP_BUFFER: Buffer; +let TEST_APNG_BUFFER: Buffer; + beforeAll(async () => { // Generate valid test images using Sharp @@ -20,6 +24,35 @@ beforeAll(async () => { TEST_GIF_BUFFER = await sharp({ create: { width: 100, height: 100, channels: 3, background: { r: 0, g: 0, b: 255 } } }).gif().toBuffer(); + + TEST_ANIMATED_WEBP_BUFFER = await sharp([ + { create: { width: 50, height: 50, channels: 4, background: { r: 0, g: 255, b: 0, alpha: 1 } } } as any, + { create: { width: 50, height: 50, channels: 4, background: { r: 255, g: 0, b: 0, alpha: 1 } } } as any, + { create: { width: 50, height: 50, channels: 4, background: { r: 0, g: 0, b: 255, alpha: 1 } } } as any, + ], { join: { animated: true } } as any) + .webp({ loop: 0, delay: [100, 100, 100] }) + .toBuffer(); + + + + // Issue #7: still WebP for the "single-frame falls back to animated:false" guard + TEST_STILL_WEBP_BUFFER = await sharp({ + create: { width: 50, height: 50, channels: 4, background: { r: 0, g: 255, b: 0, alpha: 1 } } + }).webp().toBuffer(); + + + const PNG_SIG = Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); + const buildChunk = (type: string, data: Buffer) => { + const len = Buffer.alloc(4); + len.writeUInt32BE(data.length, 0); + return Buffer.concat([len, Buffer.from(type, 'ascii'), data, Buffer.alloc(4)]); + }; + TEST_APNG_BUFFER = Buffer.concat([ + PNG_SIG, + buildChunk('IHDR', Buffer.alloc(13)), + buildChunk('acTL', Buffer.alloc(8)), + buildChunk('IDAT', Buffer.alloc(16)), + ]); }); describe('ImageProcessorService', () => { @@ -69,6 +102,48 @@ describe('ImageProcessorService', () => { const result = await service.process(request); expect(result).toBe(mockBuffer); }); + + it('should pass ICO through unchanged regardless of transformations', async () => { + const icoBuffer = Buffer.from('00000100', 'hex'); // ICO magic bytes + jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({ + buffer: icoBuffer, + metadata: { size: icoBuffer.length, format: 'x-icon' } + }); + + const request: ImageProcessingRequest = { + requestId: 'test-ico-1', + timestamp: Date.now(), + origin: { url: 'https://example.com/favicon.ico' }, + sourceImageContentType: 'image/x-icon', + transformations: [{ type: 'format', value: 'webp', source: 'auto' }], + response: { headers: {} } + }; + + const result = await service.process(request); + expect(result).toBe(icoBuffer); + expect(request.response.contentType).toBe('image/x-icon'); + }); + + it('should pass image/vnd.microsoft.icon through unchanged', async () => { + const icoBuffer = Buffer.from('00000100', 'hex'); + jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({ + buffer: icoBuffer, + metadata: { size: icoBuffer.length } + }); + + const request: ImageProcessingRequest = { + requestId: 'test-ico-2', + timestamp: Date.now(), + origin: { url: 'https://example.com/favicon.ico' }, + sourceImageContentType: 'image/vnd.microsoft.icon', + transformations: [{ type: 'resize', value: { width: 100 }, source: 'url' }], + response: { headers: {} } + }; + + const result = await service.process(request); + expect(result).toBe(icoBuffer); + expect(request.response.contentType).toBe('image/vnd.microsoft.icon'); + }); }); describe('overlay size calculation', () => { @@ -322,5 +397,193 @@ describe('ImageProcessorService', () => { expect(result).toBeInstanceOf(Buffer); }); }); + describe('animated WebP handling (Issue #7)', () => { + it('should preserve animation when source is animated WebP and output is webp', async () => { + jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({ + buffer: TEST_ANIMATED_WEBP_BUFFER, + metadata: { size: TEST_ANIMATED_WEBP_BUFFER.length, format: 'webp' } + }); + + const request: ImageProcessingRequest = { + requestId: 'test-animated-webp', + timestamp: Date.now(), + origin: { url: 'https://example.com/anim.webp' }, + sourceImageContentType: 'image/webp', + transformations: [{ type: 'resize', value: { width: 25 }, source: 'url' }], + response: { headers: {} } + }; + + const result = await service.process(request); + expect(result).toBeInstanceOf(Buffer); + + // Verify output is still multi-page (animation preserved) + const outMeta = await sharp(result, { animated: true }).metadata(); + expect(outMeta.pages).toBeGreaterThan(1); + expect(request.response.contentType).toBe('image/webp'); + }); + + it('should re-instantiate with animated=false for single-frame WebP', async () => { + jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({ + buffer: TEST_STILL_WEBP_BUFFER, + metadata: { size: TEST_STILL_WEBP_BUFFER.length, format: 'webp' } + }); + + const request: ImageProcessingRequest = { + requestId: 'test-still-webp', + timestamp: Date.now(), + origin: { url: 'https://example.com/still.webp' }, + sourceImageContentType: 'image/webp', + transformations: [{ type: 'resize', value: { width: 25 }, source: 'url' }], + response: { headers: {} } + }; + + const result = await service.process(request); + expect(result).toBeInstanceOf(Buffer); + + // Output should be a valid still WebP (pages undefined or 1) + const outMeta = await sharp(result).metadata(); + expect(outMeta.pages === undefined || outMeta.pages <= 1).toBe(true); + }); + }); + describe('animated PNG (APNG) handling (Issue #8)', () => { + it('should pass APNG through unchanged when source is image/png and buffer is APNG', async () => { + jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({ + buffer: TEST_APNG_BUFFER, + metadata: { size: TEST_APNG_BUFFER.length, format: 'png' } + }); + + const request: ImageProcessingRequest = { + requestId: 'test-apng', + timestamp: Date.now(), + origin: { url: 'https://example.com/anim.png' }, + sourceImageContentType: 'image/png', + transformations: [{ type: 'format', value: 'webp', source: 'auto' }], + response: { headers: {} } + }; + + const result = await service.process(request); + + // Buffer returned untouched, content-type stays image/png + expect(result).toBe(TEST_APNG_BUFFER); + expect(request.response.contentType).toBe('image/png'); + }); + + it('should still process a still PNG normally (no APNG short-circuit)', async () => { + const stillPng = await sharp({ + create: { width: 20, height: 20, channels: 3, background: { r: 255, g: 255, b: 255 } } + }).png().toBuffer(); + + jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({ + buffer: stillPng, + metadata: { size: stillPng.length, format: 'png' } + }); + + const request: ImageProcessingRequest = { + requestId: 'test-still-png', + timestamp: Date.now(), + origin: { url: 'https://example.com/still.png' }, + sourceImageContentType: 'image/png', + transformations: [{ type: 'resize', value: { width: 10 }, source: 'url' }], + response: { headers: {} } + }; + + const result = await service.process(request); + + // Still PNG goes through Sharp (different buffer, resized) + expect(result).not.toBe(stillPng); + const meta = await sharp(result).metadata(); + expect(meta.width).toBe(10); + }); + }); + describe('SVG passthrough (Issue #11)', () => { + it('should pass SVG through unchanged with format conversion transformation', async () => { + const svgBuffer = Buffer.from( + '' + ); + jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({ + buffer: svgBuffer, + metadata: { size: svgBuffer.length, format: 'svg' } + }); + + const request: ImageProcessingRequest = { + requestId: 'test-svg-1', + timestamp: Date.now(), + origin: { url: 'https://example.com/icon.svg' }, + sourceImageContentType: 'image/svg+xml', + transformations: [{ type: 'format', value: 'webp', source: 'auto' }], + response: { headers: {} } + }; + + const result = await service.process(request); + expect(result).toBe(svgBuffer); + expect(request.response.contentType).toBe('image/svg+xml'); + }); + + it('should pass SVG through unchanged with resize transformation (URL params ignored)', async () => { + const svgBuffer = Buffer.from(''); + jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({ + buffer: svgBuffer, + metadata: { size: svgBuffer.length, format: 'svg' } + }); + + const request: ImageProcessingRequest = { + requestId: 'test-svg-2', + timestamp: Date.now(), + origin: { url: 'https://example.com/badge.svg' }, + sourceImageContentType: 'image/svg+xml', + transformations: [{ type: 'resize', value: { width: 100 }, source: 'url' }], + response: { headers: {} } + }; + + const result = await service.process(request); + expect(result).toBe(svgBuffer); + expect(request.response.contentType).toBe('image/svg+xml'); + }); + }); + describe('BMP passthrough (Issue #13)', () => { + it('should pass image/bmp through unchanged with resize transformation (URL params ignored)', async () => { + const bmpBuffer = Buffer.from([0x42, 0x4D, 0x00, 0x00]); // BMP magic 'BM' + filler + jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({ + buffer: bmpBuffer, + metadata: { size: bmpBuffer.length, format: 'bmp' } + }); + + const request: ImageProcessingRequest = { + requestId: 'test-bmp-1', + timestamp: Date.now(), + origin: { url: 'https://example.com/card.bmp' }, + sourceImageContentType: 'image/bmp', + transformations: [{ type: 'resize', value: { width: 282 }, source: 'url' }], + response: { headers: {} } + }; + + const result = await service.process(request); + expect(result).toBe(bmpBuffer); + expect(request.response.contentType).toBe('image/bmp'); + }); + + it('should pass image/x-ms-bmp through unchanged with format transformation', async () => { + const bmpBuffer = Buffer.from([0x42, 0x4D, 0x00, 0x00]); + jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({ + buffer: bmpBuffer, + metadata: { size: bmpBuffer.length, format: 'bmp' } + }); + + const request: ImageProcessingRequest = { + requestId: 'test-bmp-2', + timestamp: Date.now(), + origin: { url: 'https://example.com/card.bmp' }, + sourceImageContentType: 'image/x-ms-bmp', + transformations: [{ type: 'format', value: 'webp', source: 'auto' }], + response: { headers: {} } + }; + + const result = await service.process(request); + expect(result).toBe(bmpBuffer); + expect(request.response.contentType).toBe('image/x-ms-bmp'); + }); + }); + + }); \ No newline at end of file diff --git a/source/container/src/services/image-processing/image-processor.service.ts b/source/container/src/services/image-processing/image-processor.service.ts index 6afc21edb..60e9de31c 100644 --- a/source/container/src/services/image-processing/image-processor.service.ts +++ b/source/container/src/services/image-processing/image-processor.service.ts @@ -8,6 +8,18 @@ import { ImageProcessingError } from './types'; import { ErrorMapper } from './utils/error-mapping'; import { TransformationMapper } from './transformation-engine/transformation-mapper'; import { EditApplicator } from './transformation-engine/edit-applicator'; +import { isApng } from './utils/apng-detector'; + +const ICO_CONTENT_TYPES = new Set([ + 'image/x-icon', + 'image/vnd.microsoft.icon', + 'image/ico', +]); +const BMP_CONTENT_TYPES = new Set([ + 'image/bmp', + 'image/x-bmp', + 'image/x-ms-bmp', +]); export class ImageProcessorService { private static instance: ImageProcessorService; @@ -38,11 +50,17 @@ export class ImageProcessorService { ); imageRequest.timings.imageProcessing.originFetchMs = Date.now() - fetchStart; - if (!imageRequest.transformations?.length) { + const isIcoSource = ICO_CONTENT_TYPES.has(imageRequest.sourceImageContentType); + const isApngSource = imageRequest.sourceImageContentType === 'image/png' && isApng(imageBuffer); + const isSvgSource = imageRequest.sourceImageContentType === 'image/svg+xml'; + const isBmpSource = BMP_CONTENT_TYPES.has(imageRequest.sourceImageContentType); + + if (!imageRequest.transformations?.length || isIcoSource || isApngSource || isSvgSource || isBmpSource) { imageRequest.response.contentType = imageRequest.sourceImageContentType; imageRequest.timings.imageProcessing.transformationApplicationMs = 0; return imageBuffer; } + // Extract source dimensions to validate auto-resize transformations const metadata = await sharp(imageBuffer).metadata(); @@ -59,7 +77,9 @@ export class ImageProcessorService { editCount: Object.keys(imageEdits).length })); - const isExpectedToBeAnimated = imageRequest.sourceImageContentType == 'image/gif'; + const ANIMATED_INPUT_CONTENT_TYPES = new Set(['image/gif', 'image/webp']); + const isExpectedToBeAnimated = ANIMATED_INPUT_CONTENT_TYPES.has(imageRequest.sourceImageContentType); + let sharpOptions = { failOnError: true, animated: isExpectedToBeAnimated @@ -78,7 +98,18 @@ export class ImageProcessorService { // We need to resolve final image format from the outputted image. Obtaining this formating from image metadata prior to being outputted is unreliable. const finalImage = await image.toBuffer({resolveWithObject: true}); - imageRequest.response.contentType = 'image/' + finalImage.info.format; + // libvips reports AVIF as 'heif' because AVIF is an AV1-compressed HEIF container. + // Disambiguate by checking the requested format transformation; serve the correct + // MIME so browsers render rather than download. + let outputFormat = finalImage.info.format; + if (outputFormat === 'heif') { + const formatTransform = imageRequest.transformations?.find(t => t.type === 'format'); + if (formatTransform?.value === 'avif') { + outputFormat = 'avif'; + } + } + imageRequest.response.contentType = 'image/' + outputFormat; + const totalImageProcessingMs = Date.now() - startTime; imageRequest.timings.imageProcessing.transformationApplicationMs = diff --git a/source/container/src/services/image-processing/origin-fetcher.test.ts b/source/container/src/services/image-processing/origin-fetcher.test.ts index d2b2ce068..a5c4af493 100644 --- a/source/container/src/services/image-processing/origin-fetcher.test.ts +++ b/source/container/src/services/image-processing/origin-fetcher.test.ts @@ -27,6 +27,23 @@ describe('OriginFetcher', () => { it('should handle case insensitive content types', () => { expect(fetcher['isValidImageContentType']('IMAGE/JPEG')).toBe(true); }); + it('should accept ICO content types (Issue #6)', () => { + expect(fetcher['isValidImageContentType']('image/x-icon')).toBe(true); + expect(fetcher['isValidImageContentType']('image/vnd.microsoft.icon')).toBe(true); + expect(fetcher['isValidImageContentType']('image/ico')).toBe(true); + }); + it('should accept SVG content type (Issue #11)', () => { + expect(fetcher['isValidImageContentType']('image/svg+xml')).toBe(true); + expect(fetcher['isValidImageContentType']('image/svg+xml; charset=utf-8')).toBe(true); + }); + + it('should accept BMP content types (Issue #13)', () => { + expect(fetcher['isValidImageContentType']('image/bmp')).toBe(true); + expect(fetcher['isValidImageContentType']('image/x-bmp')).toBe(true); + expect(fetcher['isValidImageContentType']('image/x-ms-bmp')).toBe(true); + }); + + }); describe('error handling', () => { diff --git a/source/container/src/services/image-processing/origin-fetcher.ts b/source/container/src/services/image-processing/origin-fetcher.ts index 845afa5f5..e879631e4 100644 --- a/source/container/src/services/image-processing/origin-fetcher.ts +++ b/source/container/src/services/image-processing/origin-fetcher.ts @@ -155,10 +155,18 @@ export class OriginFetcher { 'image/tiff', 'image/avif', 'image/heif', + 'image/x-icon', + 'image/vnd.microsoft.icon', + 'image/ico', + 'image/svg+xml', + 'image/bmp', // Issue #13: BMP passthrough + 'image/x-bmp', // Issue #13: legacy variant + 'image/x-ms-bmp', // Issue #13: Microsoft variant ]; return validTypes.some(type => contentType.toLowerCase().includes(type)); } + private validateImageMagicNumbers(buffer: Buffer, contentType: string | undefined, url: string): void { // Where applicable the first 4 bytes are checked against that formats starting sequence. // For formats with inconsistent or non-existant starting sequences(av1, raw, etc) this validation is skipped. diff --git a/source/container/src/services/image-processing/utils/apng-detector.test.ts b/source/container/src/services/image-processing/utils/apng-detector.test.ts new file mode 100644 index 000000000..877d3f23a --- /dev/null +++ b/source/container/src/services/image-processing/utils/apng-detector.test.ts @@ -0,0 +1,63 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import sharp from 'sharp'; +import { isApng } from './apng-detector'; + +const PNG_SIG = Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); + +function makeChunk(type: string, data: Buffer): Buffer { + const length = Buffer.alloc(4); + length.writeUInt32BE(data.length, 0); + const typeBuf = Buffer.from(type, 'ascii'); + const crc = Buffer.alloc(4); // CRC content unused by detector + return Buffer.concat([length, typeBuf, data, crc]); +} + +describe('isApng', () => { + it('returns true when acTL chunk appears before IDAT', () => { + const ihdr = makeChunk('IHDR', Buffer.alloc(13)); + const actl = makeChunk('acTL', Buffer.alloc(8)); + const idat = makeChunk('IDAT', Buffer.alloc(16)); + const apng = Buffer.concat([PNG_SIG, ihdr, actl, idat]); + + expect(isApng(apng)).toBe(true); + }); + + it('returns false when IDAT appears before acTL (still PNG)', () => { + const ihdr = makeChunk('IHDR', Buffer.alloc(13)); + const idat = makeChunk('IDAT', Buffer.alloc(16)); + const stillPng = Buffer.concat([PNG_SIG, ihdr, idat]); + + expect(isApng(stillPng)).toBe(false); + }); + + it('returns false for a real Sharp-generated still PNG', async () => { + const realPng = await sharp({ + create: { width: 10, height: 10, channels: 4, background: { r: 0, g: 0, b: 0, alpha: 1 } } + }).png().toBuffer(); + + expect(isApng(realPng)).toBe(false); + }); + + it('returns false when buffer is not a PNG', () => { + expect(isApng(Buffer.from('not a png'))).toBe(false); + }); + + it('returns false when buffer is too small to contain signature', () => { + expect(isApng(Buffer.from([0x89, 0x50]))).toBe(false); + }); + + it('returns false when chunk length would overflow scan window', () => { + const ihdr = makeChunk('IHDR', Buffer.alloc(13)); + // Forge a chunk whose declared length exceeds the buffer + const malformed = Buffer.concat([ + PNG_SIG, + ihdr, + Buffer.from([0xFF, 0xFF, 0xFF, 0xFF]), // length = 4 GB + Buffer.from('XXXX', 'ascii'), + ]); + + expect(isApng(malformed)).toBe(false); + }); +}); diff --git a/source/container/src/services/image-processing/utils/apng-detector.ts b/source/container/src/services/image-processing/utils/apng-detector.ts new file mode 100644 index 000000000..71b3df0bb --- /dev/null +++ b/source/container/src/services/image-processing/utils/apng-detector.ts @@ -0,0 +1,36 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +const PNG_SIGNATURE = Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); +const MAX_SCAN_BYTES = 16384; + +/** + * Returns true if the buffer is an animated PNG (APNG). + * APNG is detected by the presence of an `acTL` chunk before the first `IDAT` chunk, + * per the W3C APNG specification (https://www.w3.org/TR/png-3/#11APNG). + * + * Sharp/libvips cannot decode APNG animation (issue lovell/sharp#2375), so callers + * must passthrough the original buffer to preserve animation. + */ +export function isApng(buffer: Buffer): boolean { + if (buffer.length < 8 || !buffer.subarray(0, 8).equals(PNG_SIGNATURE)) { + return false; + } + + const scanLimit = Math.min(buffer.length, MAX_SCAN_BYTES); + let offset = 8; + + while (offset + 12 <= scanLimit) { + const length = buffer.readUInt32BE(offset); + const type = buffer.toString('ascii', offset + 4, offset + 8); + + if (type === 'acTL') return true; + if (type === 'IDAT') return false; + + const nextOffset = offset + 12 + length; + if (nextOffset <= offset || nextOffset > scanLimit) return false; + offset = nextOffset; + } + + return false; +} diff --git a/source/container/src/services/request-resolver/connection-manager/connection-manager.ts b/source/container/src/services/request-resolver/connection-manager/connection-manager.ts index 011265645..5d978a89f 100644 --- a/source/container/src/services/request-resolver/connection-manager/connection-manager.ts +++ b/source/container/src/services/request-resolver/connection-manager/connection-manager.ts @@ -10,7 +10,7 @@ import { UrlValidator } from '../../../utils/url-validator'; export class ConnectionManager { private static readonly TIMEOUT_MS = 5000; - private readonly s3Client = new S3Client(getOptions()); + private readonly s3Client = new S3Client({ ...getOptions(), followRegionRedirects: true }); private validateContentType(contentType: string | undefined): void { if (!contentType?.split(';')[0].trim().startsWith('image/')) { diff --git a/source/container/src/services/transformation-resolver/auto-optimization/auto-optimizer.test.ts b/source/container/src/services/transformation-resolver/auto-optimization/auto-optimizer.test.ts index 513725e41..fad70165f 100644 --- a/source/container/src/services/transformation-resolver/auto-optimization/auto-optimizer.test.ts +++ b/source/container/src/services/transformation-resolver/auto-optimization/auto-optimizer.test.ts @@ -46,16 +46,95 @@ describe('applyAutoOptimizations', () => { }); }); - it('should prioritize formats by priority order (webp, avif, jpeg, png)', () => { + it('should prefer webp over avif when both are accepted (post-revert)', () => { mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; - mockRequest.headers = { 'dit-accept': 'image/jpeg,image/avif,image/avif,*/*' }; - + mockRequest.headers = { 'dit-accept': 'image/webp,image/avif,*/*' }; + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy); - + + expect(result).toHaveLength(1); + expect(result[0].value).toBe('webp'); + }); + + it('should still pick avif when only avif is accepted (no webp present)', () => { + mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; + mockRequest.headers = { 'dit-accept': 'image/avif,image/jpeg,*/*' }; + + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy); + expect(result).toHaveLength(1); expect(result[0].value).toBe('avif'); }); + it('should prefer avif over jpeg when both are accepted', () => { + mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; + mockRequest.headers = { 'dit-accept': 'image/jpeg,image/avif,*/*' }; + + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy); + + expect(result).toHaveLength(1); + expect(result[0].value).toBe('avif'); + }); + + it('should skip format optimization when source is ICO (image/x-icon)', () => { + mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; + mockRequest.headers = { 'dit-accept': 'image/webp,*/*' }; + const imageRequest = { sourceImageContentType: 'image/x-icon' } as ImageProcessingRequest; + + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy, imageRequest); + + expect(result).toHaveLength(0); + }); + + it('should skip format optimization when source is image/vnd.microsoft.icon', () => { + mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; + mockRequest.headers = { 'dit-accept': 'image/webp,*/*' }; + const imageRequest = { sourceImageContentType: 'image/vnd.microsoft.icon' } as ImageProcessingRequest; + + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy, imageRequest); + + expect(result).toHaveLength(0); + }); + + it('should skip format optimization when source is image/ico (legacy MIME)', () => { + mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; + mockRequest.headers = { 'dit-accept': 'image/webp,*/*' }; + const imageRequest = { sourceImageContentType: 'image/ico' } as ImageProcessingRequest; + + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy, imageRequest); + + expect(result).toHaveLength(0); + }); + + it('should skip format optimization when source is SVG (Issue #11)', () => { + mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; + mockRequest.headers = { 'dit-accept': 'image/webp,*/*' }; + const imageRequest = { sourceImageContentType: 'image/svg+xml' } as ImageProcessingRequest; + + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy, imageRequest); + + expect(result).toHaveLength(0); + }); + it('should skip format optimization when source is BMP (Issue #13)', () => { + mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; + mockRequest.headers = { 'dit-accept': 'image/webp,*/*' }; + const imageRequest = { sourceImageContentType: 'image/bmp' } as ImageProcessingRequest; + + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy, imageRequest); + + expect(result).toHaveLength(0); + }); + + it('should skip format optimization when source is image/x-ms-bmp', () => { + mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; + mockRequest.headers = { 'dit-accept': 'image/webp,*/*' }; + const imageRequest = { sourceImageContentType: 'image/x-ms-bmp' } as ImageProcessingRequest; + + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy, imageRequest); + + expect(result).toHaveLength(0); + }); + it('should apply static format when policy format is not auto', () => { mockPolicy.outputs = [{ type: 'format', value: 'jpeg' }]; mockRequest.headers = { 'dit-accept': 'image/webp,*/*' }; @@ -111,6 +190,51 @@ describe('applyAutoOptimizations', () => { expect(result[0]).toEqual({ type: 'format', value: 'avif', source: 'auto' }); }); + it('should skip format conversion when source is animated-capable WebP and selected format is JPEG (Issue #7)', () => { + mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; + mockRequest.headers = { 'dit-accept': 'image/jpeg' }; + const imageRequest = { sourceImageContentType: 'image/webp' } as ImageProcessingRequest; + + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy, imageRequest); + + // JPEG cannot carry animation; auto-optimizer must not convert WebP -> JPEG + expect(result).toHaveLength(0); + }); + + it('should skip format conversion when source is WebP and selected format is PNG (Issue #7)', () => { + mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; + mockRequest.headers = { 'dit-accept': 'image/png' }; + const imageRequest = { sourceImageContentType: 'image/webp' } as ImageProcessingRequest; + + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy, imageRequest); + + expect(result).toHaveLength(0); + }); + + it('should allow format conversion when source is WebP and selected format is AVIF (animation-capable)', () => { + mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; + mockRequest.headers = { 'dit-accept': 'image/avif' }; + const imageRequest = { sourceImageContentType: 'image/webp' } as ImageProcessingRequest; + + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy, imageRequest); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ type: 'format', value: 'avif', source: 'auto' }); + }); + + it('should return [] (no-op) when source WebP matches selected WebP', () => { + mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; + mockRequest.headers = { 'dit-accept': 'image/webp' }; + const imageRequest = { sourceImageContentType: 'image/webp' } as ImageProcessingRequest; + + const result = applyAutoOptimizations(baseTransformations, mockRequest as Request, mockPolicy, imageRequest); + + // Pre-existing same-format guard at auto-optimizer.ts:96-101 still wins + expect(result).toHaveLength(0); + }); + + + it('should not restrict format selection for non-GIF sources', () => { mockPolicy.outputs = [{ type: 'format', value: 'auto' }]; mockRequest.headers = { 'dit-accept': 'image/jpeg' }; diff --git a/source/container/src/services/transformation-resolver/auto-optimization/auto-optimizer.ts b/source/container/src/services/transformation-resolver/auto-optimization/auto-optimizer.ts index e12f7955d..cb5fea74d 100644 --- a/source/container/src/services/transformation-resolver/auto-optimization/auto-optimizer.ts +++ b/source/container/src/services/transformation-resolver/auto-optimization/auto-optimizer.ts @@ -5,7 +5,10 @@ import { Request } from 'express'; import { Transformation, TransformationPolicy } from '../../../types/transformation'; import { ImageProcessingRequest } from '../../../types/image-processing-request'; +// Priority: WebP first to avoid Chromium AVIF top-level-navigation download bug. +// AVIF is still served when client explicitly accepts only AVIF. const FORMAT_PRIORITY = ['webp', 'avif', 'jpeg', 'png', 'heif', 'tiff', 'raw', 'gif']; + // TODO, DISCUSS WITH TEAM FOR OPTIMAL FORMAT PRIORITIY LIST const ANIMATION_CAPABLE_FORMATS = new Set(['webp', 'avif', 'gif']); const FORMAT_MAPPING: Record = { @@ -73,13 +76,27 @@ function getFormatOptimizations(req: Request, formatConfig: any, imageRequest?: if (!selectedFormat) { return []; } - - // Skip format conversion if source is a GIF and selected format cannot carry animation - const sourceIsGif = imageRequest?.sourceImageContentType === 'image/gif'; - if (sourceIsGif && !ANIMATION_CAPABLE_FORMATS.has(selectedFormat)) { + + + const sourceContentType = imageRequest?.sourceImageContentType; + if (sourceContentType === 'image/x-icon' || + sourceContentType === 'image/vnd.microsoft.icon' || + sourceContentType === 'image/ico' || + sourceContentType === 'image/svg+xml' || + sourceContentType === 'image/bmp' || + sourceContentType === 'image/x-bmp' || + sourceContentType === 'image/x-ms-bmp') { return []; } + + // Skip format conversion if source may be animated (GIF/WebP) and selected format cannot carry animation + const sourceMayBeAnimated = sourceContentType === 'image/gif' || sourceContentType === 'image/webp'; + if (sourceMayBeAnimated && !ANIMATION_CAPABLE_FORMATS.has(selectedFormat)) { + return []; + } + + // Check if source image format matches selected format to avoid unnecessary transformation if (imageRequest?.sourceImageContentType) { const sourceFormat = FORMAT_MAPPING[imageRequest.sourceImageContentType];