From a5ca9128bb5373eb7ebba838b82d7c7820afcb8a Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Tue, 18 Jun 2019 20:27:45 +0200 Subject: [PATCH 1/2] [FIX] serveResources: Correctly encode non UTF-8 resources The replaceStream module converts all string it processes to UTF-8. Therefore, stop using it for strings that are not UTF-8 encoded. Resolves https://github.com/SAP/ui5-server/issues/196 --- lib/middleware/serveResources.js | 2 +- .../webapp/i18n/i18n_de.properties | 1 + test/fixtures/application.a/webapp/index.html | 4 ++-- test/lib/server/acceptRemoteConnections.js | 2 +- test/lib/server/h2.js | 2 +- test/lib/server/main.js | 23 ++++++++++++++++--- 6 files changed, 26 insertions(+), 8 deletions(-) create mode 100644 test/fixtures/application.a/webapp/i18n/i18n_de.properties diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index cb03b5ae..cf0ec320 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -63,7 +63,7 @@ function createMiddleware({resourceCollections}) { let stream = resource.getStream(); - if ((type.startsWith("text/") || type === "application/javascript")) { + if (charset === "UTF-8" && (type.startsWith("text/") || type === "application/javascript")) { if (resource._project) { stream = stream.pipe(replaceStream("${version}", resource._project.version)); } else { diff --git a/test/fixtures/application.a/webapp/i18n/i18n_de.properties b/test/fixtures/application.a/webapp/i18n/i18n_de.properties new file mode 100644 index 00000000..dc888eff --- /dev/null +++ b/test/fixtures/application.a/webapp/i18n/i18n_de.properties @@ -0,0 +1 @@ +showHelloButtonText=Say �! \ No newline at end of file diff --git a/test/fixtures/application.a/webapp/index.html b/test/fixtures/application.a/webapp/index.html index 77b0207c..3d8715b9 100644 --- a/test/fixtures/application.a/webapp/index.html +++ b/test/fixtures/application.a/webapp/index.html @@ -1,9 +1,9 @@ - Application A + Application A - Version ${version} - \ No newline at end of file + diff --git a/test/lib/server/acceptRemoteConnections.js b/test/lib/server/acceptRemoteConnections.js index 1565ed6d..c5a6e6f5 100644 --- a/test/lib/server/acceptRemoteConnections.js +++ b/test/lib/server/acceptRemoteConnections.js @@ -41,6 +41,6 @@ test("Get resource from application.a (/index.html) with enabled remote connecti } t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); t.regex(res.headers["content-type"], /html/, "Correct content type"); - t.regex(res.text, /Application A<\/title>/, "Correct response"); + t.regex(res.text, /<title>Application A - Version 1.0.0<\/title>/, "Correct response"); }); }); diff --git a/test/lib/server/h2.js b/test/lib/server/h2.js index 45715bd6..1d016b0a 100644 --- a/test/lib/server/h2.js +++ b/test/lib/server/h2.js @@ -54,6 +54,6 @@ test("Get resource from application.a (/index.html)", (t) => { } t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); t.regex(res.headers["content-type"], /html/, "Correct content type"); - t.regex(res.text, /<title>Application A<\/title>/, "Correct response"); + t.regex(res.text, /<title>Application A - Version 1.0.0<\/title>/, "Correct response"); }); }); diff --git a/test/lib/server/main.js b/test/lib/server/main.js index da710209..dd7d9283 100644 --- a/test/lib/server/main.js +++ b/test/lib/server/main.js @@ -21,7 +21,7 @@ test.before((t) => { }); }); -test.after(() => { +test.after.always(() => { return new Promise((resolve, reject) => { serve.close((error) => { if (error) { @@ -40,7 +40,7 @@ test("Get resource from application.a (/index.html)", (t) => { } t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); t.regex(res.headers["content-type"], /html/, "Correct content type"); - t.regex(res.text, /<title>Application A<\/title>/, "Correct response"); + t.regex(res.text, /<title>Application A - Version 1.0.0<\/title>/, "Correct response"); }); }); @@ -51,10 +51,27 @@ test("Get resource from application.a (/i18n/i18n.properties) with correct chars } 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.text, "showHelloButtonText=Say Hello!", "Correct response"); + 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) => { + 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.body.toString("latin1"), "showHelloButtonText=Say ä!", "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("Get resource from library.a (/resources/library/a/.library)", (t) => { return request.get("/resources/library/a/.library").then((res) => { From a66337d8483fb62b68a45cd0041dfb2a7130dc29 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger <m.beutlberger@sap.com> Date: Wed, 19 Jun 2019 12:42:51 +0200 Subject: [PATCH 2/2] [INTERNAL] Only replace versions in files the build would process as well --- lib/middleware/serveResources.js | 14 ++++++--- test/fixtures/application.a/webapp/index.html | 2 +- .../application.a/webapp/versionTest.html | 9 ++++++ .../application.a/webapp/versionTest.js | 1 + test/lib/server/acceptRemoteConnections.js | 2 +- test/lib/server/h2.js | 2 +- test/lib/server/main.js | 30 +++++++++++++++++-- 7 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 test/fixtures/application.a/webapp/versionTest.html create mode 100644 test/fixtures/application.a/webapp/versionTest.js diff --git a/lib/middleware/serveResources.js b/lib/middleware/serveResources.js index cf0ec320..5976d92b 100644 --- a/lib/middleware/serveResources.js +++ b/lib/middleware/serveResources.js @@ -6,7 +6,8 @@ const etag = require("etag"); const fresh = require("fresh"); const parseurl = require("parseurl"); -const rProperties = /\.properties$/; +const rProperties = /\.properties$/i; +const rReplaceVersion = /\.(library|js|json)$/i; function isFresh(req, res) { return fresh(req.headers, { @@ -35,12 +36,13 @@ function createMiddleware({resourceCollections}) { let type; let charset; - if (rProperties.test(resource.getPath())) { + const resourcePath = resource.getPath(); + 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(resource.getPath()) || "application/octet-stream"; + type = mime.lookup(resourcePath) || "application/octet-stream"; } if (!res.getHeader("Content-Type")) { @@ -63,7 +65,11 @@ function createMiddleware({resourceCollections}) { let stream = resource.getStream(); - if (charset === "UTF-8" && (type.startsWith("text/") || type === "application/javascript")) { + // Only execute version replacement for UTF-8 encoded resources because replaceStream will always output + // 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 (resource._project) { stream = stream.pipe(replaceStream("${version}", resource._project.version)); } else { diff --git a/test/fixtures/application.a/webapp/index.html b/test/fixtures/application.a/webapp/index.html index 3d8715b9..1b875590 100644 --- a/test/fixtures/application.a/webapp/index.html +++ b/test/fixtures/application.a/webapp/index.html @@ -1,7 +1,7 @@ <!DOCTYPE html> <html> <head> - <title>Application A - Version ${version} + Application A diff --git a/test/fixtures/application.a/webapp/versionTest.html b/test/fixtures/application.a/webapp/versionTest.html new file mode 100644 index 00000000..f6b13e97 --- /dev/null +++ b/test/fixtures/application.a/webapp/versionTest.html @@ -0,0 +1,9 @@ + + + + Not replaced: ${version} + + + + + diff --git a/test/fixtures/application.a/webapp/versionTest.js b/test/fixtures/application.a/webapp/versionTest.js new file mode 100644 index 00000000..edceeba4 --- /dev/null +++ b/test/fixtures/application.a/webapp/versionTest.js @@ -0,0 +1 @@ +console.log(`${version}`); diff --git a/test/lib/server/acceptRemoteConnections.js b/test/lib/server/acceptRemoteConnections.js index c5a6e6f5..1565ed6d 100644 --- a/test/lib/server/acceptRemoteConnections.js +++ b/test/lib/server/acceptRemoteConnections.js @@ -41,6 +41,6 @@ test("Get resource from application.a (/index.html) with enabled remote connecti } t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); t.regex(res.headers["content-type"], /html/, "Correct content type"); - t.regex(res.text, /Application A - Version 1.0.0<\/title>/, "Correct response"); + t.regex(res.text, /<title>Application A<\/title>/, "Correct response"); }); }); diff --git a/test/lib/server/h2.js b/test/lib/server/h2.js index 1d016b0a..45715bd6 100644 --- a/test/lib/server/h2.js +++ b/test/lib/server/h2.js @@ -54,6 +54,6 @@ test("Get resource from application.a (/index.html)", (t) => { } t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); t.regex(res.headers["content-type"], /html/, "Correct content type"); - t.regex(res.text, /<title>Application A - Version 1.0.0<\/title>/, "Correct response"); + t.regex(res.text, /<title>Application A<\/title>/, "Correct response"); }); }); diff --git a/test/lib/server/main.js b/test/lib/server/main.js index dd7d9283..1baad208 100644 --- a/test/lib/server/main.js +++ b/test/lib/server/main.js @@ -40,7 +40,30 @@ test("Get resource from application.a (/index.html)", (t) => { } t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); t.regex(res.headers["content-type"], /html/, "Correct content type"); - t.regex(res.text, /<title>Application A - Version 1.0.0<\/title>/, "Correct response"); + t.regex(res.text, /<title>Application A<\/title>/, "Correct response"); + }); +}); + + +test("Get resource from application.a with not replaced version placeholder(/versionTest.html)", (t) => { + return request.get("/versionTest.html").then((res) => { + if (res.error) { + t.fail(res.error.text); + } + t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); + t.regex(res.headers["content-type"], /html/, "Correct content type"); + t.regex(res.text, /<title>Not replaced: \${version}<\/title>/, "Correct response"); + }); +}); + +test("Get resource from application.a with replaced version placeholder (/versionTest.js)", (t) => { + return request.get("/versionTest.js").then((res) => { + if (res.error) { + t.fail(res.error.text); + } + t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); + t.regex(res.headers["content-type"], /application\/javascript/, "Correct content type"); + t.deepEqual(res.text, "console.log(`1.0.0`);\n", "Correct response"); }); }); @@ -119,6 +142,9 @@ test("Get app_pages from discovery middleware (/discovery/app_pages)", (t) => { "app_pages": [ { "entry": "index.html" + }, + { + "entry": "versionTest.html" } ] }, "Correct response"); @@ -480,7 +506,7 @@ test("Get index of resources", (t) => { t.deepEqual(res.statusCode, 200, "Correct HTTP status code"); t.is(res.headers["content-type"], "text/html", "Correct content type"); t.is(/<title>(.*)<\/title>/i.exec(res.text)[1], "Index of /", "Found correct title"); - t.deepEqual(res.text.match(/<td/g).length, 30, "Found correct amount of <td> elements"); + t.deepEqual(res.text.match(/<td/g).length, 42, "Found correct amount of <td> elements"); }), request.get("/resources").then((res) => { t.deepEqual(res.statusCode, 200, "Correct HTTP status code");