[FIX] manifestEnhancer: Only use valid files for supportedLocales

lib/processors/manifestEnhancer.js

@@ -7,6 +7,58 @@ const log = getLogger("builder:processors:manifestEnhancer");
 
 const APP_DESCRIPTOR_V22 = new Version("1.21.0");
 
+/*
+ * Matches a legacy Java locale string, which is the format used by the UI5 Runtime (ResourceBundle)
+ * to load i18n properties files.
+ * Special case: "sr_Latn" is also supported, although the BCP47 script part is not supported by the Java locale format.
+ *
+ * Variants are limited to the format from BCP47, but with underscores instead of hyphens.
+ */
+//                          [     language     ]    [    region    ][                variants               ]
+const rLegacyJavaLocale = /^([a-z]{2,3}|sr_Latn)(?:_([A-Z]{2}|\d{3})((?:_[0-9a-zA-Z]{5,8}|_[0-9][0-9a-zA-Z]{3})*)?)?$/;
+
+// See https://github.com/SAP/openui5/blob/a8f36e430f1fac172eb705811da4a2af25483408/src/sap.ui.core/src/sap/base/i18n/ResourceBundle.js#L76
+const sapSupportabilityVariants = ["saptrc", "sappsd", "saprigi"];
+
+function getBCP47LocaleFromPropertiesFilename(locale) {
+	const match = rLegacyJavaLocale.exec(locale);
+	if (!match) {
+		return null;
+	}
+	let [, language, region, variants] = match;
+	let script;
+
+	variants = variants?.slice(1); // Remove leading underscore
+
+	// Special handling of sr_Latn (see regex above)
+	// Note: This needs to be in sync with the runtime logic:
+	// https://github.com/SAP/openui5/blob/a8f36e430f1fac172eb705811da4a2af25483408/src/sap.ui.core/src/sap/base/i18n/ResourceBundle.js#L202
+	if (language === "sr_Latn") {
+		language = "sr";
+		script = "Latn";
+	}
+
+	if (language === "en" && region === "US" && sapSupportabilityVariants.includes(variants)) {
+		// Convert to private use section
+		// Note: This needs to be in sync with the runtime logic:
+		// https://github.com/SAP/openui5/blob/a8f36e430f1fac172eb705811da4a2af25483408/src/sap.ui.core/src/sap/base/i18n/ResourceBundle.js#L190
+		variants = `x-${variants}`;
+	}
+
+	let bcp47Locale = language;
+	if (script) {
+		bcp47Locale += `-${script}`;
+	}
+	if (region) {
+		bcp47Locale += `-${region}`;
+	}
+	if (variants) {
+		// Convert to BCP47 variant format
+		bcp47Locale += `-${variants.replace(/_/g, "-")}`;
+	}
+	return bcp47Locale;
+}
+
 function isAbsoluteUrl(url) {
 	if (url.startsWith("/")) {
 		return true;
@@ -132,6 +184,8 @@ class ManifestEnhancer {
 
 		this.isModified = false;
 		this.runInvoked = false;
+
+		this.supportedLocalesCache = new Map();
 	}
 
 	markModified() {
@@ -151,7 +205,14 @@ class ManifestEnhancer {
 		}
 	}
 
-	async findSupportedLocales(i18nBundleUrl) {
+	findSupportedLocales(i18nBundleUrl) {
+		if (!this.supportedLocalesCache.has(i18nBundleUrl)) {
+			this.supportedLocalesCache.set(i18nBundleUrl, this._findSupportedLocales(i18nBundleUrl));
+		}
+		return this.supportedLocalesCache.get(i18nBundleUrl);
+	}
+
+	async _findSupportedLocales(i18nBundleUrl) {
 		const i18nBundleName = path.basename(i18nBundleUrl, ".properties");
 		const i18nBundlePrefix = `${i18nBundleName}_`;
 		const i18nBundleDir = path.dirname(i18nBundleUrl);
@@ -165,8 +226,13 @@ class ManifestEnhancer {
 			if (fileNameWithoutExtension === i18nBundleName) {
 				supportedLocales.push("");
 			} else if (fileNameWithoutExtension.startsWith(i18nBundlePrefix)) {
-				const locale = fileNameWithoutExtension.replace(i18nBundlePrefix, "");
-				supportedLocales.push(locale);
+				const fileNameLocale = fileNameWithoutExtension.replace(i18nBundlePrefix, "");
+				const bcp47Locale = getBCP47LocaleFromPropertiesFilename(fileNameLocale);
+				if (bcp47Locale) {
+					supportedLocales.push(bcp47Locale);
+				} else {
+					log.warn(`Skipping invalid file '${fileName}' for bundle '${i18nBundleUrl}'`);
+				}
 			}
 		});
 		return supportedLocales.sort();

test/expected/build/application.o/dest/i18n/i18n_en_US_saptrc.properties

@@ -0,0 +1 @@
+welcome=Hello EN US saptrc world

test/expected/build/application.o/dest/manifest.json

@@ -12,8 +12,8 @@
       "supportedLocales": [
         "",
         "en",
-        "en_US",
-        "en_US_sapprc"
+        "en-US",
+        "en-US-x-saptrc"
       ]
     }
   },
@@ -26,8 +26,8 @@
           "supportedLocales": [
             "",
             "en",
-            "en_US",
-            "en_US_sapprc"
+            "en-US",
+            "en-US-x-saptrc"
           ]
         }
       },
@@ -38,8 +38,8 @@
           "supportedLocales": [
             "",
             "en",
-            "en_US",
-            "en_US_sapprc"
+            "en-US",
+            "en-US-x-saptrc"
           ]
         }
       }

test/fixtures/application.o/webapp/i18n/i18n_en_US_saptrc.properties

@@ -0,0 +1 @@
+welcome=Hello EN US saptrc world

test/lib/processors/manifestEnhancer.js

@@ -2,6 +2,34 @@ import test from "ava";
 import sinonGlobal from "sinon";
 import esmock from "esmock";
 
+function isValidBCP47Locale(locale) {
+	if (locale === "") {
+		// Special handling of empty string, as this marks the developer locale (without locale information)
+		return true;
+	}
+
+	// See https://github.com/SAP/openui5/blob/a8f36e430f1fac172eb705811da4a2af25483408/src/sap.ui.core/src/sap/base/i18n/ResourceBundle.js#L30
+	/**
+	 * A regular expression that describes language tags according to BCP-47.
+	 *
+	 * @see BCP47 "Tags for Identifying Languages" (http://www.ietf.org/rfc/bcp/bcp47.txt)
+	 *
+	 * The matching groups are
+	 *  0=all
+	 *  1=language (shortest ISO639 code + ext. language sub tags | 4digits (reserved) | registered language sub tags)
+	 *  2=script (4 letters)
+	 *  3=region (2letter language or 3 digits)
+	 *  4=variants (separated by '-', Note: capturing group contains leading '-' to shorten the regex!)
+	 *  5=extensions (including leading singleton, multiple extensions separated by '-')
+	 *  6=private use section (including leading 'x', multiple sections separated by '-')
+	 */
+
+	// eslint-disable-next-line max-len
+	//                [-------------------- language ----------------------][--- script ---][------- region --------][------------- variants --------------][----------- extensions ------------][------ private use -------]
+	const rLocale = /^((?:[A-Z]{2,3}(?:-[A-Z]{3}){0,3})|[A-Z]{4}|[A-Z]{5,8})(?:-([A-Z]{4}))?(?:-([A-Z]{2}|[0-9]{3}))?((?:-[0-9A-Z]{5,8}|-[0-9][0-9A-Z]{3})*)((?:-[0-9A-WYZ](?:-[0-9A-Z]{2,8})+)*)(?:-(X(?:-[0-9A-Z]{1,8})+))?$/i;
+	return rLocale.test(locale);
+}
+
 test.beforeEach(async (t) => {
 	const sinon = t.context.sinon = sinonGlobal.createSandbox();
 	t.context.logWarnSpy = sinon.spy();
@@ -3032,20 +3060,140 @@ test("manifestEnhancer#getSupportedLocales", async (t) => {
 	fs.readdir.withArgs("/i18n")
 		.callsArgWith(1, null, [
 			"i18n.properties",
-			"i18n_en.properties"
+			"i18n_ar_001.properties",
+			"i18n_ar_001_variant.properties",
+			"i18n_crn.properties",
+			"i18n_en.properties",
+			"i18n_en_US.properties",
+			"i18n_en_US_saptrc.properties",
+			"i18n_en_US_saprigi.properties",
+			"i18n_sr_Latn_RS.properties",
+			"i18n_sr_Latn_RS_variant.properties"
 		]);
 
-	t.deepEqual(await manifestEnhancer.getSupportedLocales("./i18n/i18n.properties"), ["", "en"]);
-	t.deepEqual(await manifestEnhancer.getSupportedLocales("i18n/../i18n/i18n.properties"), ["", "en"]);
-	t.deepEqual(await manifestEnhancer.getSupportedLocales("ui5://sap/ui/demo/app/i18n/i18n.properties"), ["", "en"]);
+	const expectedLocales = [
+		"",
+		"ar-001",
+		"ar-001-variant",
+		"crn",
+		"en",
+		"en-US",
+		"en-US-x-saprigi",
+		"en-US-x-saptrc",
+		"sr-Latn-RS",
+		"sr-Latn-RS-variant",
+	];
+
+	const generatedLocales = await manifestEnhancer.getSupportedLocales("./i18n/i18n.properties");
+
+	t.deepEqual(generatedLocales, expectedLocales);
+	t.deepEqual(await manifestEnhancer.getSupportedLocales("i18n/../i18n/i18n.properties"), expectedLocales);
+	t.deepEqual(await manifestEnhancer.getSupportedLocales("ui5://sap/ui/demo/app/i18n/i18n.properties"), expectedLocales);
 
 	// Path traversal to root and then into application namespace
 	// This works, but is not recommended at all! It also likely fails at runtime
 	t.deepEqual(await manifestEnhancer.getSupportedLocales(
 		"../../../../../../../../../../../../resources/sap/ui/demo/app/i18n/i18n.properties"
-	), ["", "en"]);
+	), expectedLocales);
+
+	// findSupportedLocales caches requests, so for the same bundle readdir is only called once
+	t.is(fs.readdir.callCount, 1);
+
+	t.true(t.context.logVerboseSpy.notCalled, "No verbose messages should be logged");
+	t.true(t.context.logWarnSpy.notCalled, "No warnings should be logged");
+	t.true(t.context.logErrorSpy.notCalled, "No errors should be logged");
+
+	// Check whether generated locales are valid BCP47 locales, as the UI5 runtime
+	// fails if a locale is not a valid
+	generatedLocales.forEach((locale) => {
+		t.true(isValidBCP47Locale(locale), `Generated locale '${locale}' should be a valid BCP47 locale`);
+	});
+});
+
+test("manifestEnhancer#getSupportedLocales (invalid file names)", async (t) => {
+	const {fs} = t.context;
+	const {ManifestEnhancer} = t.context.__internals__;
+
+	const manifest = JSON.stringify({
+		"_version": "1.58.0",
+		"sap.app": {
+			"id": "sap.ui.demo.app"
+		}
+	});
+	const filePath = "/manifest.json";
+
+	const manifestEnhancer = new ManifestEnhancer(manifest, filePath, fs);
+
+	const fileNames = [
+		"i18n.properties",
+		"i18n_en.properties",
+
+		// Invalid: Should be "en_US"
+		"i18n_en-US.properties",
 
-	t.is(fs.readdir.callCount, 4);
+		// Invalid: Should be "zh_CN"
+		"i18n_zh_CN_.properties",
+
+		// Invalid: Script section is only supported for "sr_Latn"
+		"i18n_en_Latn_US.properties",
+
+		// Invalid: Legacy Java locale format does have a BCP47 "extension" section
+		"i18n_sr_Latn_RS_variant_f_11.properties",
+
+		// Invalid: Legacy Java locale format does have a BCP47 "private use" section
+		"i18n_sr_Latn_RS_variant_x_private.properties",
+
+		// Invalid: Legacy Java locale format does have BCP47 "extension" / "private use" sections
+		"i18n_sr_Latn_RS_variant_f_11_x_private.properties",
+
+		// Invalid: Invalid variant length (too short)
+		"i18n_de_CH_var.properties",
+
+		// Invalid: Invalid variant length (too long)
+		"i18n_de_CH_variant11.properties",
+
+		// Invalid: Invalid variant length (too long)
+		"i18n_de_CH_001FOOBAR.properties",
+
+		// Invalid: Should be "en_US_saprigi"
+		"i18n_en_US_x_saprigi.properties"
+	];
+
+	fs.readdir.withArgs("/i18n")
+		.callsArgWith(1, null, fileNames);
+
+	const expectedLocales = [
+		"",
+		"en"
+	];
+
+	t.deepEqual(await manifestEnhancer.getSupportedLocales("./i18n/i18n.properties"), expectedLocales);
+
+	t.is(fs.readdir.callCount, 1);
+
+	t.is(t.context.logWarnSpy.callCount, 10);
+	t.is(t.context.logWarnSpy.getCall(0).args[0],
+		"Skipping invalid file 'i18n_en-US.properties' for bundle 'i18n/i18n.properties'");
+	t.is(t.context.logWarnSpy.getCall(1).args[0],
+		"Skipping invalid file 'i18n_zh_CN_.properties' for bundle 'i18n/i18n.properties'");
+	t.is(t.context.logWarnSpy.getCall(2).args[0],
+		"Skipping invalid file 'i18n_en_Latn_US.properties' for bundle 'i18n/i18n.properties'");
+	t.is(t.context.logWarnSpy.getCall(3).args[0],
+		"Skipping invalid file 'i18n_sr_Latn_RS_variant_f_11.properties' for bundle 'i18n/i18n.properties'");
+	t.is(t.context.logWarnSpy.getCall(4).args[0],
+		"Skipping invalid file 'i18n_sr_Latn_RS_variant_x_private.properties' for bundle 'i18n/i18n.properties'");
+	t.is(t.context.logWarnSpy.getCall(5).args[0],
+		"Skipping invalid file 'i18n_sr_Latn_RS_variant_f_11_x_private.properties' for bundle 'i18n/i18n.properties'");
+	t.is(t.context.logWarnSpy.getCall(6).args[0],
+		"Skipping invalid file 'i18n_de_CH_var.properties' for bundle 'i18n/i18n.properties'");
+	t.is(t.context.logWarnSpy.getCall(7).args[0],
+		"Skipping invalid file 'i18n_de_CH_variant11.properties' for bundle 'i18n/i18n.properties'");
+	t.is(t.context.logWarnSpy.getCall(8).args[0],
+		"Skipping invalid file 'i18n_de_CH_001FOOBAR.properties' for bundle 'i18n/i18n.properties'");
+	t.is(t.context.logWarnSpy.getCall(9).args[0],
+		"Skipping invalid file 'i18n_en_US_x_saprigi.properties' for bundle 'i18n/i18n.properties'");
+	t.true(t.context.logVerboseSpy.notCalled, "No verbose messages should be logged");
+	t.true(t.context.logErrorSpy.notCalled, "No errors should be logged");
 });
 
 test("manifestEnhancer#getSupportedLocales (absolute / invalid URLs)", async (t) => {
@@ -3076,13 +3224,24 @@ test("manifestEnhancer#getSupportedLocales (absolute / invalid URLs)", async (t)
 	t.deepEqual(await manifestEnhancer.getSupportedLocales("sftp:i18n.properties"), []);
 	t.deepEqual(await manifestEnhancer.getSupportedLocales("file://i18n.properties"), []);
 
+	t.is(t.context.logVerboseSpy.callCount, 0, "No verbose messages should be logged");
+
 	// Path traversal to root
 	t.deepEqual(await manifestEnhancer.getSupportedLocales("../../../../../../../../../../../../i18n.properties"), []);
 
+	t.is(t.context.logVerboseSpy.callCount, 1, "One verbose message should be logged from previous call");
+	t.is(t.context.logVerboseSpy.getCall(0).args[0],
+		"/manifest.json: bundleUrl '../../../../../../../../../../../../i18n.properties' " +
+		"points to a bundle outside of the current namespace 'sap.ui.demo.app', enhancement of " +
+		"'supportedLocales' is skipped");
+
 	// Relative ui5-protocol URL
 	t.deepEqual(await manifestEnhancer.getSupportedLocales("ui5:i18n.properties"), []);
 
 	t.is(fs.readdir.callCount, 0, "readdir should not be called for any absolute / invalid URL");
+	t.is(t.context.logVerboseSpy.callCount, 1, "No additional verbose messages should be logged");
+	t.true(t.context.logWarnSpy.notCalled, "No warnings should be logged");
+	t.true(t.context.logErrorSpy.notCalled, "No errors should be logged");
 });
 
 test("manifestEnhancer#getSupportedLocales (error handling)", async (t) => {
@@ -3120,6 +3279,10 @@ test("manifestEnhancer#getSupportedLocales (error handling)", async (t) => {
 	});
 
 	t.is(fs.readdir.callCount, 2, "readdir should be called once");
+
+	t.true(t.context.logVerboseSpy.notCalled, "No verbose messages should be logged");
+	t.true(t.context.logWarnSpy.notCalled, "No warnings should be logged");
+	t.true(t.context.logErrorSpy.notCalled, "No errors should be logged");
 });
 
 test("getRelativeBundleUrlFromName", (t) => {