From 4d971b4babade56fef154dc4a7a524d6ffa8ad1b Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 24 May 2019 13:54:42 +0200 Subject: [PATCH] [FIX] Middleware: Allow usage without express server This makes all middlewares independent of the express specific `req.path` property by manually parsing the standard `req.url`. The `parseurl` library is also used within express and makes sure to not parse the same URL of a request object again by using a cache on the request object. Follow up of https://github.com/SAP/ui5-server/pull/184 Fixes: https://github.com/SAP/karma-ui5/issues/64 --- lib/middleware/csp.js | 4 ++-- lib/middleware/nonReadRequests.js | 2 +- lib/middleware/serveIndex.js | 8 +++++--- lib/middleware/serveResources.js | 6 ++++-- lib/middleware/serveThemes.js | 6 ++++-- package.json | 1 + test/lib/server/middleware/nonReadRequests.js | 10 +++++----- test/lib/server/middleware/serveIndex.js | 2 +- 8 files changed, 23 insertions(+), 16 deletions(-) diff --git a/lib/middleware/csp.js b/lib/middleware/csp.js index a5fbb15e..6422b2e5 100644 --- a/lib/middleware/csp.js +++ b/lib/middleware/csp.js @@ -1,4 +1,4 @@ -const url = require("url"); +const parseurl = require("parseurl"); const querystring = require("querystring"); const HEADER_CONTENT_SECURITY_POLICY = "Content-Security-Policy"; @@ -28,7 +28,7 @@ function createMiddleware(sCspUrlParameterName, oConfig) { } = oConfig; return function csp(req, res, next) { - const oParsedURL = url.parse(req.url); + const oParsedURL = parseurl(req); if (req.method === "POST" ) { if (req.headers["content-type"] === "application/csp-report" diff --git a/lib/middleware/nonReadRequests.js b/lib/middleware/nonReadRequests.js index f6fc685f..2a1d248c 100644 --- a/lib/middleware/nonReadRequests.js +++ b/lib/middleware/nonReadRequests.js @@ -12,7 +12,7 @@ function createMiddleware() { // Handle anything but read operations *before* the serveIndex middleware // as it will reject them with a 405 (Method not allowed) instead of 404 like our old tooling if (req.method !== "GET" && req.method !== "HEAD" && req.method !== "OPTIONS") { - res.status(404).end(`Cannot ${req.method} ${req.path}`); + res.status(404).end(`Cannot ${req.method} ${req.url}`); } else { next(); } diff --git a/lib/middleware/serveIndex.js b/lib/middleware/serveIndex.js index d246f38b..d8292c0e 100644 --- a/lib/middleware/serveIndex.js +++ b/lib/middleware/serveIndex.js @@ -1,5 +1,6 @@ const log = require("@ui5/logger").getLogger("server:middleware:serveIndex"); const mime = require("mime-types"); +const parseurl = require("parseurl"); const rProperties = /\.properties$/; @@ -138,8 +139,9 @@ function createContent(path, resourceInfos) { */ function createMiddleware({resourceCollections}) { return function serveIndex(req, res, next) { - log.verbose("\n Listing index of " +req.path); - const glob = req.path + (req.path.endsWith("/") ? "*" : "/*"); + const pathname = parseurl(req).pathname; + log.verbose("\n Listing index of " + pathname); + const glob = pathname + (pathname.endsWith("/") ? "*" : "/*"); resourceCollections.combo.byGlob(glob, {nodir: false}).then((resources) => { if (!resources || resources.length == 0) { // Not found next(); @@ -151,7 +153,7 @@ function createMiddleware({resourceCollections}) { }); const resourceInfos = createResourceInfos(resources); - res.end(createContent(req.path, resourceInfos)); + res.end(createContent(pathname, resourceInfos)); }).catch((err) => { next(err); }); diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index a4a436ef..cb03b5ae 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -4,6 +4,7 @@ const treeify = require("treeify"); const replaceStream = require("replacestream"); const etag = require("etag"); const fresh = require("fresh"); +const parseurl = require("parseurl"); const rProperties = /\.properties$/; @@ -24,7 +25,8 @@ function isFresh(req, res) { */ function createMiddleware({resourceCollections}) { return function serveResources(req, res, next) { - resourceCollections.combo.byPath(req.path).then(function(resource) { + const pathname = parseurl(req).pathname; + resourceCollections.combo.byPath(pathname).then(function(resource) { if (!resource) { // Not found next(); return; @@ -65,7 +67,7 @@ function createMiddleware({resourceCollections}) { if (resource._project) { stream = stream.pipe(replaceStream("${version}", resource._project.version)); } else { - log.verbose("Project missing from resource %s", req.path); + log.verbose("Project missing from resource %s", pathname); } } diff --git a/lib/middleware/serveThemes.js b/lib/middleware/serveThemes.js index cd4094a4..1f087a7b 100644 --- a/lib/middleware/serveThemes.js +++ b/lib/middleware/serveThemes.js @@ -2,6 +2,7 @@ const themeBuilder = require("@ui5/builder").processors.themeBuilder; const fsInterface = require("@ui5/fs").fsInterface; const etag = require("etag"); const fresh = require("fresh"); +const parseurl = require("parseurl"); function isFresh(req, res) { return fresh(req.headers, { @@ -27,7 +28,8 @@ function createMiddleware({resourceCollections}) { }); return function theme(req, res, next) { - /* req.path examples: + const pathname = parseurl(req).pathname; + /* pathname examples: /resources/sap/ui/core/themes/sap_belize/library.css */ @@ -37,7 +39,7 @@ function createMiddleware({resourceCollections}) { 3 => -RTL.css suffix 4 => -parameters.json suffix */ - const themeReq = themeRequest.exec(req.path); + const themeReq = themeRequest.exec(pathname); if (!themeReq) { next(); return; diff --git a/package.json b/package.json index 7f96142e..ef6c1010 100644 --- a/package.json +++ b/package.json @@ -106,6 +106,7 @@ "fresh": "^0.5.2", "make-dir": "^3.0.0", "mime-types": "^2.1.24", + "parseurl": "^1.3.3", "portscanner": "^2.1.1", "prompt": "^1.0.0", "replacestream": "^4.0.3", diff --git a/test/lib/server/middleware/nonReadRequests.js b/test/lib/server/middleware/nonReadRequests.js index 54e94315..7d81f97d 100644 --- a/test/lib/server/middleware/nonReadRequests.js +++ b/test/lib/server/middleware/nonReadRequests.js @@ -13,9 +13,9 @@ test("Read requests", (t) => { t.pass("Next was called."); }; - middleware({method: "GET", path: "somePath"}, res, next); - middleware({method: "HEAD", path: "somePath"}, res, next); - middleware({method: "OPTIONS", path: "somePath"}, res, next); + middleware({method: "GET", url: "/somePath"}, res, next); + middleware({method: "HEAD", url: "/somePath"}, res, next); + middleware({method: "OPTIONS", url: "/somePath"}, res, next); }); test("Non read requests results in status 404 and an error message", (t) => { @@ -33,12 +33,12 @@ test("Non read requests results in status 404 and an error message", (t) => { t.deepEqual(status, 404, "Status should be 404"); return { end: function(message) { - t.deepEqual(message, "Cannot " + method + " somePath", "Finished with error message."); + t.deepEqual(message, "Cannot " + method + " /somePath", "Finished with error message."); } }; } }; - middleware({method: method, path: "somePath"}, res, next); + middleware({method: method, url: "/somePath"}, res, next); }); }); diff --git a/test/lib/server/middleware/serveIndex.js b/test/lib/server/middleware/serveIndex.js index 42179e32..452ffabe 100644 --- a/test/lib/server/middleware/serveIndex.js +++ b/test/lib/server/middleware/serveIndex.js @@ -40,7 +40,7 @@ test.serial("Check if index for files is created", (t) => { return new Promise((resolve) => { const req = { - path: "/" + url: "/" }; const res = { writeHead: function(status, contentType) {