From 50e31098b3ddf57d19e50c850b247195f16269b9 Mon Sep 17 00:00:00 2001 From: moonrailgun Date: Thu, 23 Dec 2021 16:28:59 +0800 Subject: [PATCH 1/3] Fix cors problem in assets If we use assets, and send OPTIONS(preflight) request for this assets Its will return 404 because of its not match any action. --- src/index.js | 51 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/src/index.js b/src/index.js index c5698cdc..89c14a8d 100644 --- a/src/index.js +++ b/src/index.js @@ -316,6 +316,31 @@ module.exports = { this.sendError(req, res, err); }, + corsHandler(settings, req, res) { + // CORS headers + if (settings.cors) { + // Set CORS headers to `res` + this.writeCorsHeaders(settings, req, res, true); + + // Is it a Preflight request? + if (req.method == "OPTIONS" && req.headers["access-control-request-method"]) { + // 204 - No content + res.writeHead(204, { + "Content-Length": "0" + }); + res.end(); + + if (settings.logging) { + this.logResponse(req, res); + } + + return true; + } + } + + return false; + }, + /** * HTTP request handler. It is called from native NodeJS HTTP server. * @@ -346,6 +371,11 @@ module.exports = { } } + const shouldBreak = this.corsHandler(this.settings, req, res); + if(shouldBreak) { + return; + } + try { const result = await this.actions.rest({ req, res }, options); if (result == null) { @@ -391,24 +421,9 @@ module.exports = { await composeThen.call(this, req, res, ...route.middlewares); let params = {}; - // CORS headers - if (route.cors) { - // Set CORS headers to `res` - this.writeCorsHeaders(route, req, res, true); - - // Is it a Preflight request? - if (req.method == "OPTIONS" && req.headers["access-control-request-method"]) { - // 204 - No content - res.writeHead(204, { - "Content-Length": "0" - }); - res.end(); - - if (route.logging) - this.logResponse(req, res); - - return resolve(true); - } + const shouldBreak = this.corsHandler(route, req, res); + if(shouldBreak) { + return resolve(true); } // Merge params From a502b7ec2e8819018e1d4d042dd9a46e0712fa46 Mon Sep 17 00:00:00 2001 From: moonrailgun Date: Sat, 8 Jan 2022 00:19:03 +0800 Subject: [PATCH 2/3] Fix ci problem and change http cors check stage in httpHandler --- src/index.js | 33 +++++++++++++++------------ test/unit/service/httpHandler.spec.js | 1 + 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/index.js b/src/index.js index 89c14a8d..777fa6df 100644 --- a/src/index.js +++ b/src/index.js @@ -371,16 +371,16 @@ module.exports = { } } - const shouldBreak = this.corsHandler(this.settings, req, res); - if(shouldBreak) { - return; - } - try { const result = await this.actions.rest({ req, res }, options); if (result == null) { // Not routed. + const shouldBreak = this.corsHandler(this.settings, req, res); // check cors settings first + if(shouldBreak) { + return; + } + // Serve assets static files if (this.serve) { this.serve(req, res, err => { @@ -880,16 +880,19 @@ module.exports = { const ctx = req.$ctx; let responseType = "application/json; charset=utf-8"; - if (ctx.meta.$responseType) { - responseType = ctx.meta.$responseType; - } - if (ctx.meta.$responseHeaders) { - Object.keys(ctx.meta.$responseHeaders).forEach(key => { - if (key === "Content-Type" && !responseType) - responseType = ctx.meta.$responseHeaders[key]; - else - res.setHeader(key, ctx.meta.$responseHeaders[key]); - }); + + if (ctx) { + if (ctx.meta.$responseType) { + responseType = ctx.meta.$responseType; + } + if (ctx.meta.$responseHeaders) { + Object.keys(ctx.meta.$responseHeaders).forEach(key => { + if (key === "Content-Type" && !responseType) + responseType = ctx.meta.$responseHeaders[key]; + else + res.setHeader(key, ctx.meta.$responseHeaders[key]); + }); + } } // Return with the error as JSON object diff --git a/test/unit/service/httpHandler.spec.js b/test/unit/service/httpHandler.spec.js index 859b25dd..c9c56d07 100644 --- a/test/unit/service/httpHandler.spec.js +++ b/test/unit/service/httpHandler.spec.js @@ -17,6 +17,7 @@ const MockContext = () => Object.assign({ logger: MockLogger(), sendError: jest.fn(), send404: jest.fn(), + corsHandler: jest.fn(() => false), }); const MockRequest = () => Object.assign(jest.fn(), { headers: {} }); From e46107036e916983e60a56752a684436beda6ae1 Mon Sep 17 00:00:00 2001 From: moonrailgun Date: Sat, 8 Jan 2022 20:43:26 +0800 Subject: [PATCH 3/3] Add more test case for assets cors check --- test/integration/index.spec.js | 50 ++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/integration/index.spec.js b/test/integration/index.spec.js index bb06bed9..e7966bcd 100644 --- a/test/integration/index.spec.js +++ b/test/integration/index.spec.js @@ -559,6 +559,56 @@ describe("Test only assets", () => { }); +describe("Test assets with cors", () => { + let broker; + let service; + let server; + + beforeAll(() => { + [broker, service, server] = setup({ + assets: { + folder: path.join(__dirname, "..", "assets") + }, + routes: null, + cors: { + origin: "a" + } + }); + return broker.start(); + }); + afterAll(() => broker.stop()); + + it("pass cors and return 204", () => { + return request(server) + .options("/lorem.txt") + .set("Origin", "a") + .set("Access-Control-Request-Method", "GET") + .then(res => { + expect(res.statusCode).toBe(204); + }); + }); + + it("pass cors (not exist file) and return 204", () => { + return request(server) + .options("/not-found.txt") + .set("Origin", "a") + .set("Access-Control-Request-Method", "GET") + .then(res => { + expect(res.statusCode).toBe(204); + }); + }); + + it("not pass cors and return 204", () => { + return request(server) + .options("/lorem.txt") + .set("Origin", "b") + .set("Access-Control-Request-Method", "GET") + .then(res => { + expect(res.statusCode).toBe(403); + }); + }); +}); + describe("Test assets & API route", () => { let broker; let service;