Improve uninstall list load error handling

source/BulkCrapUninstaller/Controls/Settings/AdvancedFilters.cs

@@ -110,16 +110,16 @@ public void LoadUninstallList(string fileName)
                 uninstallListEditor1.CurrentList = result;
                 UnsavedChanges = false;
             }
-            catch (SecurityException ex)
-            {
-                PremadeDialogs.GenericError(ex);
-            }
-            catch (Exception ex)
-            {
-                PremadeDialogs.GenericError("File is not an uninstall list or it can't be opened",
-                    "Please note that uninstall lists are saved in the \"Advanced filtering\" view, not by exporting. Lists should have the .bcul extension.\n\nError message: " + ex.Message);
-            }
-        }
+            catch (SecurityException ex)
+            {
+                PremadeDialogs.GenericError(ex);
+            }
+            catch (Exception ex)
+            {
+                MessageBoxes.OpenUninstallListError(
+                    Localisable.MessageBoxes_Error_details + ex.Message);
+            }
+        }
 
         /// <summary>
         /// Show file select gui

source/BulkCrapUninstallerTests/UninstallListTests.cs

@@ -0,0 +1,54 @@
+using System.IO;
+using System.Text;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using UninstallTools.Lists;
+
+namespace BulkCrapUninstallerTests
+{
+    [TestClass]
+    public class UninstallListTests
+    {
+        [TestMethod]
+        public void ReadFromFile_EmptyFile_ThrowsInvalidDataException()
+        {
+            var path = Path.GetTempFileName();
+            try
+            {
+                File.WriteAllText(path, string.Empty);
+
+                var ex = Assert.ThrowsException<InvalidDataException>(() => UninstallList.ReadFromFile(path));
+
+                StringAssert.Contains(ex.Message, "empty");
+            }
+            finally
+            {
+                File.Delete(path);
+            }
+        }
+
+        [TestMethod]
+        public void ReadFromFile_LeadingBomAndWhitespace_LoadsList()
+        {
+            var path = Path.GetTempFileName();
+            try
+            {
+                var input = new UninstallList(new[] { new Filter("Test", "Example App") });
+                input.SaveToFile(path);
+
+                var originalContents = File.ReadAllText(path);
+                File.WriteAllText(path, "\uFEFF\r\n\t  " + originalContents, Encoding.UTF8);
+
+                var result = UninstallList.ReadFromFile(path);
+
+                Assert.IsNotNull(result);
+                Assert.AreEqual(1, result.Filters.Count);
+                Assert.AreEqual("Test", result.Filters[0].Name);
+                Assert.AreEqual("Example App", result.Filters[0].ComparisonEntries[0].FilterText);
+            }
+            finally
+            {
+                File.Delete(path);
+            }
+        }
+    }
+}

source/UninstallTools/Lists/UninstallList.cs

@@ -3,19 +3,29 @@
     Apache License Version 2.0
 */
 
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Xml;
-using System.Xml.Serialization;
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Text;
+using System.Xml;
+using System.Xml.Serialization;
 
 namespace UninstallTools.Lists
 {
-    public class UninstallList : ITestEntry
-    {
-        public UninstallList()
-        {
-        }
+    public class UninstallList : ITestEntry
+    {
+        private static readonly XmlReaderSettings ReaderSettings = new()
+        {
+            CloseInput = false,
+            DtdProcessing = DtdProcessing.Prohibit,
+            IgnoreComments = true,
+            IgnoreWhitespace = true
+        };
+
+        public UninstallList()
+        {
+        }
 
         public UninstallList(IEnumerable<Filter> items)
         {
@@ -69,16 +79,46 @@ public UninstallList(IEnumerable<Filter> items)
             return included.Value;
         }
 
-        public bool Enabled { get; set; } = true;
-
-        public static UninstallList ReadFromFile(string fileName)
-        {
-            var serializer = new XmlSerializer(typeof (UninstallList));
-            using (var reader = new XmlTextReader(fileName))
-            {
-                return serializer.Deserialize(reader) as UninstallList;
-            }
-        }
+        public bool Enabled { get; set; } = true;
+
+        public static UninstallList ReadFromFile(string fileName)
+        {
+            if (string.IsNullOrWhiteSpace(fileName))
+                throw new ArgumentException("File name cannot be empty.", nameof(fileName));
+
+            var serializer = new XmlSerializer(typeof (UninstallList));
+            using (var stream = File.OpenRead(fileName))
+            {
+                if (stream.Length == 0)
+                    throw new InvalidDataException("The uninstall list file is empty.");
+
+                try
+                {
+                    return Deserialize(serializer, XmlReader.Create(stream, ReaderSettings));
+                }
+                catch (Exception ex) when (ex is InvalidOperationException or XmlException)
+                {
+                    if (!stream.CanSeek)
+                        throw CreateInvalidDataException(ex);
+
+                    stream.Position = 0;
+                    using var textReader = new StreamReader(stream, Encoding.UTF8, true, 1024, true);
+                    var trimmedContents = textReader.ReadToEnd().TrimStart('\uFEFF', '\u200B', ' ', '\t', '\r', '\n');
+                    if (trimmedContents.Length == 0)
+                        throw CreateInvalidDataException(ex);
+
+                    using var stringReader = new StringReader(trimmedContents);
+                    try
+                    {
+                        return Deserialize(serializer, XmlReader.Create(stringReader, ReaderSettings));
+                    }
+                    catch (Exception retryEx) when (retryEx is InvalidOperationException or XmlException)
+                    {
+                        throw CreateInvalidDataException(retryEx);
+                    }
+                }
+            }
+        }
 
         public void AddItems(IEnumerable<string> items)
         {
@@ -105,10 +145,44 @@ public void SaveToFile(string fileName)
             }
         }
 
-        internal void Remove(Filter item)
-        {
-            if (Filters.Contains(item))
-                Filters.Remove(item);
-        }
-    }
-}
+        internal void Remove(Filter item)
+        {
+            if (Filters.Contains(item))
+                Filters.Remove(item);
+        }
+
+        private static UninstallList Deserialize(XmlSerializer serializer, XmlReader reader)
+        {
+            using (reader)
+            {
+                reader.MoveToContent();
+                if (reader.NodeType != XmlNodeType.Element ||
+                    !string.Equals(reader.LocalName, nameof(UninstallList), StringComparison.Ordinal))
+                {
+                    throw new InvalidDataException(
+                        $"The file does not contain an uninstall list. Expected root element '{nameof(UninstallList)}' but found '{reader.LocalName}'.");
+                }
+
+                var result = serializer.Deserialize(reader) as UninstallList;
+                if (result == null)
+                    throw new InvalidDataException("The uninstall list file could not be deserialized.");
+
+                result.Filters ??= new List<Filter>();
+                return result;
+            }
+        }
+
+        private static InvalidDataException CreateInvalidDataException(Exception ex)
+        {
+            var xmlException = ex as XmlException ?? ex.InnerException as XmlException;
+            if (xmlException != null)
+            {
+                return new InvalidDataException(
+                    $"The uninstall list XML is invalid at line {xmlException.LineNumber}, position {xmlException.LinePosition}: {xmlException.Message}",
+                    ex);
+            }
+
+            return new InvalidDataException("The uninstall list file is not valid XML.", ex);
+        }
+    }
+}