diff --git a/.github/workflows/cdk-nag.yml b/.github/workflows/cdk-nag.yml index 1bc47b8fb..4072b8119 100644 --- a/.github/workflows/cdk-nag.yml +++ b/.github/workflows/cdk-nag.yml @@ -23,4 +23,4 @@ jobs: node-version: ${{ matrix.node-version }} - run: | cd source/constructs && npm i --only=dev - npx cdk synth + npx cdk synth \ No newline at end of file diff --git a/.github/workflows/code-style-lint.yml b/.github/workflows/code-style-lint.yml index 2c47471be..c9a117163 100644 --- a/.github/workflows/code-style-lint.yml +++ b/.github/workflows/code-style-lint.yml @@ -33,4 +33,4 @@ jobs: node-version: ${{ matrix.node-version }} - run: | cd source && npm i --only=dev - npx --y eslint . --ext .ts + npx --y eslint . --ext .ts \ No newline at end of file diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 8fab20972..1ecc1b03d 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -21,4 +21,4 @@ jobs: - uses: github/codeql-action/init@v2 with: languages: ${{ matrix.language }} - - uses: github/codeql-action/analyze@v2 + - uses: github/codeql-action/analyze@v2 \ No newline at end of file diff --git a/.github/workflows/run-unit-test.yml b/.github/workflows/run-unit-test.yml index 0d9661ca2..acb73c9b5 100644 --- a/.github/workflows/run-unit-test.yml +++ b/.github/workflows/run-unit-test.yml @@ -23,4 +23,4 @@ jobs: node-version: ${{ matrix.node-version }} - run: | cd deployment - chmod +x ./run-unit-tests.sh && DEBUG=true ./run-unit-tests.sh + chmod +x ./run-unit-tests.sh && DEBUG=true ./run-unit-tests.sh \ No newline at end of file diff --git a/deployment/run-unit-tests.sh b/deployment/run-unit-tests.sh index 76d56a1af..43746bea4 100755 --- a/deployment/run-unit-tests.sh +++ b/deployment/run-unit-tests.sh @@ -53,6 +53,7 @@ declare -a lambda_packages=( "constructs" "image-handler" "custom-resource" + "solution-utils" ) export overrideWarningsEnabled=false diff --git a/source/image-handler/image-request.ts b/source/image-handler/image-request.ts index f17b25e44..24c845689 100644 --- a/source/image-handler/image-request.ts +++ b/source/image-handler/image-request.ts @@ -18,6 +18,7 @@ import { } from "./lib"; import { SecretProvider } from "./secret-provider"; import { ThumborMapper } from "./thumbor-mapper"; +import { parseJson, generateRegExp } from "../solution-utils/helpers"; type OriginalImageInfo = Partial<{ contentType: string; @@ -278,15 +279,18 @@ export class ImageRequest { if (requestType === RequestTypes.CUSTOM) { const { REWRITE_MATCH_PATTERN, REWRITE_SUBSTITUTION } = process.env; - if (typeof REWRITE_MATCH_PATTERN === "string") { - const patternStrings = REWRITE_MATCH_PATTERN.split("/"); - const flags = patternStrings.pop(); - const parsedPatternString = REWRITE_MATCH_PATTERN.slice(1, REWRITE_MATCH_PATTERN.length - 1 - flags.length); - const regExp = new RegExp(parsedPatternString, flags); - - path = path.replace(regExp, REWRITE_SUBSTITUTION); - } else { - path = path.replace(REWRITE_MATCH_PATTERN, REWRITE_SUBSTITUTION); + const REWRITE_MATCH_PATTERNS = parseJson(REWRITE_MATCH_PATTERN); + const REWRITE_SUBSTITUTIONS = parseJson(REWRITE_SUBSTITUTION); + + for (let k = 0; k < REWRITE_MATCH_PATTERNS.length; k++) { + const matchPattern = REWRITE_MATCH_PATTERNS[k]; + if (typeof matchPattern === "string") { + const regExp = generateRegExp(matchPattern); + path = path.replace(regExp, REWRITE_SUBSTITUTIONS[k]); + if (generateRegExp(matchPattern).test(path)) break; + } else { + path = path.replace(matchPattern, REWRITE_SUBSTITUTIONS[k]); + } } } diff --git a/source/image-handler/test/image-request/parse-image-edits.spec.ts b/source/image-handler/test/image-request/parse-image-edits.spec.ts index c8d4380c5..3fc837818 100644 --- a/source/image-handler/test/image-request/parse-image-edits.spec.ts +++ b/source/image-handler/test/image-request/parse-image-edits.spec.ts @@ -72,6 +72,54 @@ describe("parseImageEdits", () => { expect(result).toEqual(expectedResult); }); + it("Should pass if the proper result is returned for a sample custom-type image request - multipe image rewrite - confirm correct replacement - 1", () => { + // Arrange + const event = { + path: "/thumb/beach-100x100.jpg", + }; + + /** + * a custom request soecifying thumb should return a replacement using the first substitution entry + */ + process.env = { + REWRITE_MATCH_PATTERN: '["//thumb/g","//small/g","//large/g"]', + REWRITE_SUBSTITUTION: + '["/300x300/filters:quality(80)","/fit-in/600x600/filters:quality(80)","/fit-in/1200x1200/filters:quality(80)"]', + }; + + // Act + const imageRequest = new ImageRequest(s3Client, secretProvider); + const result = imageRequest.parseImageEdits(event, RequestTypes.CUSTOM); + + // Assert + const expectedResult = { jpeg: { quality: 80 }, resize: { height: 300, width: 300 } }; + expect(result).toEqual(expectedResult); + }); + + it("Should pass if the proper result is returned for a sample custom-type image request - multipe image rewrite - confirm correct replacement - 2", () => { + // Arrange + const event = { + path: "/large/test.jpg", + }; + + /** + * a custom request specifying large should return a replacement using the third substitution entry which includes a resize + */ + process.env = { + REWRITE_MATCH_PATTERN: '["//thumb/g","//small/g","//large/g"]', + REWRITE_SUBSTITUTION: + '["/300x300/filters:quality(80)","/fit-in/600x600/filters:quality(80)","/fit-in/1200x1200/filters:quality(100)"]', + }; + + // Act + const imageRequest = new ImageRequest(s3Client, secretProvider); + const result = imageRequest.parseImageEdits(event, RequestTypes.CUSTOM); + + // Assert + const expectedResult = { jpeg: { quality: 100 }, resize: { height: 1200, width: 1200, fit: "inside" } }; + expect(result).toEqual(expectedResult); + }); + it("Should throw an error if a requestType is not specified and/or the image edits cannot be parsed", () => { // Arrange const event = { diff --git a/source/image-handler/test/image-request/parse-image-key.spec.ts b/source/image-handler/test/image-request/parse-image-key.spec.ts index 67914dba2..949101646 100644 --- a/source/image-handler/test/image-request/parse-image-key.spec.ts +++ b/source/image-handler/test/image-request/parse-image-key.spec.ts @@ -240,6 +240,27 @@ describe("parseImageKey", () => { expect(result).toEqual(expectedResult); }); + it("Should pass if an image key value is provided in the custom request format - confirm key retrieval when multiple image rewrite specified", () => { + // Arrange + const event = { + path: "/thumb/test.jpg", + }; + + process.env = { + REWRITE_MATCH_PATTERN: '["//thumb/g","//small/g","//large/g"]', + REWRITE_SUBSTITUTION: + '["/300x300/filters:quality(80)","/fit-in/600x600/filters:quality(80)","/fit-in/1200x1200/filters:quality(80)"]', + }; + + // Act + const imageRequest = new ImageRequest(s3Client, secretProvider); + const result = imageRequest.parseImageKey(event, RequestTypes.CUSTOM); + + // Assert + const expectedResult = "test.jpg"; + expect(result).toEqual(expectedResult); + }); + it("Should throw an error if an unrecognized requestType is passed into the function as a parameter", () => { // Arrange const event = { diff --git a/source/image-handler/test/image-request/parse-request-type.spec.ts b/source/image-handler/test/image-request/parse-request-type.spec.ts index 0759bd8f7..c4dd604da 100644 --- a/source/image-handler/test/image-request/parse-request-type.spec.ts +++ b/source/image-handler/test/image-request/parse-request-type.spec.ts @@ -88,6 +88,24 @@ describe("parseRequestType", () => { expect(result).toEqual(expectedResult); }); + it("Should pass if the method detects a custom request with multiple rewrite patterns", () => { + // Arrange + const event = { path: "/additionalImageRequestParameters/image.jpg" }; + process.env = { + REWRITE_MATCH_PATTERN: '["//thumb/g","//small/g","//large/g"]', + REWRITE_SUBSTITUTION: + '["/300x300/filters:quality(80)","/fit-in/600x600/filters:quality(80)","/fit-in/1200x1200/filters:quality(80)"]', + }; + + // Act + const imageRequest = new ImageRequest(s3Client, secretProvider); + const result = imageRequest.parseRequestType(event); + + // Assert + const expectedResult = RequestTypes.CUSTOM; + expect(result).toEqual(expectedResult); + }); + it("Should throw an error if the method cannot determine the request type based on the three groups given", () => { // Arrange const event = { path: "12x12e24d234r2ewxsad123d34r.bmp" }; diff --git a/source/image-handler/thumbor-mapper.ts b/source/image-handler/thumbor-mapper.ts index 5cc7b4c6a..6fc305632 100644 --- a/source/image-handler/thumbor-mapper.ts +++ b/source/image-handler/thumbor-mapper.ts @@ -5,6 +5,7 @@ import Color from "color"; import ColorName from "color-name"; import { ImageEdits, ImageFitTypes, ImageFormatTypes } from "./lib"; +import { parseJson, generateRegExp } from "../solution-utils/helpers"; export class ThumborMapper { private static readonly EMPTY_IMAGE_EDITS: ImageEdits = {}; @@ -54,14 +55,18 @@ export class ThumborMapper { } else { let parsedPath = ""; - if (typeof REWRITE_MATCH_PATTERN === "string") { - const patternStrings = REWRITE_MATCH_PATTERN.split("/"); - const flags = patternStrings.pop(); - const parsedPatternString = REWRITE_MATCH_PATTERN.slice(1, REWRITE_MATCH_PATTERN.length - 1 - flags.length); - const regExp = new RegExp(parsedPatternString, flags); - parsedPath = path.replace(regExp, REWRITE_SUBSTITUTION); - } else { - parsedPath = path.replace(REWRITE_MATCH_PATTERN, REWRITE_SUBSTITUTION); + const REWRITE_MATCH_PATTERNS = parseJson(REWRITE_MATCH_PATTERN); + const REWRITE_SUBSTITUTIONS = parseJson(REWRITE_SUBSTITUTION); + + for (let k = 0; k < REWRITE_MATCH_PATTERNS.length; k++) { + const matchPattern = REWRITE_MATCH_PATTERNS[k]; + if (typeof matchPattern === "string") { + const regExp = generateRegExp(matchPattern); + parsedPath = path.replace(regExp, REWRITE_SUBSTITUTIONS[k]); + if (generateRegExp(matchPattern).test(path)) break; + } else { + parsedPath = path.replace(matchPattern, REWRITE_SUBSTITUTIONS[k]); + } } return parsedPath; diff --git a/source/image-handler/tsconfig.json b/source/image-handler/tsconfig.json index 56ebe595b..95ab2c31d 100644 --- a/source/image-handler/tsconfig.json +++ b/source/image-handler/tsconfig.json @@ -10,6 +10,6 @@ "sourceMap": true, "types": ["node", "@types/jest"] }, - "include": ["**/*.ts"], + "include": ["**/*.ts", "../solution-utils/test/helpers.spec.ts"], "exclude": ["package", "dist", "**/*.map"] } diff --git a/source/solution-utils/helpers.ts b/source/solution-utils/helpers.ts index 767f3472b..98c111071 100644 --- a/source/solution-utils/helpers.ts +++ b/source/solution-utils/helpers.ts @@ -9,3 +9,28 @@ export function isNullOrWhiteSpace(str: string): boolean { return !str || str.replace(/\s/g, "") === ""; } + +/** + * Determine if this appears to be json or a plain string + * @param str Input string to evaluate + * @returns Either a json object or a plain string + */ +export function parseJson(str) { + try { + return JSON.parse(str); + } catch (e) { + return [str]; + } +} + +/** + * Given a string that may describe actions in an image request, create a regex from the string + * @param matchPattern string to operate on + * @returns regex + */ +export function generateRegExp(matchPattern) { + const patternStrings = matchPattern.split("/"); + const flags = patternStrings.pop(); + const parsedPatternString = matchPattern.slice(1, matchPattern.length - 1 - flags.length); + return new RegExp(parsedPatternString, flags); +} diff --git a/source/solution-utils/test/get-options.test.ts b/source/solution-utils/test/get-options.test.ts index b8da776f6..8ea83e17e 100644 --- a/source/solution-utils/test/get-options.test.ts +++ b/source/solution-utils/test/get-options.test.ts @@ -24,6 +24,7 @@ describe("getOptions", () => { }); it("will return an empty object when environment variables are missing", () => { + // eslint-disable-next-line @typescript-eslint/no-var-requires const { getOptions } = require("../get-options"); expect.assertions(4); @@ -42,6 +43,7 @@ describe("getOptions", () => { }); it("will return an object with the custom user agent string", () => { + // eslint-disable-next-line @typescript-eslint/no-var-requires const { getOptions } = require("../get-options"); expect.assertions(1); expect(getOptions()).toEqual({ diff --git a/source/solution-utils/test/helpers.spec.ts b/source/solution-utils/test/helpers.spec.ts new file mode 100644 index 000000000..254461662 --- /dev/null +++ b/source/solution-utils/test/helpers.spec.ts @@ -0,0 +1,62 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { isNullOrWhiteSpace, parseJson, generateRegExp } from "../helpers"; + +describe("helpers", () => { + it("Should pass if the proper result is returned for a whitespace only string", () => { + const result = isNullOrWhiteSpace(" "); + + const expectedResult = true; + expect(result).toEqual(expectedResult); + }); + + it("Should pass if the proper result is returned for a null string", () => { + const result = isNullOrWhiteSpace(""); + + const expectedResult = true; + expect(result).toEqual(expectedResult); + }); + + it("Should pass if the proper result is returned for non-whitespace containing string", () => { + const result = isNullOrWhiteSpace("abc"); + + const expectedResult = false; + expect(result).toEqual(expectedResult); + }); + + it("Should pass if the proper result is returned for a singe entry", () => { + const result = parseJson("filter:"); + + const expectedResult = ["filter:"]; + expect(result).toEqual(expectedResult); + }); + + it("Should pass if the proper result is returned for a null string", () => { + const result = parseJson(""); + + const expectedResult = [""]; + expect(result).toEqual(expectedResult); + }); + + it("Should pass if the proper result is returned for json with multiple objects", () => { + const result = parseJson('["//thumb/g","//small/g","//large/g"]'); + + const expectedResult = ["//thumb/g", "//small/g", "//large/g"]; + expect(result).toEqual(expectedResult); + }); + + it("Should pass if the proper result is returned for a simple regex", () => { + const result = generateRegExp("/thumb/g"); + + const expectedResult1 = /thumb/g; + expect(result).toEqual(expectedResult1); + }); + + it("Should pass if the proper result is returned for a simple rege with embedded slash that must be escaped", () => { + const result = generateRegExp("//thumb/g"); + + const expectedResult1 = /\/thumb/g; + expect(result).toEqual(expectedResult1); + }); +}); diff --git a/source/solution-utils/tsconfig.json b/source/solution-utils/tsconfig.json index 56ebe595b..f5ec83c87 100644 --- a/source/solution-utils/tsconfig.json +++ b/source/solution-utils/tsconfig.json @@ -10,6 +10,6 @@ "sourceMap": true, "types": ["node", "@types/jest"] }, - "include": ["**/*.ts"], + "include": ["**/*.ts", "test/helpers.spec.ts"], "exclude": ["package", "dist", "**/*.map"] }