From 695beb64617050333f2452c8f425613756ce3e17 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Mon, 22 Jul 2019 11:06:07 +0200 Subject: [PATCH 01/10] [FEATURE] Properties File Escaping serveResources uses stringEscaper to escape .properties files. --- lib/middleware/MiddlewareManager.js | 11 ++- lib/middleware/serveIndex.js | 2 +- lib/middleware/serveResources.js | 25 +++++-- test/lib/server/main.js | 10 +-- test/lib/server/middleware/serveIndex.js | 2 +- test/lib/server/middleware/serveResources.js | 75 ++++++++++++++++++++ 6 files changed, 113 insertions(+), 12 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..c7c03fc8 100644 --- a/lib/middleware/serveIndex.js +++ b/lib/middleware/serveIndex.js @@ -16,7 +16,7 @@ const GB = KB * KB * KB; */ function getMimeType(resource) { if (rProperties.test(resource.getPath())) { - return "text/plain;charset=ISO-8859-1"; + return "text/plain;charset=UTF-8"; } else { return mime.lookup(resource.getPath()) || "application/octet-stream"; } diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index e1ade546..9223b539 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -23,7 +23,7 @@ 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}) { +function createMiddleware({resources, tree: project}) { return function serveResources(req, res, next) { const pathname = parseurl(req).pathname; resources.all.byPath(pathname).then(function(resource) { @@ -36,17 +36,34 @@ function createMiddleware({resources}) { log.verbose("\n" + treeify.asTree(resource.getPathTree())); } + const resourcePath = resource.getPath(); + if (rProperties.test(resourcePath)) { + // Special handling for *.properties files escape non ascii characters. + + const stringEscaper = require("@ui5/builder").processors.stringEscaper; + return stringEscaper({ + resources: [resource], options: { + encoding: project && project.resources && project.resources.propertiesFileEncoding + } + }).then(() => { + return resource; + }); + } + return resource; + }).then(function(resource) { + if (!resource) { // Not found + return; + } let type; let charset; const resourcePath = resource.getPath(); + // Special handling for *.properties files which are encoded with charset ISO-8859-1. if (rProperties.test(resourcePath)) { - // Special handling for *.properties files which are encoded with charset ISO-8859-1. type = "text/plain"; - charset = "ISO-8859-1"; + charset = "UTF-8"; } else { type = mime.lookup(resourcePath) || "application/octet-stream"; } - if (!res.getHeader("Content-Type")) { if (!charset) { charset = mime.charset(type); diff --git a/test/lib/server/main.js b/test/lib/server/main.js index 1baad208..73ca6d48 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 charset 'UTF-8'", (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(res.headers["content-type"], "text/plain; charset=UTF-8", "Correct content type and charset"); t.deepEqual(Buffer.from(res.text, "latin1").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 encoding 'UTF-8'", (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", + t.deepEqual(res.headers["content-type"], "text/plain; charset=UTF-8", "Correct content type and charset"); - t.deepEqual(res.body.toString("latin1"), "showHelloButtonText=Say ä!", "Correct response"); + t.deepEqual(res.body.toString("utf8"), "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..357105cc 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*text\/plain;charset=UTF-8<\/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..a1062b91 --- /dev/null +++ b/test/lib/server/middleware/serveResources.js @@ -0,0 +1,75 @@ +const test = require("ava"); +const sinon = require("sinon"); +const resourceFactory = require("@ui5/fs").resourceFactory; + +test.serial("Check if properties file is served properly", (t) => { + t.plan(4); + const serveResourcesMiddleware = require("../../../../lib/middleware/serveResources"); + const writeResource = function(writer, path, size = 0, stringContent = "abc") { + const resource = resourceFactory.createResource({path, string: stringContent}); + resource.getStatInfo = function() { + return { + ino: 0, + ctime: new Date(), + mtime: new Date(), + size: size, + isDirectory: function() { + return false; + } + }; + }; + resource.getStream = function() { + return { + pipe: function(res) { + res.end(resource.getString()); + } + }; + }; + return writer.write(resource).then(() => { + return resource; + }); + }; + + 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 = { + writeHead: function(status, contentType) { + }, + getHeader: function(string) { + + }, + setHeader: function(string, header) { + + } + }; + const setHeaderSpy = sinon.spy(response, "setHeader"); + return new Promise((resolve, reject) => { + const req = { + url: "/myFile3.properties", + headers: {} + }; + response.end = function(content) { + content.then(resolve); + }; + const next = function(err) { + reject(new Error(`Next callback called with error: ${err.message}`)); + }; + middleware(req, response, next); + }).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, "text/plain; charset=UTF-8"); + }); + }); +}); + From a5da8ef6d2b619e5849e1409372abfc2f290188e Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Wed, 24 Jul 2019 09:06:46 +0200 Subject: [PATCH 02/10] Adjust implementation according to latest changes in builder Default is "ISO-8859-1" --- lib/middleware/serveResources.js | 8 +- test/lib/server/middleware/serveResources.js | 140 +++++++++++++++---- 2 files changed, 116 insertions(+), 32 deletions(-) diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index 9223b539..287d6e21 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -40,10 +40,12 @@ function createMiddleware({resources, tree: project}) { if (rProperties.test(resourcePath)) { // Special handling for *.properties files escape non ascii characters. - const stringEscaper = require("@ui5/builder").processors.stringEscaper; - return stringEscaper({ + const nonAsciiEscaper = require("@ui5/builder").processors.nonAsciiEscaper; + const propertiesFileEncoding = project && project.resources && project.resources.propertiesFileEncoding; + const encoding = nonAsciiEscaper.getEncodingFromAlias(propertiesFileEncoding || "ISO-8859-1"); + return nonAsciiEscaper({ resources: [resource], options: { - encoding: project && project.resources && project.resources.propertiesFileEncoding + encoding } }).then(() => { return resource; diff --git a/test/lib/server/middleware/serveResources.js b/test/lib/server/middleware/serveResources.js index a1062b91..a4ec86d5 100644 --- a/test/lib/server/middleware/serveResources.js +++ b/test/lib/server/middleware/serveResources.js @@ -1,34 +1,82 @@ 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 = 0, stringContent = "abc") { + const resource = resourceFactory.createResource({path, buffer: Buffer.from(stringContent, "latin1")}); + resource.getStatInfo = function() { + return { + ino: 0, + ctime: new Date(), + mtime: new Date(), + size: size, + isDirectory: function() { + return false; + } + }; + }; + resource.getStream = function() { + return { + pipe: function(res) { + res.end(resource.getString()); + } + }; + }; + return writer.write(resource).then(() => { + return resource; + }); +}; +const fakeResponse = { + writeHead: function(status, contentType) {}, + getHeader: function(string) {}, + setHeader: function(string, header) {} +}; test.serial("Check if properties file is served properly", (t) => { t.plan(4); - const serveResourcesMiddleware = require("../../../../lib/middleware/serveResources"); - const writeResource = function(writer, path, size = 0, stringContent = "abc") { - const resource = resourceFactory.createResource({path, string: stringContent}); - resource.getStatInfo = function() { - return { - ino: 0, - ctime: new Date(), - mtime: new Date(), - size: size, - isDirectory: function() { - return false; + + 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: { + propertiesFileEncoding: "ISO-8859-1" } + } + }); + + const response = Object.assign({}, fakeResponse); + + const setHeaderSpy = sinon.spy(response, "setHeader"); + return new Promise((resolve, reject) => { + const req = { + url: "/myFile3.properties", + headers: {} }; - }; - resource.getStream = function() { - return { - pipe: function(res) { - res.end(resource.getString()); - } + response.end = function(content) { + content.then(resolve); }; - }; - return writer.write(resource).then(() => { - return resource; + const next = function(err) { + reject(new Error(`Next callback called with error: ${err.message}`)); + }; + middleware(req, response, next); + }).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, "text/plain; charset=UTF-8"); }); - }; + }); +}); + +test.serial("Check if properties file is served properly with UTF-8", (t) => { + t.plan(4); const readerWriter = resourceFactory.createAdapter({virBasePath: "/"}); @@ -37,19 +85,54 @@ test.serial("Check if properties file is served properly", (t) => { const middleware = serveResourcesMiddleware({ resources: { all: readerWriter + }, + tree: { + resources: { + propertiesFileEncoding: "UTF-8" + } } }); - const response = { - writeHead: function(status, contentType) { - }, - getHeader: function(string) { + const response = Object.assign({}, fakeResponse); - }, - setHeader: function(string, header) { + const setHeaderSpy = sinon.spy(response, "setHeader"); + return new Promise((resolve, reject) => { + const req = { + url: "/myFile3.properties", + headers: {} + }; + response.end = function(content) { + content.then(resolve); + }; + const next = function(err) { + reject(new Error(`Next callback called with error: ${err.message}`)); + }; + middleware(req, response, next); + }).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, "text/plain; charset=UTF-8"); + }); + }); +}); + +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 = Object.assign({}, fakeResponse); + const setHeaderSpy = sinon.spy(response, "setHeader"); return new Promise((resolve, reject) => { const req = { @@ -72,4 +155,3 @@ fame=stra\\u00dfe`); }); }); }); - From c9167d8e03c30283f73d25b8ac5efb284ab580b8 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Thu, 25 Jul 2019 12:55:20 +0200 Subject: [PATCH 03/10] Switch to async await to simplify workflow --- lib/middleware/serveResources.js | 32 +++++++------------- test/lib/server/middleware/serveResources.js | 2 +- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index 287d6e21..f10149b7 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -24,9 +24,10 @@ function isFresh(req, res) { * @returns {Function} Returns a server middleware closure. */ function createMiddleware({resources, tree: project}) { - return function serveResources(req, res, next) { - const pathname = parseurl(req).pathname; - resources.all.byPath(pathname).then(function(resource) { + 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; @@ -37,32 +38,21 @@ function createMiddleware({resources, tree: project}) { } const resourcePath = resource.getPath(); + let type; + let charset; if (rProperties.test(resourcePath)) { // Special handling for *.properties files escape non ascii characters. + type = "text/plain"; + charset = "UTF-8"; const nonAsciiEscaper = require("@ui5/builder").processors.nonAsciiEscaper; const propertiesFileEncoding = project && project.resources && project.resources.propertiesFileEncoding; const encoding = nonAsciiEscaper.getEncodingFromAlias(propertiesFileEncoding || "ISO-8859-1"); - return nonAsciiEscaper({ + await nonAsciiEscaper({ resources: [resource], options: { encoding } - }).then(() => { - return resource; }); - } - return resource; - }).then(function(resource) { - if (!resource) { // Not found - return; - } - let type; - let charset; - const resourcePath = resource.getPath(); - // Special handling for *.properties files which are encoded with charset ISO-8859-1. - if (rProperties.test(resourcePath)) { - type = "text/plain"; - charset = "UTF-8"; } else { type = mime.lookup(resourcePath) || "application/octet-stream"; } @@ -99,9 +89,9 @@ function createMiddleware({resources, tree: project}) { } stream.pipe(res); - }).catch((err) => { + } catch (err) { next(err); - }); + } }; } diff --git a/test/lib/server/middleware/serveResources.js b/test/lib/server/middleware/serveResources.js index a4ec86d5..2b05a436 100644 --- a/test/lib/server/middleware/serveResources.js +++ b/test/lib/server/middleware/serveResources.js @@ -2,7 +2,7 @@ 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 = 0, stringContent = "abc") { +const writeResource = function(writer, path, size, stringContent) { const resource = resourceFactory.createResource({path, buffer: Buffer.from(stringContent, "latin1")}); resource.getStatInfo = function() { return { From e32dabba85ad9af50c13d2ac82ee47db86021c68 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Fri, 26 Jul 2019 09:31:43 +0200 Subject: [PATCH 04/10] adjust config structure change of propertiesFileEncoding in ui5.yml --- lib/middleware/serveResources.js | 2 +- test/lib/server/middleware/serveResources.js | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index f10149b7..14a61bfe 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -46,7 +46,7 @@ function createMiddleware({resources, tree: project}) { charset = "UTF-8"; const nonAsciiEscaper = require("@ui5/builder").processors.nonAsciiEscaper; - const propertiesFileEncoding = project && project.resources && project.resources.propertiesFileEncoding; + const propertiesFileEncoding = project && project.resources && project.resources.configuration && project.resources.configuration.propertiesFileEncoding; const encoding = nonAsciiEscaper.getEncodingFromAlias(propertiesFileEncoding || "ISO-8859-1"); await nonAsciiEscaper({ resources: [resource], options: { diff --git a/test/lib/server/middleware/serveResources.js b/test/lib/server/middleware/serveResources.js index 2b05a436..181ab41f 100644 --- a/test/lib/server/middleware/serveResources.js +++ b/test/lib/server/middleware/serveResources.js @@ -45,7 +45,9 @@ test.serial("Check if properties file is served properly", (t) => { }, tree: { resources: { - propertiesFileEncoding: "ISO-8859-1" + configuration: { + propertiesFileEncoding: "ISO-8859-1" + } } } }); @@ -88,7 +90,9 @@ test.serial("Check if properties file is served properly with UTF-8", (t) => { }, tree: { resources: { - propertiesFileEncoding: "UTF-8" + configuration: { + propertiesFileEncoding: "UTF-8" + } } } }); From 9832dd39a6b7d59ea73cab86e66ac7007db0cbc6 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Fri, 26 Jul 2019 11:35:09 +0200 Subject: [PATCH 05/10] Rename encoding property option propertiesFileEncoding -> propertiesSourceFileEncoding --- lib/middleware/serveResources.js | 4 ++-- test/lib/server/middleware/serveResources.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index 14a61bfe..b63d6dfb 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -46,8 +46,8 @@ function createMiddleware({resources, tree: project}) { charset = "UTF-8"; const nonAsciiEscaper = require("@ui5/builder").processors.nonAsciiEscaper; - const propertiesFileEncoding = project && project.resources && project.resources.configuration && project.resources.configuration.propertiesFileEncoding; - const encoding = nonAsciiEscaper.getEncodingFromAlias(propertiesFileEncoding || "ISO-8859-1"); + const propertiesSourceFileEncoding = project && project.resources && project.resources.configuration && project.resources.configuration.propertiesSourceFileEncoding ; + const encoding = nonAsciiEscaper.getEncodingFromAlias(propertiesSourceFileEncoding || "ISO-8859-1"); await nonAsciiEscaper({ resources: [resource], options: { encoding diff --git a/test/lib/server/middleware/serveResources.js b/test/lib/server/middleware/serveResources.js index 181ab41f..fb637d98 100644 --- a/test/lib/server/middleware/serveResources.js +++ b/test/lib/server/middleware/serveResources.js @@ -46,7 +46,7 @@ test.serial("Check if properties file is served properly", (t) => { tree: { resources: { configuration: { - propertiesFileEncoding: "ISO-8859-1" + propertiesSourceFileEncoding : "ISO-8859-1" } } } @@ -91,7 +91,7 @@ test.serial("Check if properties file is served properly with UTF-8", (t) => { tree: { resources: { configuration: { - propertiesFileEncoding: "UTF-8" + propertiesSourceFileEncoding : "UTF-8" } } } From 87a7474d8c8e1719e355a0339da4d6a2b454d0c8 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Fri, 26 Jul 2019 13:46:41 +0200 Subject: [PATCH 06/10] Change property name for encoding propertiesSourceFileEncoding -> propertiesFileSourceEncoding --- lib/middleware/serveResources.js | 4 ++-- test/lib/server/middleware/serveResources.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index b63d6dfb..2b9273f6 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -46,8 +46,8 @@ function createMiddleware({resources, tree: project}) { charset = "UTF-8"; const nonAsciiEscaper = require("@ui5/builder").processors.nonAsciiEscaper; - const propertiesSourceFileEncoding = project && project.resources && project.resources.configuration && project.resources.configuration.propertiesSourceFileEncoding ; - const encoding = nonAsciiEscaper.getEncodingFromAlias(propertiesSourceFileEncoding || "ISO-8859-1"); + 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 diff --git a/test/lib/server/middleware/serveResources.js b/test/lib/server/middleware/serveResources.js index fb637d98..003a53ac 100644 --- a/test/lib/server/middleware/serveResources.js +++ b/test/lib/server/middleware/serveResources.js @@ -46,7 +46,7 @@ test.serial("Check if properties file is served properly", (t) => { tree: { resources: { configuration: { - propertiesSourceFileEncoding : "ISO-8859-1" + propertiesFileSourceEncoding : "ISO-8859-1" } } } @@ -91,7 +91,7 @@ test.serial("Check if properties file is served properly with UTF-8", (t) => { tree: { resources: { configuration: { - propertiesSourceFileEncoding : "UTF-8" + propertiesFileSourceEncoding : "UTF-8" } } } From 12a47a0f4355b87f8340685451dbad1bd7f09859 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Fri, 26 Jul 2019 15:46:16 +0200 Subject: [PATCH 07/10] serve resources now uses normal content-type independent from file type No special logic for properties files regarding charset and type. --- lib/middleware/serveIndex.js | 6 +----- lib/middleware/serveResources.js | 19 +++++-------------- test/lib/server/middleware/serveResources.js | 4 ++-- 3 files changed, 8 insertions(+), 21 deletions(-) diff --git a/lib/middleware/serveIndex.js b/lib/middleware/serveIndex.js index c7c03fc8..511094dc 100644 --- a/lib/middleware/serveIndex.js +++ b/lib/middleware/serveIndex.js @@ -15,11 +15,7 @@ const GB = KB * KB * KB; * @returns {string} mime type */ function getMimeType(resource) { - if (rProperties.test(resource.getPath())) { - return "text/plain;charset=UTF-8"; - } 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 2b9273f6..62e88a15 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -38,29 +38,20 @@ function createMiddleware({resources, tree: project}) { } const resourcePath = resource.getPath(); - let type; - let charset; + const type = mime.lookup(resourcePath) || "application/octet-stream"; + const charset = mime.charset(type); if (rProperties.test(resourcePath)) { // Special handling for *.properties files escape non ascii characters. - type = "text/plain"; - charset = "UTF-8"; - 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"); + 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 } }); - } else { - type = mime.lookup(resourcePath) || "application/octet-stream"; } if (!res.getHeader("Content-Type")) { - if (!charset) { - charset = mime.charset(type); - } - res.setHeader("Content-Type", type + (charset ? "; charset=" + charset : "")); } @@ -80,7 +71,7 @@ function createMiddleware({resources, tree: project}) { // 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 { diff --git a/test/lib/server/middleware/serveResources.js b/test/lib/server/middleware/serveResources.js index 003a53ac..a3de7bf0 100644 --- a/test/lib/server/middleware/serveResources.js +++ b/test/lib/server/middleware/serveResources.js @@ -46,7 +46,7 @@ test.serial("Check if properties file is served properly", (t) => { tree: { resources: { configuration: { - propertiesFileSourceEncoding : "ISO-8859-1" + propertiesFileSourceEncoding: "ISO-8859-1" } } } @@ -91,7 +91,7 @@ test.serial("Check if properties file is served properly with UTF-8", (t) => { tree: { resources: { configuration: { - propertiesFileSourceEncoding : "UTF-8" + propertiesFileSourceEncoding: "UTF-8" } } } From cd6aea65aff957da930a1c909472d1939947bd46 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Fri, 26 Jul 2019 15:54:26 +0200 Subject: [PATCH 08/10] fix tests --- lib/middleware/serveIndex.js | 2 -- test/lib/server/main.js | 6 +++--- test/lib/server/middleware/serveIndex.js | 2 +- test/lib/server/middleware/serveResources.js | 6 +++--- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/middleware/serveIndex.js b/lib/middleware/serveIndex.js index 511094dc..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; diff --git a/test/lib/server/main.js b/test/lib/server/main.js index 73ca6d48..47ea1c4a 100644 --- a/test/lib/server/main.js +++ b/test/lib/server/main.js @@ -73,8 +73,8 @@ test("Get resource from application.a (/i18n/i18n.properties) with correct chars t.fail(res.error.text); } t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); - t.deepEqual(res.headers["content-type"], "text/plain; charset=UTF-8", "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 and charset"); + t.deepEqual(res.body.toString("latin1"), "showHelloButtonText=Say Hello!", "Correct response"); }); }); @@ -86,7 +86,7 @@ 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=UTF-8", + t.deepEqual(res.headers["content-type"], "application/octet-stream", "Correct content type and charset"); t.deepEqual(res.body.toString("utf8"), "showHelloButtonText=Say \\u00e4!", "Correct response"); diff --git a/test/lib/server/middleware/serveIndex.js b/test/lib/server/middleware/serveIndex.js index 357105cc..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=UTF-8<\/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 index a3de7bf0..8ab681fc 100644 --- a/test/lib/server/middleware/serveResources.js +++ b/test/lib/server/middleware/serveResources.js @@ -72,7 +72,7 @@ test.serial("Check if properties file is served properly", (t) => { fame=stra\\u00dfe`); t.is(setHeaderSpy.callCount, 2); t.is(setStringSpy.callCount, 1); - t.is(setHeaderSpy.getCall(0).lastArg, "text/plain; charset=UTF-8"); + t.is(setHeaderSpy.getCall(0).lastArg, "application/octet-stream"); }); }); }); @@ -117,7 +117,7 @@ test.serial("Check if properties file is served properly with UTF-8", (t) => { fame=stra\\ufffde`); t.is(setHeaderSpy.callCount, 2); t.is(setStringSpy.callCount, 1); - t.is(setHeaderSpy.getCall(0).lastArg, "text/plain; charset=UTF-8"); + t.is(setHeaderSpy.getCall(0).lastArg, "application/octet-stream"); }); }); }); @@ -155,7 +155,7 @@ test.serial("Check if properties file is served properly without property settin fame=stra\\u00dfe`); t.is(setHeaderSpy.callCount, 2); t.is(setStringSpy.callCount, 1); - t.is(setHeaderSpy.getCall(0).lastArg, "text/plain; charset=UTF-8"); + t.is(setHeaderSpy.getCall(0).lastArg, "application/octet-stream"); }); }); }); From 04f7f746b923c53a0b36d1b6a2ab57450a4a14ab Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Mon, 29 Jul 2019 11:13:25 +0200 Subject: [PATCH 09/10] Restructure serveResources tests to simplify them Fix test messages in main.js --- test/lib/server/main.js | 12 +-- test/lib/server/middleware/serveResources.js | 95 +++++++++----------- 2 files changed, 47 insertions(+), 60 deletions(-) diff --git a/test/lib/server/main.js b/test/lib/server/main.js index 47ea1c4a..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 'UTF-8'", (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"], "application/octet-stream", "Correct content type and charset"); - t.deepEqual(res.body.toString("latin1"), "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 'UTF-8'", (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) => { @@ -87,9 +87,9 @@ test("Get resource from application.a (/i18n/i18n_de.properties) with correct en } t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); t.deepEqual(res.headers["content-type"], "application/octet-stream", - "Correct content type and charset"); + "Correct content type"); - t.deepEqual(res.body.toString("utf8"), "showHelloButtonText=Say \\u00e4!", "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/serveResources.js b/test/lib/server/middleware/serveResources.js index 8ab681fc..7133869f 100644 --- a/test/lib/server/middleware/serveResources.js +++ b/test/lib/server/middleware/serveResources.js @@ -3,25 +3,21 @@ const sinon = require("sinon"); const resourceFactory = require("@ui5/fs").resourceFactory; const serveResourcesMiddleware = require("../../../../lib/middleware/serveResources"); const writeResource = function(writer, path, size, stringContent) { - const resource = resourceFactory.createResource({path, buffer: Buffer.from(stringContent, "latin1")}); - resource.getStatInfo = function() { - return { - ino: 0, - ctime: new Date(), - mtime: new Date(), - size: size, - isDirectory: function() { - return false; - } - }; - }; - resource.getStream = function() { - return { - pipe: function(res) { - res.end(resource.getString()); - } - }; + 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 native functionality to make serveResources (middleware) run to the end + sinon.stub(resource, "getStream").returns({ + pipe: function() { + } + }); return writer.write(resource).then(() => { return resource; }); @@ -55,18 +51,15 @@ test.serial("Check if properties file is served properly", (t) => { const response = Object.assign({}, fakeResponse); const setHeaderSpy = sinon.spy(response, "setHeader"); - return new Promise((resolve, reject) => { - const req = { - url: "/myFile3.properties", - headers: {} - }; - response.end = function(content) { - content.then(resolve); - }; - const next = function(err) { - reject(new Error(`Next callback called with error: ${err.message}`)); - }; - middleware(req, response, next); + 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`); @@ -100,18 +93,15 @@ test.serial("Check if properties file is served properly with UTF-8", (t) => { const response = Object.assign({}, fakeResponse); const setHeaderSpy = sinon.spy(response, "setHeader"); - return new Promise((resolve, reject) => { - const req = { - url: "/myFile3.properties", - headers: {} - }; - response.end = function(content) { - content.then(resolve); - }; - const next = function(err) { - reject(new Error(`Next callback called with error: ${err.message}`)); - }; - middleware(req, response, next); + 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`); @@ -138,18 +128,15 @@ test.serial("Check if properties file is served properly without property settin const response = Object.assign({}, fakeResponse); const setHeaderSpy = sinon.spy(response, "setHeader"); - return new Promise((resolve, reject) => { - const req = { - url: "/myFile3.properties", - headers: {} - }; - response.end = function(content) { - content.then(resolve); - }; - const next = function(err) { - reject(new Error(`Next callback called with error: ${err.message}`)); - }; - middleware(req, response, next); + 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`); From 62dd62fd1f94ca2df95bd1d95e394645324b4772 Mon Sep 17 00:00:00 2001 From: Tobias Sorn Date: Mon, 29 Jul 2019 13:20:40 +0200 Subject: [PATCH 10/10] Add comment to serveResource test The resource is drained because it is piped in the serveResources middleware. Therefore the getStream and pipe functionality is stubbed. Such that its content can be compared in the test. --- test/lib/server/middleware/serveResources.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/lib/server/middleware/serveResources.js b/test/lib/server/middleware/serveResources.js index 7133869f..542aacc8 100644 --- a/test/lib/server/middleware/serveResources.js +++ b/test/lib/server/middleware/serveResources.js @@ -13,7 +13,7 @@ const writeResource = function(writer, path, size, stringContent) { } }; const resource = resourceFactory.createResource({path, buffer: Buffer.from(stringContent, "latin1"), statInfo}); - // stub resource native functionality to make serveResources (middleware) run to the end + // 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() { } @@ -28,6 +28,11 @@ const fakeResponse = { setHeader: function(string, header) {} }; +test.afterEach.always((t) => { + sinon.restore(); +}); + + test.serial("Check if properties file is served properly", (t) => { t.plan(4); @@ -48,7 +53,7 @@ test.serial("Check if properties file is served properly", (t) => { } }); - const response = Object.assign({}, fakeResponse); + const response = fakeResponse; const setHeaderSpy = sinon.spy(response, "setHeader"); const req = { @@ -90,7 +95,7 @@ test.serial("Check if properties file is served properly with UTF-8", (t) => { } }); - const response = Object.assign({}, fakeResponse); + const response = fakeResponse; const setHeaderSpy = sinon.spy(response, "setHeader"); const req = { @@ -125,7 +130,7 @@ test.serial("Check if properties file is served properly without property settin } }); - const response = Object.assign({}, fakeResponse); + const response = fakeResponse; const setHeaderSpy = sinon.spy(response, "setHeader"); const req = { @@ -133,7 +138,7 @@ test.serial("Check if properties file is served properly without property settin headers: {} }; const next = function(err) { - throw new Error(`Next callback called with error: ${err.message}`); + throw new Error(`Next callback called with error: ${err.stack}`); }; return middleware(req, response, next).then((o) => { return resource.getString();