Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rewrite-pattern): multiple rewrite pattern and substitution support #418

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/cdk-nag.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ jobs:
node-version: ${{ matrix.node-version }}
- run: |
cd source/constructs && npm i --only=dev
npx cdk synth
npx cdk synth
2 changes: 1 addition & 1 deletion .github/workflows/code-style-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion .github/workflows/run-unit-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions deployment/run-unit-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ declare -a lambda_packages=(
"constructs"
"image-handler"
"custom-resource"
"solution-utils"
)

export overrideWarningsEnabled=false
Expand Down
22 changes: 13 additions & 9 deletions source/image-handler/image-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]);
}
}
}

Expand Down
48 changes: 48 additions & 0 deletions source/image-handler/test/image-request/parse-image-edits.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
21 changes: 21 additions & 0 deletions source/image-handler/test/image-request/parse-image-key.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
18 changes: 18 additions & 0 deletions source/image-handler/test/image-request/parse-request-type.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" };
Expand Down
21 changes: 13 additions & 8 deletions source/image-handler/thumbor-mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion source/image-handler/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@
"sourceMap": true,
"types": ["node", "@types/jest"]
},
"include": ["**/*.ts"],
"include": ["**/*.ts", "../solution-utils/test/helpers.spec.ts"],
"exclude": ["package", "dist", "**/*.map"]
}
25 changes: 25 additions & 0 deletions source/solution-utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

returning array?

}
}

/**
* 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);
}
2 changes: 2 additions & 0 deletions source/solution-utils/test/get-options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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({
Expand Down
62 changes: 62 additions & 0 deletions source/solution-utils/test/helpers.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
2 changes: 1 addition & 1 deletion source/solution-utils/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@
"sourceMap": true,
"types": ["node", "@types/jest"]
},
"include": ["**/*.ts"],
"include": ["**/*.ts", "test/helpers.spec.ts"],
"exclude": ["package", "dist", "**/*.map"]
}