From f135101aef9747c1330e623054359e4a0b82dd4d Mon Sep 17 00:00:00 2001 From: Bennet Gallein Date: Thu, 30 Nov 2023 09:56:46 +0100 Subject: [PATCH 1/8] feat(thumbor-urls): add logic to parse bucket from path --- source/image-handler/image-request.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source/image-handler/image-request.ts b/source/image-handler/image-request.ts index 06ac23cf3..e1c4f27a8 100644 --- a/source/image-handler/image-request.ts +++ b/source/image-handler/image-request.ts @@ -223,6 +223,13 @@ export class ImageRequest { } else if (requestType === RequestTypes.THUMBOR || requestType === RequestTypes.CUSTOM) { // Use the default image source bucket env var const sourceBuckets = this.getAllowedSourceBuckets(); + // Take the path and split it at "/" to get each "word" in the url as array + let potentialBucket = event.path.split("/"); + // filter out all parts that are not an bucket-url + potentialBucket = potentialBucket.filter(e => sourceBuckets.includes(e)); + // return the first match + if (potentialBucket.length > 0) return potentialBucket[0]; + return sourceBuckets[0]; } else { throw new ImageHandlerError( From 2589355e919b95d466301ffff407db422c1035ce Mon Sep 17 00:00:00 2001 From: Bennet Gallein Date: Thu, 30 Nov 2023 09:57:04 +0100 Subject: [PATCH 2/8] feat: add tests --- .../image-request/parse-image-bucket.spec.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/source/image-handler/test/image-request/parse-image-bucket.spec.ts b/source/image-handler/test/image-request/parse-image-bucket.spec.ts index d3173dd50..c757d8195 100644 --- a/source/image-handler/test/image-request/parse-image-bucket.spec.ts +++ b/source/image-handler/test/image-request/parse-image-bucket.spec.ts @@ -126,4 +126,30 @@ describe("parseImageBucket", () => { }); } }); + + it("should parse bucket-name from first part in thubor request but fail since it's not allowed", () => { + // Arrange + const event = { path: "/filters:grayscale()/test-bucket/test-image-001.jpg" }; + process.env.SOURCE_BUCKETS = "allowedBucket001, allowedBucket002"; + + // Act + const imageRequest = new ImageRequest(s3Client, secretProvider); + + const bucket = imageRequest.parseImageBucket(event, RequestTypes.THUMBOR); + // Assert + expect(bucket).toEqual("allowedBucket001") + }) + + it("should parse bucket-name from first part in thubor request and return it", () => { + // Arrange + const event = { path: "/filters:grayscale()/test-bucket/test-image-001.jpg" }; + process.env.SOURCE_BUCKETS = "allowedBucket001, test-bucket"; + + // Act + const imageRequest = new ImageRequest(s3Client, secretProvider); + + const bucket = imageRequest.parseImageBucket(event, RequestTypes.THUMBOR); + // Assert + expect(bucket).toEqual("test-bucket") + }) }); From 7aed95cbd672a8c72e8ab3766c17b8cba82354e7 Mon Sep 17 00:00:00 2001 From: Bennet Gallein Date: Fri, 1 Dec 2023 15:07:07 +0100 Subject: [PATCH 3/8] fix: replace bucket-name in url if present --- source/image-handler/image-request.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source/image-handler/image-request.ts b/source/image-handler/image-request.ts index e1c4f27a8..614200eb8 100644 --- a/source/image-handler/image-request.ts +++ b/source/image-handler/image-request.ts @@ -101,7 +101,7 @@ export class ImageRequest { imageRequestInfo.requestType = this.parseRequestType(event); imageRequestInfo.bucket = this.parseImageBucket(event, imageRequestInfo.requestType); - imageRequestInfo.key = this.parseImageKey(event, imageRequestInfo.requestType); + imageRequestInfo.key = this.parseImageKey(event, imageRequestInfo.requestType, bucket); imageRequestInfo.edits = this.parseImageEdits(event, imageRequestInfo.requestType); const originalImage = await this.getOriginalImage(imageRequestInfo.bucket, imageRequestInfo.key); @@ -272,7 +272,7 @@ export class ImageRequest { * @param requestType Type of the request. * @returns The name of the appropriate Amazon S3 key. */ - public parseImageKey(event: ImageHandlerEvent, requestType: RequestTypes): string { + public parseImageKey(event: ImageHandlerEvent, requestType: RequestTypes, bucket: string): string { if (requestType === RequestTypes.DEFAULT) { // Decode the image request and return the image key const { key } = this.decodeRequest(event); @@ -299,6 +299,7 @@ export class ImageRequest { return decodeURIComponent( path + .replace(`/${bucket}`, '') .replace(/\/\d+x\d+:\d+x\d+(?=\/)/g, "") .replace(/\/\d+x\d+(?=\/)/g, "") .replace(/filters:watermark\(.*\)/u, "") From 57137dde535b2749eee3bb2e8e7ba93a19fe9cc4 Mon Sep 17 00:00:00 2001 From: Bennet Gallein Date: Fri, 1 Dec 2023 15:10:01 +0100 Subject: [PATCH 4/8] fix: also replace key --- source/image-handler/image-request.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/image-handler/image-request.ts b/source/image-handler/image-request.ts index 614200eb8..d3b6f6a9a 100644 --- a/source/image-handler/image-request.ts +++ b/source/image-handler/image-request.ts @@ -299,7 +299,6 @@ export class ImageRequest { return decodeURIComponent( path - .replace(`/${bucket}`, '') .replace(/\/\d+x\d+:\d+x\d+(?=\/)/g, "") .replace(/\/\d+x\d+(?=\/)/g, "") .replace(/filters:watermark\(.*\)/u, "") @@ -307,6 +306,7 @@ export class ImageRequest { .replace(/\/fit-in(?=\/)/g, "") .replace(/^\/+/g, "") .replace(/^\/+/, "") + .replace(new RegExp(bucket + "\/"), '') ); } From 7fad9797e4d619e328610c01701cc3f91790d785 Mon Sep 17 00:00:00 2001 From: Bennet Gallein Date: Fri, 1 Dec 2023 15:18:25 +0100 Subject: [PATCH 5/8] fix: well --- source/image-handler/image-request.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/image-handler/image-request.ts b/source/image-handler/image-request.ts index d3b6f6a9a..0dbb2475e 100644 --- a/source/image-handler/image-request.ts +++ b/source/image-handler/image-request.ts @@ -101,7 +101,7 @@ export class ImageRequest { imageRequestInfo.requestType = this.parseRequestType(event); imageRequestInfo.bucket = this.parseImageBucket(event, imageRequestInfo.requestType); - imageRequestInfo.key = this.parseImageKey(event, imageRequestInfo.requestType, bucket); + imageRequestInfo.key = this.parseImageKey(event, imageRequestInfo.requestType, imageRequestInfo.bucket); imageRequestInfo.edits = this.parseImageEdits(event, imageRequestInfo.requestType); const originalImage = await this.getOriginalImage(imageRequestInfo.bucket, imageRequestInfo.key); @@ -272,7 +272,7 @@ export class ImageRequest { * @param requestType Type of the request. * @returns The name of the appropriate Amazon S3 key. */ - public parseImageKey(event: ImageHandlerEvent, requestType: RequestTypes, bucket: string): string { + public parseImageKey(event: ImageHandlerEvent, requestType: RequestTypes, bucket: string = null): string { if (requestType === RequestTypes.DEFAULT) { // Decode the image request and return the image key const { key } = this.decodeRequest(event); From bf21c1996ff65256d3fcdc3dcb51da3b1b05738b Mon Sep 17 00:00:00 2001 From: Bennet Gallein Date: Mon, 11 Dec 2023 16:30:52 +0100 Subject: [PATCH 6/8] chore(tests): added test to verify that correct bucket gets selected when non provided --- .../test/image-request/parse-image-bucket.spec.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/source/image-handler/test/image-request/parse-image-bucket.spec.ts b/source/image-handler/test/image-request/parse-image-bucket.spec.ts index c757d8195..7f463825a 100644 --- a/source/image-handler/test/image-request/parse-image-bucket.spec.ts +++ b/source/image-handler/test/image-request/parse-image-bucket.spec.ts @@ -127,7 +127,7 @@ describe("parseImageBucket", () => { } }); - it("should parse bucket-name from first part in thubor request but fail since it's not allowed", () => { + it("should parse bucket-name from first part in thumbor request but fail since it's not allowed", () => { // Arrange const event = { path: "/filters:grayscale()/test-bucket/test-image-001.jpg" }; process.env.SOURCE_BUCKETS = "allowedBucket001, allowedBucket002"; @@ -152,4 +152,17 @@ describe("parseImageBucket", () => { // Assert expect(bucket).toEqual("test-bucket") }) + + it("should take bucket-name from env-variable if not present in the URL", () => { + // Arrange + const event = { path: "/filters:grayscale()/test-image-001.jpg" }; + process.env.SOURCE_BUCKETS = "allowedBucket001, test-bucket"; + + // Act + const imageRequest = new ImageRequest(s3Client, secretProvider); + + const bucket = imageRequest.parseImageBucket(event, RequestTypes.THUMBOR); + // Assert + expect(bucket).toEqual("allowedBucket001") + }) }); From 52038a166d8536c47caea7d259f15273dc43c0d7 Mon Sep 17 00:00:00 2001 From: Bennet Gallein Date: Mon, 11 Dec 2023 16:31:36 +0100 Subject: [PATCH 7/8] fix: typo in test title --- .../image-handler/test/image-request/parse-image-bucket.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/image-handler/test/image-request/parse-image-bucket.spec.ts b/source/image-handler/test/image-request/parse-image-bucket.spec.ts index 7f463825a..663bbbc1b 100644 --- a/source/image-handler/test/image-request/parse-image-bucket.spec.ts +++ b/source/image-handler/test/image-request/parse-image-bucket.spec.ts @@ -140,7 +140,7 @@ describe("parseImageBucket", () => { expect(bucket).toEqual("allowedBucket001") }) - it("should parse bucket-name from first part in thubor request and return it", () => { + it("should parse bucket-name from first part in thumbor request and return it", () => { // Arrange const event = { path: "/filters:grayscale()/test-bucket/test-image-001.jpg" }; process.env.SOURCE_BUCKETS = "allowedBucket001, test-bucket"; From eabd936ff79b6acc39fea552f37bb27bfe2fd3f1 Mon Sep 17 00:00:00 2001 From: Bennet Gallein Date: Mon, 11 Dec 2023 18:57:36 +0100 Subject: [PATCH 8/8] feat: switch to s3:bucket syntax --- source/image-handler/image-request.ts | 7 +++++-- .../test/image-request/parse-image-bucket.spec.ts | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/source/image-handler/image-request.ts b/source/image-handler/image-request.ts index 0dbb2475e..643cdf333 100644 --- a/source/image-handler/image-request.ts +++ b/source/image-handler/image-request.ts @@ -224,7 +224,10 @@ export class ImageRequest { // Use the default image source bucket env var const sourceBuckets = this.getAllowedSourceBuckets(); // Take the path and split it at "/" to get each "word" in the url as array - let potentialBucket = event.path.split("/"); + let potentialBucket = event.path + .split("/") + .filter(e => e.startsWith('s3:')) + .map(e => e.replace("s3:", "")); // filter out all parts that are not an bucket-url potentialBucket = potentialBucket.filter(e => sourceBuckets.includes(e)); // return the first match @@ -306,7 +309,7 @@ export class ImageRequest { .replace(/\/fit-in(?=\/)/g, "") .replace(/^\/+/g, "") .replace(/^\/+/, "") - .replace(new RegExp(bucket + "\/"), '') + .replace(new RegExp("s3:" + bucket + "\/"), '') ); } diff --git a/source/image-handler/test/image-request/parse-image-bucket.spec.ts b/source/image-handler/test/image-request/parse-image-bucket.spec.ts index 663bbbc1b..e55897dcb 100644 --- a/source/image-handler/test/image-request/parse-image-bucket.spec.ts +++ b/source/image-handler/test/image-request/parse-image-bucket.spec.ts @@ -129,7 +129,7 @@ describe("parseImageBucket", () => { it("should parse bucket-name from first part in thumbor request but fail since it's not allowed", () => { // Arrange - const event = { path: "/filters:grayscale()/test-bucket/test-image-001.jpg" }; + const event = { path: "/filters:grayscale()/s3:test-bucket/test-image-001.jpg" }; process.env.SOURCE_BUCKETS = "allowedBucket001, allowedBucket002"; // Act @@ -142,7 +142,7 @@ describe("parseImageBucket", () => { it("should parse bucket-name from first part in thumbor request and return it", () => { // Arrange - const event = { path: "/filters:grayscale()/test-bucket/test-image-001.jpg" }; + const event = { path: "/filters:grayscale()/s3:test-bucket/test-image-001.jpg" }; process.env.SOURCE_BUCKETS = "allowedBucket001, test-bucket"; // Act