From 2dc692385320f02541bb59244de24a2fbff5efe0 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 16 Aug 2024 16:33:05 +0200 Subject: [PATCH 1/6] [FIX] manifestEnhancer: Only use valid files for supportedLocales This fixes two problems that could have occurred: A properties file with an invalid locale was still taken into the list of supported locales, which then caused a runtime exception in the ResourceBundle as it validates the input. Another problem was that properties files could have a valid name according to BCP47, but the file won't be ever requested with that name. This is due to the fact that the ResourceBundle does not use the extension / private use sections of the locale when creating the request URL. In both cases, the properties file is now ignored and no entry for the supportedLocales is created. --- lib/processors/manifestEnhancer.js | 44 +++++++++++- test/lib/processors/manifestEnhancer.js | 93 +++++++++++++++++++++++-- 2 files changed, 131 insertions(+), 6 deletions(-) diff --git a/lib/processors/manifestEnhancer.js b/lib/processors/manifestEnhancer.js index 11edb3f8a..c7a130e28 100644 --- a/lib/processors/manifestEnhancer.js +++ b/lib/processors/manifestEnhancer.js @@ -7,6 +7,44 @@ const log = getLogger("builder:processors:manifestEnhancer"); const APP_DESCRIPTOR_V22 = new Version("1.21.0"); +// NOTE: The following regular expression is taken from sap/base/i18n/ResourceBundle and should be kept in sync with it: +// https://github.com/SAP/openui5/blob/22183ac8f3e31e819dd1084f7dbe71ff75b16bb9/src/sap.ui.core/src/sap/base/i18n/ResourceBundle.js +/** + * 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; + +function isValidPropertiesFilenameLocale(locale) { + // The ResourceBundle at runtime only requests locales with an underscore as separator. + // Files that are using a dash as separator will not be loaded and should therefore not be added + // to the supportedLocales. + if (locale.includes("-")) { + return false; + } + // However, the regular expression only allows dashes so the input needs to be converted. + const m = rLocale.exec(locale.replace(/_/g, "-")); + if (!m) { + return false; + } + // Extensions and private use sections are not part of the filename requested by the ResourceBundle + // (see normalize function in sap/base/i18n/ResourceBundle). + // Properties files must therefore not contain these parts. + return !m[5] && !m[6]; +} + function isAbsoluteUrl(url) { if (url.startsWith("/")) { return true; @@ -166,7 +204,11 @@ class ManifestEnhancer { supportedLocales.push(""); } else if (fileNameWithoutExtension.startsWith(i18nBundlePrefix)) { const locale = fileNameWithoutExtension.replace(i18nBundlePrefix, ""); - supportedLocales.push(locale); + if (isValidPropertiesFilenameLocale(locale)) { + supportedLocales.push(locale); + } else { + log.verbose(`Skipping invalid locale '${locale}' for bundle '${i18nBundleUrl}'`); + } } }); return supportedLocales.sort(); diff --git a/test/lib/processors/manifestEnhancer.js b/test/lib/processors/manifestEnhancer.js index fc3416116..586a60822 100644 --- a/test/lib/processors/manifestEnhancer.js +++ b/test/lib/processors/manifestEnhancer.js @@ -3032,20 +3032,88 @@ test("manifestEnhancer#getSupportedLocales", async (t) => { fs.readdir.withArgs("/i18n") .callsArgWith(1, null, [ "i18n.properties", - "i18n_en.properties" + "i18n_en.properties", + "i18n_en_US.properties", + "i18n_en_US_sapprc.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 = [ + "", "en", "en_US", "en_US_sapprc" + ]; + + t.deepEqual(await manifestEnhancer.getSupportedLocales("./i18n/i18n.properties"), 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); t.is(fs.readdir.callCount, 4); + + 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("manifestEnhancer#getSupportedLocales (invalid locales)", 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", + + // Invalid: Should be "zh_CN" + "i18n_zh_CN_.properties", + + // Invalid: Runtime does not include "extension" in file request + "i18n_sr_Latn_RS_variant_f_11.properties", + + // Invalid: Runtime does not include "privateuse" in file request + "i18n_sr_Latn_RS_variant_x_private.properties", + + // Invalid: Runtime does not include "extension" / "privateuse" in file request + "i18n_sr_Latn_RS_variant_f_11_x_private.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.logVerboseSpy.callCount, 5); + t.is(t.context.logVerboseSpy.getCall(0).args[0], + "Skipping invalid locale 'en-US' for bundle 'i18n/i18n.properties'"); + t.is(t.context.logVerboseSpy.getCall(1).args[0], + "Skipping invalid locale 'zh_CN_' for bundle 'i18n/i18n.properties'"); + t.is(t.context.logVerboseSpy.getCall(2).args[0], + "Skipping invalid locale 'sr_Latn_RS_variant_f_11' for bundle 'i18n/i18n.properties'"); + t.is(t.context.logVerboseSpy.getCall(3).args[0], + "Skipping invalid locale 'sr_Latn_RS_variant_x_private' for bundle 'i18n/i18n.properties'"); + t.is(t.context.logVerboseSpy.getCall(4).args[0], + "Skipping invalid locale 'sr_Latn_RS_variant_f_11_x_private' for bundle 'i18n/i18n.properties'"); + 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 (absolute / invalid URLs)", async (t) => { @@ -3076,13 +3144,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 +3199,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) => { From f88a759332e7f5cebb04490a10c73efdf78aad48 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 11 Sep 2024 16:49:22 +0200 Subject: [PATCH 2/6] [INTERNAL] Log warning + prevent duplicate messages --- lib/processors/manifestEnhancer.js | 13 ++++++++++-- test/lib/processors/manifestEnhancer.js | 27 +++++++++++++------------ 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/lib/processors/manifestEnhancer.js b/lib/processors/manifestEnhancer.js index c7a130e28..16eaf523e 100644 --- a/lib/processors/manifestEnhancer.js +++ b/lib/processors/manifestEnhancer.js @@ -170,6 +170,8 @@ class ManifestEnhancer { this.isModified = false; this.runInvoked = false; + + this.supportedLocalesCache = new Map(); } markModified() { @@ -189,7 +191,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); @@ -207,7 +216,7 @@ class ManifestEnhancer { if (isValidPropertiesFilenameLocale(locale)) { supportedLocales.push(locale); } else { - log.verbose(`Skipping invalid locale '${locale}' for bundle '${i18nBundleUrl}'`); + log.warn(`Skipping invalid file '${fileName}' for bundle '${i18nBundleUrl}'`); } } }); diff --git a/test/lib/processors/manifestEnhancer.js b/test/lib/processors/manifestEnhancer.js index 586a60822..8c6d51c39 100644 --- a/test/lib/processors/manifestEnhancer.js +++ b/test/lib/processors/manifestEnhancer.js @@ -3051,7 +3051,8 @@ test("manifestEnhancer#getSupportedLocales", async (t) => { "../../../../../../../../../../../../resources/sap/ui/demo/app/i18n/i18n.properties" ), expectedLocales); - t.is(fs.readdir.callCount, 4); + // 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"); @@ -3101,18 +3102,18 @@ test("manifestEnhancer#getSupportedLocales (invalid locales)", async (t) => { t.is(fs.readdir.callCount, 1); - t.is(t.context.logVerboseSpy.callCount, 5); - t.is(t.context.logVerboseSpy.getCall(0).args[0], - "Skipping invalid locale 'en-US' for bundle 'i18n/i18n.properties'"); - t.is(t.context.logVerboseSpy.getCall(1).args[0], - "Skipping invalid locale 'zh_CN_' for bundle 'i18n/i18n.properties'"); - t.is(t.context.logVerboseSpy.getCall(2).args[0], - "Skipping invalid locale 'sr_Latn_RS_variant_f_11' for bundle 'i18n/i18n.properties'"); - t.is(t.context.logVerboseSpy.getCall(3).args[0], - "Skipping invalid locale 'sr_Latn_RS_variant_x_private' for bundle 'i18n/i18n.properties'"); - t.is(t.context.logVerboseSpy.getCall(4).args[0], - "Skipping invalid locale 'sr_Latn_RS_variant_f_11_x_private' for bundle 'i18n/i18n.properties'"); - t.true(t.context.logWarnSpy.notCalled, "No warnings should be logged"); + t.is(t.context.logWarnSpy.callCount, 5); + 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_sr_Latn_RS_variant_f_11.properties' for bundle 'i18n/i18n.properties'"); + t.is(t.context.logWarnSpy.getCall(3).args[0], + "Skipping invalid file 'i18n_sr_Latn_RS_variant_x_private.properties' for bundle 'i18n/i18n.properties'"); + t.is(t.context.logWarnSpy.getCall(4).args[0], + "Skipping invalid file 'i18n_sr_Latn_RS_variant_f_11_x_private.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"); }); From e3face7e3c54cacd2d34de7f08f714e7cb794372 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 12 Sep 2024 15:03:24 +0200 Subject: [PATCH 3/6] [INTERNAL] Improve locale check and handle specal case of sr_Latn --- lib/processors/manifestEnhancer.js | 78 +++++++++++++------------ test/lib/processors/manifestEnhancer.js | 37 ++++++++---- 2 files changed, 69 insertions(+), 46 deletions(-) diff --git a/lib/processors/manifestEnhancer.js b/lib/processors/manifestEnhancer.js index 16eaf523e..1ff7a20c6 100644 --- a/lib/processors/manifestEnhancer.js +++ b/lib/processors/manifestEnhancer.js @@ -7,42 +7,47 @@ const log = getLogger("builder:processors:manifestEnhancer"); const APP_DESCRIPTOR_V22 = new Version("1.21.0"); -// NOTE: The following regular expression is taken from sap/base/i18n/ResourceBundle and should be kept in sync with it: -// https://github.com/SAP/openui5/blob/22183ac8f3e31e819dd1084f7dbe71ff75b16bb9/src/sap.ui.core/src/sap/base/i18n/ResourceBundle.js -/** - * 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 '-') +/* + * 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. */ -// 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; - -function isValidPropertiesFilenameLocale(locale) { - // The ResourceBundle at runtime only requests locales with an underscore as separator. - // Files that are using a dash as separator will not be loaded and should therefore not be added - // to the supportedLocales. - if (locale.includes("-")) { - return false; +// [ language ] [ region ] [ variants ] +const rLegacyJavaLocale = /^([a-z]{2,3}|sr_Latn)(?:_([A-Z]{2}|\d{3})(?:_([a-zA-Z0-9]+))?)?$/; + +const sapSupportabilityLocales = ["saptrc", "sappsd", "saprigi"]; + +function getBCP47LocaleFromPropertiesFilename(locale) { + const match = rLegacyJavaLocale.exec(locale); + if (!match) { + return null; } - // However, the regular expression only allows dashes so the input needs to be converted. - const m = rLocale.exec(locale.replace(/_/g, "-")); - if (!m) { - return false; + let [, language, region, variants] = match; + let script; + + // Special handling of sr_Latn (see regex above) + // Note: This needs to be in sync with the runtime logic in sap/base/i18n/ResourceBundle + if (language === "sr_Latn") { + language = "sr"; + script = "Latn"; + } + + if (language === "en" && region === "US" && sapSupportabilityLocales.includes(variants)) { + // Convert to private use section (aligned with ResourceBundle behavior) + variants = `x-${variants}`; + } + + let bcp47Locale = language; + if (script) { + bcp47Locale += `-${script}`; + } + if (region) { + bcp47Locale += `-${region}`; + } + if (variants) { + bcp47Locale += `-${variants}`; } - // Extensions and private use sections are not part of the filename requested by the ResourceBundle - // (see normalize function in sap/base/i18n/ResourceBundle). - // Properties files must therefore not contain these parts. - return !m[5] && !m[6]; + return bcp47Locale; } function isAbsoluteUrl(url) { @@ -212,9 +217,10 @@ class ManifestEnhancer { if (fileNameWithoutExtension === i18nBundleName) { supportedLocales.push(""); } else if (fileNameWithoutExtension.startsWith(i18nBundlePrefix)) { - const locale = fileNameWithoutExtension.replace(i18nBundlePrefix, ""); - if (isValidPropertiesFilenameLocale(locale)) { - 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}'`); } diff --git a/test/lib/processors/manifestEnhancer.js b/test/lib/processors/manifestEnhancer.js index 8c6d51c39..4d956a071 100644 --- a/test/lib/processors/manifestEnhancer.js +++ b/test/lib/processors/manifestEnhancer.js @@ -3032,13 +3032,22 @@ test("manifestEnhancer#getSupportedLocales", async (t) => { fs.readdir.withArgs("/i18n") .callsArgWith(1, null, [ "i18n.properties", + "i18n_ar_001.properties", + "i18n_crn.properties", "i18n_en.properties", "i18n_en_US.properties", - "i18n_en_US_sapprc.properties", + "i18n_en_US_saptrc.properties", + "i18n_sr_Latn_RS.properties" ]); const expectedLocales = [ - "", "en", "en_US", "en_US_sapprc" + "", + "ar-001", + "crn", + "en", + "en-US", + "en-US-x-saptrc", + "sr-Latn-RS" ]; t.deepEqual(await manifestEnhancer.getSupportedLocales("./i18n/i18n.properties"), expectedLocales); @@ -3059,7 +3068,7 @@ test("manifestEnhancer#getSupportedLocales", async (t) => { t.true(t.context.logErrorSpy.notCalled, "No errors should be logged"); }); -test("manifestEnhancer#getSupportedLocales (invalid locales)", async (t) => { +test("manifestEnhancer#getSupportedLocales (invalid file names)", async (t) => { const {fs} = t.context; const {ManifestEnhancer} = t.context.__internals__; @@ -3083,35 +3092,43 @@ test("manifestEnhancer#getSupportedLocales (invalid locales)", async (t) => { // Invalid: Should be "zh_CN" "i18n_zh_CN_.properties", - // Invalid: Runtime does not include "extension" in file request + // 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: Runtime does not include "privateuse" in file request + // Invalid: Legacy Java locale format does have a BCP47 "private use" section "i18n_sr_Latn_RS_variant_x_private.properties", - // Invalid: Runtime does not include "extension" / "privateuse" in file request + // Invalid: Legacy Java locale format does have BCP47 "extension" / "private use" sections "i18n_sr_Latn_RS_variant_f_11_x_private.properties" ]; fs.readdir.withArgs("/i18n") .callsArgWith(1, null, fileNames); - const expectedLocales = ["", "en"]; + const expectedLocales = [ + "", + "en" + ]; t.deepEqual(await manifestEnhancer.getSupportedLocales("./i18n/i18n.properties"), expectedLocales); t.is(fs.readdir.callCount, 1); - t.is(t.context.logWarnSpy.callCount, 5); + t.is(t.context.logWarnSpy.callCount, 6); 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_sr_Latn_RS_variant_f_11.properties' for bundle 'i18n/i18n.properties'"); + "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_x_private.properties' for bundle 'i18n/i18n.properties'"); + "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.true(t.context.logVerboseSpy.notCalled, "No verbose messages should be logged"); t.true(t.context.logErrorSpy.notCalled, "No errors should be logged"); From 646061fa8aac8276d7399bea16bee012b418f76e Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 12 Sep 2024 15:08:27 +0200 Subject: [PATCH 4/6] [INTERNAL] Rename sapprc to saptrc sapprc is not a valid supportability locale --- .../build/application.o/dest/Component-preload.js | 4 ++-- .../application.o/dest/i18n/i18n_en_US_sapprc.properties | 1 - .../application.o/dest/i18n/i18n_en_US_saptrc.properties | 1 + test/expected/build/application.o/dest/manifest.json | 8 ++++---- .../webapp/i18n/i18n_en_US_sapprc.properties | 1 - .../webapp/i18n/i18n_en_US_saptrc.properties | 1 + 6 files changed, 8 insertions(+), 8 deletions(-) delete mode 100644 test/expected/build/application.o/dest/i18n/i18n_en_US_sapprc.properties create mode 100644 test/expected/build/application.o/dest/i18n/i18n_en_US_saptrc.properties delete mode 100644 test/fixtures/application.o/webapp/i18n/i18n_en_US_sapprc.properties create mode 100644 test/fixtures/application.o/webapp/i18n/i18n_en_US_saptrc.properties diff --git a/test/expected/build/application.o/dest/Component-preload.js b/test/expected/build/application.o/dest/Component-preload.js index a721ea556..612099beb 100644 --- a/test/expected/build/application.o/dest/Component-preload.js +++ b/test/expected/build/application.o/dest/Component-preload.js @@ -4,7 +4,7 @@ sap.ui.require.preload({ "application/o/i18n/i18n.properties":'welcome=Hello world', "application/o/i18n/i18n_en.properties":'welcome=Hello EN world', "application/o/i18n/i18n_en_US.properties":'welcome=Hello EN US world', - "application/o/i18n/i18n_en_US_sapprc.properties":'welcome=Hello EN US sapprc world', - "application/o/manifest.json":'{"_version":"1.22.0","sap.app":{"id":"application.o","type":"application","applicationVersion":{"version":"1.0.0"},"title":"{{title}}","i18n":{"bundleUrl":"i18n/i18n.properties","supportedLocales":["","en","en_US","en_US_sapprc"]}},"sap.ui5":{"models":{"i18n":{"type":"sap.ui.model.resource.ResourceModel","settings":{"bundleName":"application.o.i18n.i18n","supportedLocales":["","en","en_US","en_US_sapprc"]}},"i18n-ui5":{"type":"sap.ui.model.resource.ResourceModel","settings":{"bundleUrl":"ui5://application/o/i18n/i18n.properties","supportedLocales":["","en","en_US","en_US_sapprc"]}}}}}' + "application/o/i18n/i18n_en_US_saptrc.properties":'welcome=Hello EN US saptrc world', + "application/o/manifest.json":'{"_version":"1.22.0","sap.app":{"id":"application.o","type":"application","applicationVersion":{"version":"1.0.0"},"title":"{{title}}","i18n":{"bundleUrl":"i18n/i18n.properties","supportedLocales":["","en","en_US","en_US_saptrc"]}},"sap.ui5":{"models":{"i18n":{"type":"sap.ui.model.resource.ResourceModel","settings":{"bundleName":"application.o.i18n.i18n","supportedLocales":["","en","en_US","en_US_saptrc"]}},"i18n-ui5":{"type":"sap.ui.model.resource.ResourceModel","settings":{"bundleUrl":"ui5://application/o/i18n/i18n.properties","supportedLocales":["","en","en_US","en_US_saptrc"]}}}}}' }); //# sourceMappingURL=Component-preload.js.map diff --git a/test/expected/build/application.o/dest/i18n/i18n_en_US_sapprc.properties b/test/expected/build/application.o/dest/i18n/i18n_en_US_sapprc.properties deleted file mode 100644 index 2376090d4..000000000 --- a/test/expected/build/application.o/dest/i18n/i18n_en_US_sapprc.properties +++ /dev/null @@ -1 +0,0 @@ -welcome=Hello EN US sapprc world \ No newline at end of file diff --git a/test/expected/build/application.o/dest/i18n/i18n_en_US_saptrc.properties b/test/expected/build/application.o/dest/i18n/i18n_en_US_saptrc.properties new file mode 100644 index 000000000..88ea197de --- /dev/null +++ b/test/expected/build/application.o/dest/i18n/i18n_en_US_saptrc.properties @@ -0,0 +1 @@ +welcome=Hello EN US saptrc world \ No newline at end of file diff --git a/test/expected/build/application.o/dest/manifest.json b/test/expected/build/application.o/dest/manifest.json index a64786ccc..055c3232f 100644 --- a/test/expected/build/application.o/dest/manifest.json +++ b/test/expected/build/application.o/dest/manifest.json @@ -13,7 +13,7 @@ "", "en", "en_US", - "en_US_sapprc" + "en_US_saptrc" ] } }, @@ -27,7 +27,7 @@ "", "en", "en_US", - "en_US_sapprc" + "en_US_saptrc" ] } }, @@ -39,10 +39,10 @@ "", "en", "en_US", - "en_US_sapprc" + "en_US_saptrc" ] } } } } -} \ No newline at end of file +} diff --git a/test/fixtures/application.o/webapp/i18n/i18n_en_US_sapprc.properties b/test/fixtures/application.o/webapp/i18n/i18n_en_US_sapprc.properties deleted file mode 100644 index 2376090d4..000000000 --- a/test/fixtures/application.o/webapp/i18n/i18n_en_US_sapprc.properties +++ /dev/null @@ -1 +0,0 @@ -welcome=Hello EN US sapprc world \ No newline at end of file diff --git a/test/fixtures/application.o/webapp/i18n/i18n_en_US_saptrc.properties b/test/fixtures/application.o/webapp/i18n/i18n_en_US_saptrc.properties new file mode 100644 index 000000000..88ea197de --- /dev/null +++ b/test/fixtures/application.o/webapp/i18n/i18n_en_US_saptrc.properties @@ -0,0 +1 @@ +welcome=Hello EN US saptrc world \ No newline at end of file From 31c02fd438c927a4ae37f716311ec96287a2684b Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 12 Sep 2024 15:58:45 +0200 Subject: [PATCH 5/6] [INTERNAL] Update expected build results --- .../build/application.o/dest/Component-preload.js | 2 +- .../build/application.o/dest/manifest.json | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/expected/build/application.o/dest/Component-preload.js b/test/expected/build/application.o/dest/Component-preload.js index 612099beb..bb7adb35c 100644 --- a/test/expected/build/application.o/dest/Component-preload.js +++ b/test/expected/build/application.o/dest/Component-preload.js @@ -5,6 +5,6 @@ sap.ui.require.preload({ "application/o/i18n/i18n_en.properties":'welcome=Hello EN world', "application/o/i18n/i18n_en_US.properties":'welcome=Hello EN US world', "application/o/i18n/i18n_en_US_saptrc.properties":'welcome=Hello EN US saptrc world', - "application/o/manifest.json":'{"_version":"1.22.0","sap.app":{"id":"application.o","type":"application","applicationVersion":{"version":"1.0.0"},"title":"{{title}}","i18n":{"bundleUrl":"i18n/i18n.properties","supportedLocales":["","en","en_US","en_US_saptrc"]}},"sap.ui5":{"models":{"i18n":{"type":"sap.ui.model.resource.ResourceModel","settings":{"bundleName":"application.o.i18n.i18n","supportedLocales":["","en","en_US","en_US_saptrc"]}},"i18n-ui5":{"type":"sap.ui.model.resource.ResourceModel","settings":{"bundleUrl":"ui5://application/o/i18n/i18n.properties","supportedLocales":["","en","en_US","en_US_saptrc"]}}}}}' + "application/o/manifest.json":'{"_version":"1.22.0","sap.app":{"id":"application.o","type":"application","applicationVersion":{"version":"1.0.0"},"title":"{{title}}","i18n":{"bundleUrl":"i18n/i18n.properties","supportedLocales":["","en","en-US","en-US-x-saptrc"]}},"sap.ui5":{"models":{"i18n":{"type":"sap.ui.model.resource.ResourceModel","settings":{"bundleName":"application.o.i18n.i18n","supportedLocales":["","en","en-US","en-US-x-saptrc"]}},"i18n-ui5":{"type":"sap.ui.model.resource.ResourceModel","settings":{"bundleUrl":"ui5://application/o/i18n/i18n.properties","supportedLocales":["","en","en-US","en-US-x-saptrc"]}}}}}' }); //# sourceMappingURL=Component-preload.js.map diff --git a/test/expected/build/application.o/dest/manifest.json b/test/expected/build/application.o/dest/manifest.json index 055c3232f..8ca743734 100644 --- a/test/expected/build/application.o/dest/manifest.json +++ b/test/expected/build/application.o/dest/manifest.json @@ -12,8 +12,8 @@ "supportedLocales": [ "", "en", - "en_US", - "en_US_saptrc" + "en-US", + "en-US-x-saptrc" ] } }, @@ -26,8 +26,8 @@ "supportedLocales": [ "", "en", - "en_US", - "en_US_saptrc" + "en-US", + "en-US-x-saptrc" ] } }, @@ -38,11 +38,11 @@ "supportedLocales": [ "", "en", - "en_US", - "en_US_saptrc" + "en-US", + "en-US-x-saptrc" ] } } } } -} +} \ No newline at end of file From 2745dba3137d96092f947f8719a8e521592e60fc Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 13 Sep 2024 13:42:59 +0200 Subject: [PATCH 6/6] [INTERNAL] Validate variants and generated locales against BCP47 --- lib/processors/manifestEnhancer.js | 23 +++++--- test/lib/processors/manifestEnhancer.js | 72 +++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 12 deletions(-) diff --git a/lib/processors/manifestEnhancer.js b/lib/processors/manifestEnhancer.js index 1ff7a20c6..0b319301a 100644 --- a/lib/processors/manifestEnhancer.js +++ b/lib/processors/manifestEnhancer.js @@ -11,11 +11,14 @@ 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})(?:_([a-zA-Z0-9]+))?)?$/; +// [ 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})*)?)?$/; -const sapSupportabilityLocales = ["saptrc", "sappsd", "saprigi"]; +// 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); @@ -25,15 +28,20 @@ function getBCP47LocaleFromPropertiesFilename(locale) { 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 in sap/base/i18n/ResourceBundle + // 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" && sapSupportabilityLocales.includes(variants)) { - // Convert to private use section (aligned with ResourceBundle behavior) + 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}`; } @@ -45,7 +53,8 @@ function getBCP47LocaleFromPropertiesFilename(locale) { bcp47Locale += `-${region}`; } if (variants) { - bcp47Locale += `-${variants}`; + // Convert to BCP47 variant format + bcp47Locale += `-${variants.replace(/_/g, "-")}`; } return bcp47Locale; } diff --git a/test/lib/processors/manifestEnhancer.js b/test/lib/processors/manifestEnhancer.js index 4d956a071..be27ff73e 100644 --- a/test/lib/processors/manifestEnhancer.js +++ b/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(); @@ -3033,24 +3061,32 @@ test("manifestEnhancer#getSupportedLocales", async (t) => { .callsArgWith(1, null, [ "i18n.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_sr_Latn_RS.properties" + "i18n_en_US_saprigi.properties", + "i18n_sr_Latn_RS.properties", + "i18n_sr_Latn_RS_variant.properties" ]); 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", + "sr-Latn-RS-variant", ]; - t.deepEqual(await manifestEnhancer.getSupportedLocales("./i18n/i18n.properties"), expectedLocales); + 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); @@ -3066,6 +3102,12 @@ test("manifestEnhancer#getSupportedLocales", async (t) => { 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) => { @@ -3102,7 +3144,19 @@ test("manifestEnhancer#getSupportedLocales (invalid file names)", async (t) => { "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" + "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") @@ -3117,7 +3171,7 @@ test("manifestEnhancer#getSupportedLocales (invalid file names)", async (t) => { t.is(fs.readdir.callCount, 1); - t.is(t.context.logWarnSpy.callCount, 6); + 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], @@ -3130,6 +3184,14 @@ test("manifestEnhancer#getSupportedLocales (invalid file names)", async (t) => { "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"); });