From a854139f4344530de1a42268828231a4d38c7c91 Mon Sep 17 00:00:00 2001 From: Daniel Phang Date: Mon, 31 Aug 2020 02:21:42 -0700 Subject: [PATCH] fix(lambda-at-edge): fix for 404s on public files (#577) --- .../lambda-at-edge/src/default-handler.ts | 6 +-- .../default-handler-with-404.test.ts | 2 +- .../default-handler-with-basepath.test.ts | 45 +++++++++++++++++++ .../default-handler/default-handler.test.ts | 45 +++++++++++++++++++ 4 files changed, 94 insertions(+), 4 deletions(-) diff --git a/packages/libs/lambda-at-edge/src/default-handler.ts b/packages/libs/lambda-at-edge/src/default-handler.ts index 649f6be923..3dd1a62705 100644 --- a/packages/libs/lambda-at-edge/src/default-handler.ts +++ b/packages/libs/lambda-at-edge/src/default-handler.ts @@ -303,16 +303,16 @@ const handleOriginResponse = async ({ }) => { const response = event.Records[0].cf.response; const request = event.Records[0].cf.request; - const uri = normaliseUri(request.uri); const { status } = response; if (status !== "403") { - const pagePath = router(manifest)(uri); - if (pagePath === "pages/404.html") { + // Set 404 status code for 404.html page. We do not need normalised URI as it will always be "/404.html" + if (request.uri === "/404.html") { response.status = "404"; response.statusDescription = "Not Found"; } return response; } + const uri = normaliseUri(request.uri); const { domainName, region } = request.origin!.s3!; const bucketName = domainName.replace(`.s3.${region}.amazonaws.com`, ""); // It's usually better to do this outside the handler, but we need to know the bucket region diff --git a/packages/libs/lambda-at-edge/tests/default-handler/default-handler-with-404.test.ts b/packages/libs/lambda-at-edge/tests/default-handler/default-handler-with-404.test.ts index f745dda373..dfccc56b58 100644 --- a/packages/libs/lambda-at-edge/tests/default-handler/default-handler-with-404.test.ts +++ b/packages/libs/lambda-at-edge/tests/default-handler/default-handler-with-404.test.ts @@ -68,7 +68,7 @@ describe("Lambda@Edge", () => { it("static 404 page should return CloudFront 404 status code after successful S3 origin response", async () => { const event = createCloudFrontEvent({ - uri: "/page/does/not/exist", + uri: "/404.html", host: "mydistribution.cloudfront.net", config: { eventType: "origin-response" } as any, response: { diff --git a/packages/libs/lambda-at-edge/tests/default-handler/default-handler-with-basepath.test.ts b/packages/libs/lambda-at-edge/tests/default-handler/default-handler-with-basepath.test.ts index c4b10d9bb0..368b38b263 100644 --- a/packages/libs/lambda-at-edge/tests/default-handler/default-handler-with-basepath.test.ts +++ b/packages/libs/lambda-at-edge/tests/default-handler/default-handler-with-basepath.test.ts @@ -170,6 +170,21 @@ describe("Lambda@Edge", () => { await runRedirectTest(path, expectedRedirect); } ); + + it("terms.html should return 200 status after successful S3 Origin response", async () => { + const event = createCloudFrontEvent({ + uri: "/terms.html", + host: "mydistribution.cloudfront.net", + config: { eventType: "origin-response" } as any, + response: { + status: "200" + } as any + }); + + const response = (await handler(event)) as CloudFrontResultResponse; + + expect(response.status).toEqual("200"); + }); }); describe("Public files routing", () => { @@ -194,6 +209,21 @@ describe("Lambda@Edge", () => { expect(request.uri).toEqual("/manifest.json"); }); + it("public file should return 200 status after successful S3 Origin response", async () => { + const event = createCloudFrontEvent({ + uri: "/manifest.json", + host: "mydistribution.cloudfront.net", + config: { eventType: "origin-response" } as any, + response: { + status: "200" + } as any + }); + + const response = (await handler(event)) as CloudFrontResultResponse; + + expect(response.status).toEqual("200"); + }); + it.each` path | expectedRedirect ${"/basepath/favicon.ico/"} | ${"/basepath/favicon.ico"} @@ -511,6 +541,21 @@ describe("Lambda@Edge", () => { expect(response.status).toEqual("404"); } ); + + it("404.html should return 404 status after successful S3 Origin response", async () => { + const event = createCloudFrontEvent({ + uri: "/404.html", + host: "mydistribution.cloudfront.net", + config: { eventType: "origin-response" } as any, + response: { + status: "200" + } as any + }); + + const response = (await handler(event)) as CloudFrontResultResponse; + + expect(response.status).toEqual("404"); + }); }); describe("500 page", () => { diff --git a/packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts b/packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts index 8266c6d627..ffe31dd718 100644 --- a/packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts +++ b/packages/libs/lambda-at-edge/tests/default-handler/default-handler.test.ts @@ -171,6 +171,21 @@ describe("Lambda@Edge", () => { await runRedirectTest(path, expectedRedirect); } ); + + it("terms.html should return 200 status after successful S3 Origin response", async () => { + const event = createCloudFrontEvent({ + uri: "/terms.html", + host: "mydistribution.cloudfront.net", + config: { eventType: "origin-response" } as any, + response: { + status: "200" + } as any + }); + + const response = (await handler(event)) as CloudFrontResultResponse; + + expect(response.status).toEqual("200"); + }); }); describe("Public files routing", () => { @@ -195,6 +210,21 @@ describe("Lambda@Edge", () => { expect(request.uri).toEqual("/manifest.json"); }); + it("public file should return 200 status after successful S3 Origin response", async () => { + const event = createCloudFrontEvent({ + uri: "/manifest.json", + host: "mydistribution.cloudfront.net", + config: { eventType: "origin-response" } as any, + response: { + status: "200" + } as any + }); + + const response = (await handler(event)) as CloudFrontResultResponse; + + expect(response.status).toEqual("200"); + }); + it.each` path | expectedRedirect ${"/favicon.ico/"} | ${"/favicon.ico"} @@ -475,6 +505,21 @@ describe("Lambda@Edge", () => { expect(response.status).toEqual("404"); } ); + + it("404.html should return 404 status after successful S3 Origin response", async () => { + const event = createCloudFrontEvent({ + uri: "/404.html", + host: "mydistribution.cloudfront.net", + config: { eventType: "origin-response" } as any, + response: { + status: "200" + } as any + }); + + const response = (await handler(event)) as CloudFrontResultResponse; + + expect(response.status).toEqual("404"); + }); }); describe("500 page", () => {