diff --git a/source/image-handler/package-lock.json b/source/image-handler/package-lock.json index 7d71fed75..eb2208a26 100644 --- a/source/image-handler/package-lock.json +++ b/source/image-handler/package-lock.json @@ -946,6 +946,7 @@ "resolved": "https://registry.npmjs.org/@babel/core/-/core-7.28.5.tgz", "integrity": "sha512-e7jT4DxYvIDLk1ZHmU/m/mB19rex9sv0c2ftBtjSBv+kVM/902eh0fINUzD7UwLLNR+jU585GxUJ8/EBfAM5fw==", "dev": true, + "peer": true, "dependencies": { "@babel/code-frame": "^7.27.1", "@babel/generator": "^7.28.5", @@ -4697,6 +4698,7 @@ "resolved": "https://registry.npmjs.org/aws-sdk-client-mock/-/aws-sdk-client-mock-4.1.0.tgz", "integrity": "sha512-h/tOYTkXEsAcV3//6C1/7U4ifSpKyJvb6auveAepqqNJl6TdZaPFEtKjBQNf8UxQdDP850knB2i/whq4zlsxJw==", "dev": true, + "peer": true, "dependencies": { "@types/sinon": "^17.0.3", "sinon": "^18.0.1", @@ -4872,6 +4874,7 @@ "url": "https://github.com/sponsors/ai" } ], + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.8.19", "caniuse-lite": "^1.0.30001751", @@ -5293,6 +5296,7 @@ "dev": true, "hasInstallScript": true, "license": "MIT", + "peer": true, "bin": { "esbuild": "bin/esbuild" }, @@ -5853,6 +5857,7 @@ "integrity": "sha512-AkXIIFcaazymvey2i/+F94XRnM6TsVLZDhBMLsd1Sf/W0wzsvvpjeyUrCZD6HGG4SDYPgDJDBKeiJTBb10WzMg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@jest/core": "30.3.0", "@jest/types": "30.3.0", @@ -7731,6 +7736,7 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, + "peer": true, "engines": { "node": ">=12" }, @@ -7943,6 +7949,7 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" diff --git a/source/image-handler/src/image-request.ts b/source/image-handler/src/image-request.ts index 2cb633128..f98b9283f 100644 --- a/source/image-handler/src/image-request.ts +++ b/source/image-handler/src/image-request.ts @@ -9,6 +9,7 @@ import { ImageFormatTypes, ImageHandlerError, ImageRequestInfo, + isClientError, RequestTypes, StatusCodes, } from './lib'; @@ -92,6 +93,11 @@ export class ImageRequest { */ public async setup(event: APIGatewayProxyEventV2): Promise { try { + // Recover requests where a whole `srcset` value was placed into a single image URL + // (see fixSrcSetPath). Sanitizing once here means every downstream parser + // (request type, key, edits) operates on the first, valid candidate. + event = { ...event, rawPath: ImageRequest.fixSrcSetPath(event.rawPath) }; + let imageRequestInfo: ImageRequestInfo = {}; imageRequestInfo.requestType = this.parseRequestType(event); @@ -132,7 +138,9 @@ export class ImageRequest { return imageRequestInfo; } catch (error) { - if (error.code && error.code !== 'NoSuchKey') { + if (isClientError(error)) { + logger.debug('Client error while setting up the image request. Error: ', error); + } else { logger.warn('Error occurred while setting up the image request. Error: ', error); } @@ -140,6 +148,21 @@ export class ImageRequest { } } + /** + * fixes image source set requests that somehow reach the image handler + * @param rawPath The raw request path. + * @returns The sanitized path, or the original path if it is not a srcset-style request. + */ + public static fixSrcSetPath(rawPath: string): string { + // Bounded quantifiers on the descriptor digits and a single greedy `[\s\S]*$` tail (instead + // of the ambiguous `(?:[\s,].*)?$`) keep this linear — avoiding the polynomial backtracking + // CodeQL flags for a regex run on uncontrolled request input. + return (rawPath ?? '').replace( + /(\.(?:jpe?g|png|webp|tiff?|svg|gif|avif))(?:%20|\s)\d{1,6}(?:\.\d{1,4})?[wx][\s\S]*$/i, + '$1', + ); + } + /** * Gets the original image from an Amazon S3 bucket. * @param bucket The name of the bucket containing the image. diff --git a/source/image-handler/src/index.ts b/source/image-handler/src/index.ts index 27f58e28d..f266558bc 100755 --- a/source/image-handler/src/index.ts +++ b/source/image-handler/src/index.ts @@ -3,7 +3,7 @@ import { ImageHandler } from './image-handler'; import { ImageRequest } from './image-request'; -import { Headers, ImageHandlerExecutionResult, StatusCodes } from './lib'; +import { Headers, ImageHandlerExecutionResult, isClientError, StatusCodes } from './lib'; import { S3 } from '@aws-sdk/client-s3'; import { APIGatewayProxyEventV2 } from 'aws-lambda'; import { Logger } from '@aws-lambda-powertools/logger'; @@ -78,11 +78,16 @@ export async function handler(event: APIGatewayProxyEventV2): Promise; @@ -10,7 +10,16 @@ export type Headers = Record; export type ImageEdits = Record; export class ImageHandlerError extends Error { - constructor(public readonly status: StatusCodes, public readonly code: string, public readonly message: string) { + constructor( + public readonly status: StatusCodes, + public readonly code: string, + public readonly message: string, + ) { super(); } } + +export function isClientError(error: unknown, statusCode?: number): boolean { + const status = statusCode ?? (error as { status?: number })?.status; + return typeof status === 'number' && status >= 400 && status < 500; +} diff --git a/source/image-handler/src/thumbor-mapper.ts b/source/image-handler/src/thumbor-mapper.ts index 53fc8aa9f..4c798d1d6 100644 --- a/source/image-handler/src/thumbor-mapper.ts +++ b/source/image-handler/src/thumbor-mapper.ts @@ -9,6 +9,13 @@ import { ImageEdits, ImageFitTypes, ImageFormatTypes } from './lib'; export class ThumborMapper { private static readonly EMPTY_IMAGE_EDITS: ImageEdits = {}; + // Upper bound (px, longest side) applied to otherwise-unbounded `0x0` resize requests + // (e.g. `fit-in/0x0`). Without a bound, sharp returns the image at its full native (cropped) + // resolution; for multi-megapixel originals the base64-encoded result exceeds the 6 MB Lambda + // payload limit and fails with `TooLargeImageException`. Override via the + // `UNBOUNDED_FIT_IN_MAX_DIMENSION` environment variable. + private static readonly UNBOUNDED_FIT_IN_MAX_DIMENSION = parseInt(process.env.UNBOUNDED_FIT_IN_MAX_DIMENSION) || 4000; + /** * Initializer function for creating a new Thumbor mapping, used by the image * handler to perform image modifications based on legacy URL path requests. @@ -451,8 +458,19 @@ export class ThumborMapper { if (width === 0 || height === 0) { resizeEdit.resize.fit = ImageFitTypes.INSIDE; } - resizeEdit.resize.width = width === 0 ? null : width; - resizeEdit.resize.height = height === 0 ? null : height; + + if (width === 0 && height === 0) { + // `0x0` (e.g. `fit-in/0x0`) requests no resize bound, which would return the image at + // its full native (cropped) resolution and can blow past the 6 MB Lambda payload limit. + // Clamp the longest side so large images are downscaled to fit; `withoutEnlargement` + // leaves images already smaller than the bound untouched (no upscaling). + resizeEdit.resize.width = ThumborMapper.UNBOUNDED_FIT_IN_MAX_DIMENSION; + resizeEdit.resize.height = ThumborMapper.UNBOUNDED_FIT_IN_MAX_DIMENSION; + resizeEdit.resize.withoutEnlargement = true; + } else { + resizeEdit.resize.width = width === 0 ? null : width; + resizeEdit.resize.height = height === 0 ? null : height; + } return resizeEdit; } diff --git a/source/image-handler/test/image-request/fix-srcset-path.spec.ts b/source/image-handler/test/image-request/fix-srcset-path.spec.ts new file mode 100644 index 000000000..5d5e8bb6c --- /dev/null +++ b/source/image-handler/test/image-request/fix-srcset-path.spec.ts @@ -0,0 +1,90 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { ImageRequest } from '../../src/image-request'; + +describe('fixSrcSetPath', () => { + it('Should recover the first candidate from a srcset with width descriptors (the reported failing request)', () => { + // Arrange + const rawPath = + '/2022/10/673obCnS14wB/175x0:450x450/fit-in/90x0/rachel-holmes-quelle-gamedistribution.jpg%2090w,%20https://images.t-online.de/2022/10/673obCnS14wB/175x0:450x450/fit-in/180x0/rachel-holmes-quelle-gamedistribution.jpg%20180w'; + + // Act + const result = ImageRequest.fixSrcSetPath(rawPath); + + // Assert + expect(result).toEqual('/2022/10/673obCnS14wB/175x0:450x450/fit-in/90x0/rachel-holmes-quelle-gamedistribution.jpg'); + }); + + it('Should recover the first candidate from a srcset with pixel-density descriptors', () => { + // Arrange + const rawPath = + '/2026/06/1Ff18x3yYonF/0x222:4283x2409/fit-in/168x0/joshua-kimmich.jpg%201x,%20https://images.t-online.de/2026/06/1Ff18x3yYonF/0x222:4283x2409/fit-in/336x0/joshua-kimmich.jpg%202x'; + + // Act + const result = ImageRequest.fixSrcSetPath(rawPath); + + // Assert + expect(result).toEqual('/2026/06/1Ff18x3yYonF/0x222:4283x2409/fit-in/168x0/joshua-kimmich.jpg'); + }); + + it('Should handle fractional pixel-density descriptors', () => { + // Arrange + const rawPath = + '/2024/07/abc/fit-in/400x0/image.jpg%201.5x,%20https://images.t-online.de/2024/07/abc/fit-in/800x0/image.jpg%203x'; + + // Act + const result = ImageRequest.fixSrcSetPath(rawPath); + + // Assert + expect(result).toEqual('/2024/07/abc/fit-in/400x0/image.jpg'); + }); + + it('Should strip a trailing descriptor even when there is only a single candidate', () => { + // Arrange + const rawPath = '/2024/07/abc/fit-in/90x0/image.jpg%2090w'; + + // Act + const result = ImageRequest.fixSrcSetPath(rawPath); + + // Assert + expect(result).toEqual('/2024/07/abc/fit-in/90x0/image.jpg'); + }); + + it('Should also handle literal (non URL-encoded) spaces', () => { + // Arrange + const rawPath = + '/2024/07/abc/fit-in/90x0/image.jpg 90w, https://images.t-online.de/2024/07/abc/fit-in/180x0/image.jpg 180w'; + + // Act + const result = ImageRequest.fixSrcSetPath(rawPath); + + // Assert + expect(result).toEqual('/2024/07/abc/fit-in/90x0/image.jpg'); + }); + + it.each([ + '/2022/10/673obCnS14wB/175x0:450x450/fit-in/90x0/rachel-holmes-quelle-gamedistribution.jpg', + '/filters:rotate(90)/filters:grayscale()/thumbor-image.jpg', + '/fit-in/400x400/filters:watermark(bucket,folder/key.png,0,0)/image.jpg', + '/filters:rotate(90)/filters:grayscale()/thumbor-image (1) suffix.jpg', + '/eyJidWNrZXQiOiJteS1zYW1wbGUtYnVja2V0Iiwia2V5Ijoic2FtcGxlLWltYWdlLTAwMS5qcGcifQ==', + ])('Should leave a valid request path unchanged: %s', rawPath => { + expect(ImageRequest.fixSrcSetPath(rawPath)).toEqual(rawPath); + }); + + it('Should not be confused by commas inside a watermark filter', () => { + // Arrange — a comma exists, but there is no " " signature + const rawPath = '/fit-in/400x400/filters:watermark(bucket,key.png,0,0)/image.jpg'; + + // Act + const result = ImageRequest.fixSrcSetPath(rawPath); + + // Assert + expect(result).toEqual(rawPath); + }); + + it('Should handle an empty path gracefully', () => { + expect(ImageRequest.fixSrcSetPath('')).toEqual(''); + }); +}); diff --git a/source/image-handler/test/image-request/setup.spec.ts b/source/image-handler/test/image-request/setup.spec.ts index fbda83571..ab4694fb0 100644 --- a/source/image-handler/test/image-request/setup.spec.ts +++ b/source/image-handler/test/image-request/setup.spec.ts @@ -480,4 +480,56 @@ describe('setup', () => { expect(mockS3Client).toHaveReceivedCommandWith(GetObjectCommand, { Bucket: 'test', Key: '中文' }); expect(imageRequestInfo).toEqual(expectedResult); }); + + it('Should recover a srcset-style request (whole srcset placed into one URL) and serve the first candidate', async () => { + // Arrange — the reported failing request: an entire `srcset` value in a single URL + const event = build_event({ + rawPath: + '/2022/10/673obCnS14wB/175x0:450x450/fit-in/90x0/rachel-holmes-quelle-gamedistribution.jpg%2090w,%20https://images.t-online.de/2022/10/673obCnS14wB/175x0:450x450/fit-in/180x0/rachel-holmes-quelle-gamedistribution.jpg%20180w', + }); + process.env.SOURCE_BUCKETS = 'allowedBucket001, allowedBucket002'; + + // Mock + mockS3Client.on(GetObjectCommand).resolves({ Body: sdkStreamFromString('SampleImageContent\n') }); + + // Act + const imageRequest = new ImageRequest(new S3({})); + const imageRequestInfo = await imageRequest.setup(event); + + // Assert — resolves to the first candidate's image and edits instead of throwing a 400 + expect(mockS3Client).toHaveReceivedCommandWith(GetObjectCommand, { + Bucket: 'allowedBucket001', + Key: '2022/10/673obCnS14wB/image.jpg', + }); + expect(imageRequestInfo.requestType).toEqual(RequestTypes.THUMBOR); + expect(imageRequestInfo.key).toEqual('2022/10/673obCnS14wB/image.jpg'); + expect(imageRequestInfo.edits).toEqual({ + crop: { left: 175, top: 0, width: 450, height: 450 }, + resize: { fit: 'inside', width: 90, height: null }, + }); + }); + + it('Should recover a srcset-style request that uses pixel-density descriptors', async () => { + // Arrange + const event = build_event({ + rawPath: + '/2026/06/1Ff18x3yYonF/0x222:4283x2409/fit-in/168x0/joshua-kimmich.jpg%201x,%20https://images.t-online.de/2026/06/1Ff18x3yYonF/0x222:4283x2409/fit-in/336x0/joshua-kimmich.jpg%202x', + }); + process.env.SOURCE_BUCKETS = 'allowedBucket001, allowedBucket002'; + + // Mock + mockS3Client.on(GetObjectCommand).resolves({ Body: sdkStreamFromString('SampleImageContent\n') }); + + // Act + const imageRequest = new ImageRequest(new S3({})); + const imageRequestInfo = await imageRequest.setup(event); + + // Assert + expect(mockS3Client).toHaveReceivedCommandWith(GetObjectCommand, { + Bucket: 'allowedBucket001', + Key: '2026/06/1Ff18x3yYonF/image.jpg', + }); + expect(imageRequestInfo.requestType).toEqual(RequestTypes.THUMBOR); + expect(imageRequestInfo.key).toEqual('2026/06/1Ff18x3yYonF/image.jpg'); + }); }); diff --git a/source/image-handler/test/lib/logstash-formatter.spec.ts b/source/image-handler/test/lib/logstash-formatter.spec.ts new file mode 100644 index 000000000..e2ca9f264 --- /dev/null +++ b/source/image-handler/test/lib/logstash-formatter.spec.ts @@ -0,0 +1,65 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { LogStashFormatter } from '../../src/lib/LogstashFormatter'; +import { ImageHandlerError, StatusCodes } from '../../src/lib'; + +describe('LogStashFormatter.formatError', () => { + const formatter = new LogStashFormatter(); + + it('Should normalize an AWS SDK error `Code` (capital) to lowercase `code`', () => { + // Arrange — shape of a deserialized AWS SDK v3 S3 error (e.g. NoSuchKey) + const error = Object.assign(new Error('The specified key does not exist.'), { + name: 'NoSuchKey', + Code: 'NoSuchKey', + $metadata: { httpStatusCode: 404 }, + }); + + // Act + const formatted = formatter.formatError(error); + + // Assert + expect(formatted.code).toEqual('NoSuchKey'); + expect(formatted).not.toHaveProperty('Code'); + // unrelated AWS attributes are still preserved + expect(formatted.$metadata).toEqual({ httpStatusCode: 404 }); + }); + + it('Should keep the lowercase `code` of an ImageHandlerError unchanged', () => { + // Arrange + const error = new ImageHandlerError(StatusCodes.BAD_REQUEST, 'RequestTypeError', 'bad request'); + + // Act + const formatted = formatter.formatError(error); + + // Assert + expect(formatted.code).toEqual('RequestTypeError'); + expect(formatted.status).toEqual(StatusCodes.BAD_REQUEST); + expect(formatted).not.toHaveProperty('Code'); + }); + + it('Should not overwrite an existing lowercase `code` when both are present', () => { + // Arrange + const error = Object.assign(new Error('boom'), { code: 'lowercase-wins', Code: 'CapitalIgnored' }); + + // Act + const formatted = formatter.formatError(error); + + // Assert + expect(formatted.code).toEqual('lowercase-wins'); + expect(formatted).not.toHaveProperty('Code'); + }); + + it('Should leave errors without a code untouched', () => { + // Arrange + const error = new Error('plain error'); + + // Act + const formatted = formatter.formatError(error); + + // Assert + expect(formatted).not.toHaveProperty('code'); + expect(formatted).not.toHaveProperty('Code'); + expect(formatted.message).toEqual('plain error'); + }); +}); diff --git a/source/image-handler/test/thumbor-mapper/resize.spec.ts b/source/image-handler/test/thumbor-mapper/resize.spec.ts index 9cc590aba..10d9e9e57 100644 --- a/source/image-handler/test/thumbor-mapper/resize.spec.ts +++ b/source/image-handler/test/thumbor-mapper/resize.spec.ts @@ -90,7 +90,7 @@ describe('resize', () => { expect(edits).toEqual(expectedResult.edits); }); - it('Should pass if the proper edit translations are applied and in the correct order', () => { + it('Should clamp an unbounded 0x0 request to the max dimension without enlarging', () => { // Arrange const path = '/0x0/test-image-001.jpg'; @@ -100,7 +100,7 @@ describe('resize', () => { // Assert const expectedResult = { - edits: { resize: { width: null, height: null, fit: 'inside' } }, + edits: { resize: { width: 4000, height: 4000, fit: 'inside', withoutEnlargement: true } }, }; expect(edits).toEqual(expectedResult.edits); });