Merge pull request #289 from moonrailgun/moonrailgun/assets-cors-pref…

…light-req

Fix cors problem in assets

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.
 		 *
@@ -351,6 +376,11 @@ module.exports = {
 				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 => {
@@ -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
@@ -865,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

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;

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: {} });