From dd4844d53b787dc14bc5eecae2bc5674425200b7 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Mon, 29 Jul 2019 16:56:32 +0200 Subject: [PATCH] [FEATURE] Properties File Escaping (#214) In serveResources middleware use nonAsciiEscaper processor to escape .properties files. --- lib/middleware/MiddlewareManager.js | 11 +- lib/middleware/serveIndex.js | 8 +- lib/middleware/serveResources.js | 38 ++--- test/lib/server/main.js | 14 +- test/lib/server/middleware/serveIndex.js | 2 +- test/lib/server/middleware/serveResources.js | 153 +++++++++++++++++++ 6 files changed, 191 insertions(+), 35 deletions(-) create mode 100644 test/lib/server/middleware/serveResources.js diff --git a/lib/middleware/MiddlewareManager.js b/lib/middleware/MiddlewareManager.js index e3c6b04d..a6ac2001 100644 --- a/lib/middleware/MiddlewareManager.js +++ b/lib/middleware/MiddlewareManager.js @@ -108,7 +108,16 @@ class MiddlewareManager { await this.addMiddleware("discovery", { mountPath: "/discovery" }); - await this.addMiddleware("serveResources"); + await this.addMiddleware("serveResources", { + wrapperCallback: (serveResourcesModule) => { + return ({resources}) => { + return serveResourcesModule({ + resources, + tree: this.tree + }); + }; + } + }); await this.addMiddleware("serveThemes"); await this.addMiddleware("versionInfo", { mountPath: "/resources/sap-ui-version.json", diff --git a/lib/middleware/serveIndex.js b/lib/middleware/serveIndex.js index 606e8888..3df8b077 100644 --- a/lib/middleware/serveIndex.js +++ b/lib/middleware/serveIndex.js @@ -2,8 +2,6 @@ const log = require("@ui5/logger").getLogger("server:middleware:serveIndex"); const mime = require("mime-types"); const parseurl = require("parseurl"); -const rProperties = /\.properties$/; - const KB = 1024; const MB = KB * KB; const GB = KB * KB * KB; @@ -15,11 +13,7 @@ const GB = KB * KB * KB; * @returns {string} mime type */ function getMimeType(resource) { - if (rProperties.test(resource.getPath())) { - return "text/plain;charset=ISO-8859-1"; - } else { - return mime.lookup(resource.getPath()) || "application/octet-stream"; - } + return mime.lookup(resource.getPath()) || "application/octet-stream"; } /** diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index e1ade546..62e88a15 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -23,10 +23,11 @@ function isFresh(req, res) { * @param {module:@ui5/fs.AbstractReader} resources.all Resource collection which contains the workspace and the project dependencies * @returns {Function} Returns a server middleware closure. */ -function createMiddleware({resources}) { - return function serveResources(req, res, next) { - const pathname = parseurl(req).pathname; - resources.all.byPath(pathname).then(function(resource) { +function createMiddleware({resources, tree: project}) { + return async function serveResources(req, res, next) { + try { + const pathname = parseurl(req).pathname; + const resource = await resources.all.byPath(pathname); if (!resource) { // Not found next(); return; @@ -36,22 +37,21 @@ function createMiddleware({resources}) { log.verbose("\n" + treeify.asTree(resource.getPathTree())); } - let type; - let charset; const resourcePath = resource.getPath(); + const type = mime.lookup(resourcePath) || "application/octet-stream"; + const charset = mime.charset(type); if (rProperties.test(resourcePath)) { - // Special handling for *.properties files which are encoded with charset ISO-8859-1. - type = "text/plain"; - charset = "ISO-8859-1"; - } else { - type = mime.lookup(resourcePath) || "application/octet-stream"; + // Special handling for *.properties files escape non ascii characters. + const nonAsciiEscaper = require("@ui5/builder").processors.nonAsciiEscaper; + const propertiesFileSourceEncoding = project && project.resources && project.resources.configuration && project.resources.configuration.propertiesFileSourceEncoding; + const encoding = nonAsciiEscaper.getEncodingFromAlias(propertiesFileSourceEncoding || "ISO-8859-1"); + await nonAsciiEscaper({ + resources: [resource], options: { + encoding + } + }); } - if (!res.getHeader("Content-Type")) { - if (!charset) { - charset = mime.charset(type); - } - res.setHeader("Content-Type", type + (charset ? "; charset=" + charset : "")); } @@ -71,7 +71,7 @@ function createMiddleware({resources}) { // UTF-8 anyways. // Also, only process .library, *.js and *.json files. Just like it's done in Application- // and LibraryBuilder - if (charset === "UTF-8" && rReplaceVersion.test(resourcePath)) { + if ((!charset || charset === "UTF-8") && rReplaceVersion.test(resourcePath)) { if (resource._project) { stream = stream.pipe(replaceStream("${version}", resource._project.version)); } else { @@ -80,9 +80,9 @@ function createMiddleware({resources}) { } stream.pipe(res); - }).catch((err) => { + } catch (err) { next(err); - }); + } }; } diff --git a/test/lib/server/main.js b/test/lib/server/main.js index 1baad208..67c6583e 100644 --- a/test/lib/server/main.js +++ b/test/lib/server/main.js @@ -67,18 +67,18 @@ test("Get resource from application.a with replaced version placeholder (/versio }); }); -test("Get resource from application.a (/i18n/i18n.properties) with correct charset 'ISO-8859-1'", (t) => { +test("Get resource from application.a (/i18n/i18n.properties) with correct content-type", (t) => { return request.get("/i18n/i18n.properties").then((res) => { if (res.error) { t.fail(res.error.text); } t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); - t.deepEqual(res.headers["content-type"], "text/plain; charset=ISO-8859-1", "Correct content type and charset"); - t.deepEqual(Buffer.from(res.text, "latin1").toString(), "showHelloButtonText=Say Hello!", "Correct response"); + t.deepEqual(res.headers["content-type"], "application/octet-stream", "Correct content type"); + t.deepEqual(res.body.toString(), "showHelloButtonText=Say Hello!", "Correct response"); }); }); -test("Get resource from application.a (/i18n/i18n_de.properties) with correct encoding 'ISO-8859-1'", (t) => { +test("Get resource from application.a (/i18n/i18n_de.properties) with correct content-type'", (t) => { return request.get("/i18n/i18n_de.properties") .responseType("arraybuffer") .then((res) => { @@ -86,10 +86,10 @@ test("Get resource from application.a (/i18n/i18n_de.properties) with correct en t.fail(res.error.text); } t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); - t.deepEqual(res.headers["content-type"], "text/plain; charset=ISO-8859-1", - "Correct content type and charset"); + t.deepEqual(res.headers["content-type"], "application/octet-stream", + "Correct content type"); - t.deepEqual(res.body.toString("latin1"), "showHelloButtonText=Say ä!", "Correct response"); + t.deepEqual(res.body.toString(), "showHelloButtonText=Say \\u00e4!", "Correct response"); // Because it took so long to figure this out I keep the below line. It is equivalent to the deepEqual above // t.deepEqual(res.body.toString("latin1"), Buffer.from("showHelloButtonText=Say \u00e4!", "latin1").toString("latin1"), // "Correct response"); diff --git a/test/lib/server/middleware/serveIndex.js b/test/lib/server/middleware/serveIndex.js index f2bba1d0..9b4dff53 100644 --- a/test/lib/server/middleware/serveIndex.js +++ b/test/lib/server/middleware/serveIndex.js @@ -41,7 +41,7 @@ test.serial("Check if index for files is created", (t) => { end: function(content) { t.regex(content, /1\.00 KB<\/td>\s*myFile1\.meh<\/a><\/td>\s*application\/octet-stream<\/td>/); t.regex(content, /1\.00 MB<\/td>\s*myFile2\.js<\/a><\/td>\s*application\/javascript<\/td>/g); - t.regex(content, /1\.00 GB<\/td>\s*myFile3\.properties<\/a><\/td>\s*text\/plain;charset=ISO-8859-1<\/td>/g); + t.regex(content, /1\.00 GB<\/td>\s*myFile3\.properties<\/a><\/td>\s*application\/octet-stream<\/td>/g); resolve(); }, }; diff --git a/test/lib/server/middleware/serveResources.js b/test/lib/server/middleware/serveResources.js new file mode 100644 index 00000000..542aacc8 --- /dev/null +++ b/test/lib/server/middleware/serveResources.js @@ -0,0 +1,153 @@ +const test = require("ava"); +const sinon = require("sinon"); +const resourceFactory = require("@ui5/fs").resourceFactory; +const serveResourcesMiddleware = require("../../../../lib/middleware/serveResources"); +const writeResource = function(writer, path, size, stringContent) { + const statInfo = { + ino: 0, + ctime: new Date(), + mtime: new Date(), + size: size, + isDirectory: function() { + return false; + } + }; + const resource = resourceFactory.createResource({path, buffer: Buffer.from(stringContent, "latin1"), statInfo}); + // stub resource functionality in order to be able to get the Resource's content. Otherwise it would be drained. + sinon.stub(resource, "getStream").returns({ + pipe: function() { + } + }); + return writer.write(resource).then(() => { + return resource; + }); +}; +const fakeResponse = { + writeHead: function(status, contentType) {}, + getHeader: function(string) {}, + setHeader: function(string, header) {} +}; + +test.afterEach.always((t) => { + sinon.restore(); +}); + + +test.serial("Check if properties file is served properly", (t) => { + t.plan(4); + + const readerWriter = resourceFactory.createAdapter({virBasePath: "/"}); + + return writeResource(readerWriter, "/myFile3.properties", 1024 * 1024, "key=titel\nfame=straße").then((resource) => { + const setStringSpy = sinon.spy(resource, "setString"); + const middleware = serveResourcesMiddleware({ + resources: { + all: readerWriter + }, + tree: { + resources: { + configuration: { + propertiesFileSourceEncoding: "ISO-8859-1" + } + } + } + }); + + const response = fakeResponse; + + const setHeaderSpy = sinon.spy(response, "setHeader"); + const req = { + url: "/myFile3.properties", + headers: {} + }; + const next = function(err) { + throw new Error(`Next callback called with error: ${err.message}`); + }; + return middleware(req, response, next).then((o) => { + return resource.getString(); + }).then((content) => { + t.is(content, `key=titel +fame=stra\\u00dfe`); + t.is(setHeaderSpy.callCount, 2); + t.is(setStringSpy.callCount, 1); + t.is(setHeaderSpy.getCall(0).lastArg, "application/octet-stream"); + }); + }); +}); + +test.serial("Check if properties file is served properly with UTF-8", (t) => { + t.plan(4); + + const readerWriter = resourceFactory.createAdapter({virBasePath: "/"}); + + return writeResource(readerWriter, "/myFile3.properties", 1024 * 1024, "key=titel\nfame=straße").then((resource) => { + const setStringSpy = sinon.spy(resource, "setString"); + const middleware = serveResourcesMiddleware({ + resources: { + all: readerWriter + }, + tree: { + resources: { + configuration: { + propertiesFileSourceEncoding: "UTF-8" + } + } + } + }); + + const response = fakeResponse; + + const setHeaderSpy = sinon.spy(response, "setHeader"); + const req = { + url: "/myFile3.properties", + headers: {} + }; + const next = function(err) { + throw new Error(`Next callback called with error: ${err.message}`); + }; + return middleware(req, response, next).then((o) => { + return resource.getString(); + }).then((content) => { + t.is(content, `key=titel +fame=stra\\ufffde`); + t.is(setHeaderSpy.callCount, 2); + t.is(setStringSpy.callCount, 1); + t.is(setHeaderSpy.getCall(0).lastArg, "application/octet-stream"); + }); + }); +}); + +test.serial("Check if properties file is served properly without property setting", (t) => { + t.plan(4); + + const readerWriter = resourceFactory.createAdapter({virBasePath: "/"}); + + return writeResource(readerWriter, "/myFile3.properties", 1024 * 1024, "key=titel\nfame=straße").then((resource) => { + const setStringSpy = sinon.spy(resource, "setString"); + const middleware = serveResourcesMiddleware({ + resources: { + all: readerWriter + } + }); + + const response = fakeResponse; + + const setHeaderSpy = sinon.spy(response, "setHeader"); + const req = { + url: "/myFile3.properties", + headers: {} + }; + const next = function(err) { + throw new Error(`Next callback called with error: ${err.stack}`); + }; + return middleware(req, response, next).then((o) => { + return resource.getString(); + }).then((content) => { + t.is(content, `key=titel +fame=stra\\u00dfe`); + t.is(setHeaderSpy.callCount, 2); + t.is(setStringSpy.callCount, 1); + t.is(setHeaderSpy.getCall(0).lastArg, "application/octet-stream"); + }); + }); +});