Skip to content
This repository has been archived by the owner on Jan 28, 2025. It is now read-only.

Commit

Permalink
fix(core): render static pages for all http methods, for options meth…
Browse files Browse the repository at this point in the history
…ods return allowed methods (#2024)
  • Loading branch information
dphang authored Nov 9, 2021
1 parent 5c4f841 commit 9fe3216
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,13 @@ describe("Pages Tests", () => {
});

["DELETE", "POST", "OPTIONS", "PUT", "PATCH"].forEach((method) => {
it(`disallows HTTP method for path ${path} with 4xx error: ${method}`, () => {
it(`allows HTTP method for path ${path} with 2xx status: ${method}`, () => {
cy.request({
url: path,
method: method,
failOnStatusCode: false
method: method
}).then((response) => {
expect(response.status).to.be.at.least(400);
expect(response.status).to.be.lessThan(500);
expect(response.status).to.be.at.least(200);
expect(response.status).to.be.lessThan(300);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,13 @@ describe("Static Files Tests", () => {
});

["DELETE", "POST", "OPTIONS", "PUT", "PATCH"].forEach((method) => {
it(`disallows HTTP method for path ${path} with 4xx error: ${method}`, () => {
it(`allows HTTP method for path ${path} with 2xx status: ${method}`, () => {
cy.request({
url: path,
method: method,
failOnStatusCode: false
method: method
}).then((response) => {
expect(response.status).to.be.at.least(400);
expect(response.status).to.be.lessThan(500);
expect(response.status).to.be.at.least(200);
expect(response.status).to.be.lessThan(300);
});
});
});
Expand Down
16 changes: 6 additions & 10 deletions packages/libs/core/src/defaultHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,16 +124,12 @@ const staticRequest = async (
const staticRoute = route.isStatic ? (route as StaticRoute) : undefined;
const statusCode = route?.statusCode ?? 200;

// For PUT, DELETE, PATCH, POST, OPTIONS just return a 405 response as these should not be supported for a page fetch.
// TODO: OPTIONS should be able to be supported now.
if (
req.method === "PUT" ||
req.method === "DELETE" ||
req.method === "PATCH" ||
req.method === "POST" ||
req.method === "OPTIONS"
) {
res.writeHead(405);
// For PUT, DELETE, PATCH, POST just return the page as this is a static page, so HTTP method doesn't really do anything.
// For OPTIONS, we should not return the content but instead return allowed methods.
if (req.method === "OPTIONS") {
res.writeHead(204, {
Allow: "OPTIONS, GET, HEAD, POST, PUT, PATCH, DELETE"
});
res.end();
return await responsePromise;
}
Expand Down
18 changes: 6 additions & 12 deletions packages/libs/lambda-at-edge/src/render/renderStaticPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,12 @@ export const renderStaticPage = async ({
const staticRoute = route.isStatic ? (route as StaticRoute) : undefined;
const statusCode = route?.statusCode ?? 200;

// For PUT, DELETE, PATCH, POST, OPTIONS just return a 405 response as these are unsupported S3 methods
// when using CloudFront S3 origin to return the page, so we keep it in parity.
// TODO: now that we are directly calling S3 in the origin request handler,
// we could implement OPTIONS method as well.
if (
request.method === "PUT" ||
request.method === "DELETE" ||
request.method === "PATCH" ||
request.method === "POST" ||
request.method === "OPTIONS"
) {
res.writeHead(405);
// For PUT, DELETE, PATCH, POST just return the page as this is a static page, so HTTP method doesn't really do anything.
// For OPTIONS, we should not return the content but instead return allowed methods.
if (req.method === "OPTIONS") {
res.writeHead(204, {
Allow: "OPTIONS, GET, HEAD, POST, PUT, PATCH, DELETE"
});
res.end();
return await responsePromise;
}
Expand Down

0 comments on commit 9fe3216

Please sign in to comment.