fix: Improve domain parsing, avoid trying to resolve onion/i2p favicons

src/Core/Core.csproj

@@ -29,6 +29,7 @@
 		<PackageReference Include="Microsoft.Maui.Controls" Version="$(MauiVersion)" />
 		<PackageReference Include="Microsoft.Maui.Controls.Compatibility" Version="$(MauiVersion)" />
 		<PackageReference Include="Microsoft.Extensions.Logging.Debug" Version="7.0.0" />
+		<PackageReference Include="Nager.PublicSuffix" Version="3.2.0" />
 	  <PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
 	  <PackageReference Include="MessagePack" Version="2.5.124" />
 	  <PackageReference Include="CsvHelper" Version="30.0.1" />

src/Core/Utilities/CoreHelpers.cs

@@ -6,6 +6,7 @@ using System.Text.RegularExpressions;
 using System.Web;
 using Bit.Core.Models.Domain;
 using Bit.Core.Services;
+using Nager.PublicSuffix;
 using Newtonsoft.Json;
 using Color = Microsoft.Maui.Graphics.Color;
 
@@ -19,6 +20,7 @@ namespace Bit.Core.Utilities
             "(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\." +
             "(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$";
 
+        // TODO: What's the point of this regex? Why not use the public suffix list data?
         public static readonly string TldEndingRegex =
             ".*\\.(com|net|org|edu|uk|gov|ca|de|jp|fr|au|ru|ch|io|es|us|co|xyz|info|ly|mil)$";
 
@@ -41,7 +43,7 @@ namespace Bit.Core.Utilities
         /// <summary>
         /// Returns the host (and not port) of the given uri.
         /// Does not support plain hostnames without a protocol.
-        /// 
+        ///
         /// Input => Output examples:
         /// <para>https://bitwarden.com => bitwarden.com</para>
         /// <para>https://login.bitwarden.com:1337 => login.bitwarden.com</para>
@@ -53,14 +55,15 @@ namespace Bit.Core.Utilities
         /// </summary>
         public static string GetHostname(string uriString)
         {
+            // TODO: The summary above seems to be wrong, this method does (and always has) support(ed) "plain hostnames without a protocol"
             var uri = GetUri(uriString);
-            return string.IsNullOrEmpty(uri?.Host) ? null : uri.Host;
+            return uri?.Host;
         }
 
         /// <summary>
         /// Returns the host and port of the given uri.
-        /// Does not support plain hostnames without 
+        /// Does not support plain hostnames without
-        /// 
+        ///
         /// Input => Output examples:
         /// <para>https://bitwarden.com => bitwarden.com</para>
         /// <para>https://login.bitwarden.com:1337 => login.bitwarden.com:1337</para>
@@ -89,8 +92,8 @@ namespace Bit.Core.Utilities
 
         /// <summary>
         /// Returns the second and top level domain of the given uri.
-        /// Does not support plain hostnames without 
+        /// Does not support plain hostnames without
-        /// 
+        ///
         /// Input => Output examples:
         /// <para>https://bitwarden.com => bitwarden.com</para>
         /// <para>https://login.bitwarden.com:1337 => bitwarden.com</para>
@@ -104,44 +107,42 @@ namespace Bit.Core.Utilities
         {
             var uri = GetUri(uriString);
             if (uri == null)
-            {
                 return null;
-            }
 
+            // TODO: What's the point of checking for "localhost" here? GetUri("localhost") always returns null
+            // TODO: Also, neither "localhost" nor any IPv4 (IPv6 addresses are never matched here), are "domain" (as in, domains with a name and tld)
             if (uri.Host == "localhost" || Regex.IsMatch(uri.Host, IpRegex))
-            {
                 return uri.Host;
-            }
+
-            try
+            // TODO: Resolving on every invocation is probably not a good idea
-            {
+            var domainParser = ServiceContainer.Resolve<IDomainParser>();
-                if (DomainName.TryParseBaseDomain(uri.Host, out var baseDomain))
+            var domainInfo = domainParser.Parse(uri.Host);
-                {
+            if (domainInfo == null)
-                    return baseDomain ?? uri.Host;
+                //TODO: Doing this results in non-domains being returned (like for example a Tor or I2P link) - return null instead
-                }
+                return uri.Host;
-            }
+
-            catch { }
+            return domainInfo.RegistrableDomain;
-            return null;
         }
 
         public static Uri GetUri(string uriString)
         {
             if (string.IsNullOrWhiteSpace(uriString))
-            {
                 return null;
-            }
+
-            var hasHttpProtocol = uriString.StartsWith("http://") || uriString.StartsWith("https://");
+            // TODO: Checking for a dot here (as before) means localhost (or any IPv6) is not recognised as a
-            if (!hasHttpProtocol && !uriString.Contains("://") && uriString.Contains("."))
+            // valid uri (but 127.0.0.1 is), even though all of them are perfectly valid
-            {
+            if (!IsHttpUrl(uriString) && uriString.Contains('.'))
-                if (Uri.TryCreate("http://" + uriString, UriKind.Absolute, out var uri))
+                uriString = $"http://{uriString}";
-                {
+
-                    return uri;
+            return Uri.TryCreate(uriString, UriKind.Absolute, out var uri) ? uri : null;
-                }
+        }
-            }
+
-            if (Uri.TryCreate(uriString, UriKind.Absolute, out var uri2))
+        private static bool IsHttpUrl(string url)
-            {
+        {
-                return uri2;
+            if (url.Length > 6 && url[..7].Equals(Uri.UriSchemeHttp, StringComparison.InvariantCultureIgnoreCase))
-            }
+                return true;
-            return null;
+
+            return url.Length > 7 && url[..8].Equals("https://", StringComparison.InvariantCultureIgnoreCase);
         }
 
         public static void NestedTraverse<T>(List<TreeNode<T>> nodeTree, int partIndex, string[] parts,

src/Core/Utilities/IconImageConverter.cs

@@ -6,6 +6,7 @@ using Bit.Core.Models.View;
 using Bit.Core.Utilities;
 using Microsoft.Maui.Controls;
 using Microsoft.Maui;
+using Nager.PublicSuffix;
 
 namespace Bit.App.Utilities
 {
@@ -42,33 +43,42 @@ namespace Bit.App.Utilities
             {
                 foreach (var uri in cipher.Login.Uris.Where(u => u.Uri != null))
                 {
-                    var hostnameUri = uri.Uri;
+                    var domain = GetValidDomainOrNull(uri.Uri);
-                    var isWebsite = false;
+                    if (domain != null)
-                    if (!hostnameUri.Contains("."))
                     {
-                        continue;
+                        image = GetIconUrl(domain);
-                    }
-                    if (!hostnameUri.Contains("://"))
-                    {
-                        hostnameUri = string.Concat("http://", hostnameUri);
-                    }
-                    isWebsite = hostnameUri.StartsWith("http");
-
-                    if (isWebsite)
-                    {
-                        image = GetIconUrl(hostnameUri);
                         break;
                     }
                 }
             }
+
             return image;
         }
 
-        private static string GetIconUrl(string hostnameUri)
+        // TODO: Assumes that only valid domains (not IP addresses) have a favicon.
+        // This might be shortsighted in the event that
+        //     a) a webservice is hosted on an IP
+        //     b) the icon server is user-supplied and has access to intranet services etc.
+        private static string GetValidDomainOrNull(string uriString)
         {
-            IEnvironmentService _environmentService = ServiceContainer.Resolve<IEnvironmentService>("environmentService");
+            var domainParser = ServiceContainer.Resolve<IDomainParser>();
+            var uri = CoreHelpers.GetUri(uriString);
+            if (uri == null)
+                return null;
+
+            if (uri.Host.EndsWith(".onion") || uri.Host.EndsWith(".i2p"))
+                return null;
+
+            var domainInfo = domainParser.Parse(uri.Host);
+            return domainInfo?.RegistrableDomain;
+        }
+
+        // TODO: Getting the service and re-formatting the icon API string doesn't have to be done for every single requested domain, right?
+        private static string GetIconUrl(string domain)
+        {
+            IEnvironmentService _environmentService =
+                ServiceContainer.Resolve<IEnvironmentService>("environmentService");
 
-            var hostname = CoreHelpers.GetHostname(hostnameUri);
             var iconsUrl = _environmentService.IconsUrl;
             if (string.IsNullOrWhiteSpace(iconsUrl))
             {
@@ -81,7 +91,8 @@ namespace Bit.App.Utilities
                     iconsUrl = "https://icons.bitwarden.net";
                 }
             }
-            return string.Format("{0}/{1}/icon.png", iconsUrl, hostname);
+
+            return string.Format("{0}/{1}/icon.png", iconsUrl, domain);
         }
     }
 }

src/Core/Utilities/MauiAssetRuleProvider.cs

@@ -0,0 +1,36 @@
+using Nager.PublicSuffix.Extensions;
+using Nager.PublicSuffix.Models;
+using Nager.PublicSuffix.RuleParsers;
+using Nager.PublicSuffix.RuleProviders;
+
+namespace Bit.Core.Utilities;
+
+public class MauiAssetRuleProvider : BaseRuleProvider
+{
+    private readonly string _fileName;
+
+    public MauiAssetRuleProvider(string fileName = "public_suffix_list.dat") => _fileName = fileName;
+
+    public override async Task<bool> BuildAsync(bool ignoreCache = false, CancellationToken cancellationToken = default)
+    {
+        var rules = new TldRuleParser().ParseRules(await LoadFromMauiAssetAsync().ConfigureAwait(false)).ToArray();
+        new DomainDataStructure("*", new TldRule("*")).AddRules(rules);
+        CreateDomainDataStructure(rules);
+        return true;
+    }
+
+    private async Task<string> LoadFromMauiAssetAsync()
+    {
+        try
+        {
+            await using var stream = await FileSystem.OpenAppPackageFileAsync(_fileName);
+            using var reader = new StreamReader(stream);
+
+            return await reader.ReadToEndAsync().ConfigureAwait(false);
+        }
+        catch
+        {
+            return null;
+        }
+    }
+}

src/Core/Utilities/ServiceContainer.cs

@@ -3,6 +3,7 @@ using System.Globalization;
 using System.Text;
 using Bit.Core.Abstractions;
 using Bit.Core.Services;
+using Nager.PublicSuffix;
 
 namespace Bit.Core.Utilities
 {
@@ -11,7 +12,7 @@ namespace Bit.Core.Utilities
         public static ConcurrentDictionary<string, object> RegisteredServices { get; set; } = new ConcurrentDictionary<string, object>();
         public static bool Inited { get; set; }
 
-        public static void Init(string customUserAgent = null, string clearCipherCacheKey = null,
+        public static async Task Init(string customUserAgent = null, string clearCipherCacheKey = null,
             string[] allClearCipherCacheKeys = null)
         {
             if (Inited)
@@ -119,6 +120,11 @@ namespace Bit.Core.Utilities
 #if ANDROID
             Register<IAssetLinksService>(new AssetLinksService(apiService));
 #endif
+
+            var ruleProvider = new MauiAssetRuleProvider();
+            await ruleProvider.BuildAsync();
+
+            Register<IDomainParser>("domainParser", new DomainParser(ruleProvider));
         }
 
         public static void Register<T>(string serviceName, T obj)