[FEATURE] Properties File Escaping (#214)

In serveResources middleware use nonAsciiEscaper processor to escape .properties files.

Author: tobiasso85

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",

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";
 }
 
 /**

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);
-		});
+		}
 	};
 }
 

test/lib/server/main.js

@@ -67,29 +67,29 @@ 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) => {
 			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"], "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");

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, /<td title="1024 Bytes">1\.00 KB<\/td>\s*<td><a href="\/myFile1\.meh">myFile1\.meh<\/a><\/td>\s*<td>application\/octet-stream<\/td>/);
 					t.regex(content, /<td title="1048576 Bytes">1\.00 MB<\/td>\s*<td><a href="\/myFile2\.js">myFile2\.js<\/a><\/td>\s*<td>application\/javascript<\/td>/g);
-					t.regex(content, /<td title="1073741824 Bytes">1\.00 GB<\/td>\s*<td><a href="\/myFile3\.properties">myFile3\.properties<\/a><\/td>\s*<td>text\/plain;charset=ISO-8859-1<\/td>/g);
+					t.regex(content, /<td title="1073741824 Bytes">1\.00 GB<\/td>\s*<td><a href="\/myFile3\.properties">myFile3\.properties<\/a><\/td>\s*<td>application\/octet-stream<\/td>/g);
 					resolve();
 				},
 			};

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");
+		});
+	});
+});