Validate l10n ID is non-empty to prevent crash on empty string ID

CHANGELOG.md

@@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [L10NSharp] Removed emailForSubmissions parameter from LocalizationManager.Create. Since the localization dialog was jettisoned, it no longer makes sense to store this information on the localization manager.
 - [L10NSharp.Windows.Forms] Removed emailForSubmissions parameter (8th parameter) from LocalizationManagerWinforms.Create. Since the localization dialog was jettisoned, it no longer makes sense to store this information on the localization manager.
 - [L10NSharp] Replaced the .NET 8.0 target with .NET Standard 2.0 for broader compatibility.
+- [L10NSharp] BREAKING CHANGE: `LocalizationManager.GetString`, `GetDynamicString`, and `GetDynamicStringOrEnglish` now throw `ArgumentException` when called with a null or empty string ID. Previously, empty IDs were silently accepted and could produce a malformed XLIFF file that crashed on next launch.
 
 ### Fixed
 

src/L10NSharp.Tests/LocalizationManagerTestsBase.cs

@@ -289,6 +289,66 @@ public void GetDynamicStringInEnglish_NoDefault_FindsEnglish()
 			}
 		}
 
+		[TestCase(null)]
+		[TestCase("")]
+		[TestCase(" ")]
+		public void GetDynamicString_WithNullOrEmptyOrWhitespaceId_ThrowsArgumentException(string id)
+		{
+			Assert.Throws<ArgumentException>(() =>
+				LocalizationManager.GetDynamicString(AppId, id, "some text"));
+		}
+
+		[TestCase(null)]
+		[TestCase("")]
+		[TestCase(" ")]
+		public void GetDynamicStringOrEnglish_WithNullOrEmptyOrWhitespaceId_ThrowsArgumentException(string id)
+		{
+			Assert.Throws<ArgumentException>(() =>
+				LocalizationManagerInternal<T>.GetDynamicStringOrEnglish(AppId, id, "some text", null, "en"));
+		}
+
+		[TestCase(null)]
+		[TestCase("")]
+		[TestCase(" ")]
+		public void GetString_WithNullOrEmptyOrWhitespaceId_ThrowsArgumentException(string id)
+		{
+			Assert.Throws<ArgumentException>(() =>
+				LocalizationManager.GetString(id, "some text"));
+		}
+
+		[TestCase(null)]
+		[TestCase("")]
+		[TestCase(" ")]
+		public void GetString_WithPreferredLanguageIds_WithNullOrEmptyOrWhitespaceId_ThrowsArgumentException(string id)
+		{
+			Assert.Throws<ArgumentException>(() =>
+				LocalizationManager.GetString(id, "some text", null, new[] { "en" }, out _));
+		}
+
+		[TestCase(null)]
+		[TestCase("")]
+		[TestCase(" ")]
+		public void UpdateLocalizedInfo_WithNullOrEmptyOrWhitespaceId_DoesNotAddEntry(string id)
+		{
+			using (var folder = new TempFolder())
+			{
+				SetupManager(folder);
+				var cache = LocalizationManagerInternal<T>.LoadedManagers[AppId].StringCache;
+				var locInfo = new LocalizingInfo(id)
+				{
+					LangId = "fr",
+					Text = "some text",
+					UpdateFields = UpdateFields.Text
+				};
+
+				cache.UpdateLocalizedInfo(locInfo);
+
+				// The guard in XliffTransUnitUpdater.Update should have prevented an empty-id
+				// entry from being added (which previously produced a malformed XLIFF file).
+				Assert.That(cache.GetString("fr", id ?? ""), Is.Null);
+			}
+		}
+
 		[Test]
 		public void GetDynamicStringOrEnglish_LmDisposed_GivesUsefulException()
 		{

src/L10NSharp/LocalizationManagerInternal.cs

@@ -461,6 +461,8 @@ public static string GetDynamicString(string appId, string id, string englishTex
 		/// ------------------------------------------------------------------------------------
 		public static string GetDynamicString(string appId, string id, string englishText, string comment)
 		{
+			if (string.IsNullOrWhiteSpace(id))
+				throw new ArgumentException("id may not be null or empty.", nameof(id));
 			return GetDynamicStringOrEnglish(appId, id, englishText, comment, LocalizationManager.UILanguageId);
 		}
 
@@ -479,6 +481,8 @@ public static string GetDynamicString(string appId, string id, string englishTex
 		public static string GetDynamicStringOrEnglish(string appId, string id, string englishText,
 			string comment, string langId)
 		{
+			if (string.IsNullOrWhiteSpace(id))
+				throw new ArgumentException("id may not be null or empty.", nameof(id));
 			// This happens in unit test environments or apps that have imported a library that
 			// is localized, but the app itself isn't initializing L10N yet.
 			if (LoadedManagers.Count == 0)
@@ -706,6 +710,8 @@ public static string GetString(string stringId, string englishText, string comme
 		public static string GetString(string stringId, string englishText, string comment, string englishToolTipText,
 			string englishShortcutKey, IComponent component)
 		{
+			if (string.IsNullOrWhiteSpace(stringId))
+				throw new ArgumentException("id may not be null or empty.", nameof(stringId));
 			return GetStringFromAnyLocalizationManager(stringId) ??
 				LocalizationManager.StripOffLocalizationInfoFromText(englishText);
 		}
@@ -721,6 +727,9 @@ public static string GetString(string stringId, string englishText, string comme
 		public static string GetString(string stringId, string englishText, string comment,
 			IEnumerable<string> preferredLanguageIds, out string languageIdUsed)
 		{
+			if (string.IsNullOrWhiteSpace(stringId))
+				throw new ArgumentException("id may not be null or empty.", nameof(stringId));
+
 			if (preferredLanguageIds == null)
 				throw new ArgumentNullException(nameof(preferredLanguageIds));
 

src/L10NSharp/XLiffUtils/XliffTransUnitUpdater.cs

@@ -46,6 +46,8 @@ internal bool Update(LocalizingInfo locInfo)
 			// Can't do anything without a language id.
 			if (string.IsNullOrEmpty(locInfo.LangId))
 				return _updated;
+			if (string.IsNullOrEmpty(locInfo.Id))
+				return _updated;
 
 			var xliffSource = _stringCache.GetDocument(_defaultLang);
 			Debug.Assert(xliffSource != null);