Symbol cache cleanup/fixes

src/Microsoft.Diagnostics.Runtime.Tests/src/SymbolGroupTests.cs

@@ -0,0 +1,66 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.Diagnostics.Runtime.Implementation;
+using Xunit;
+
+namespace Microsoft.Diagnostics.Runtime.Tests
+{
+    /// <summary>
+    /// Tests for SymbolGroup.EnumerateEntries
+    /// </summary>
+    public class SymbolGroupTests
+    {
+        [Fact]
+        public void TestSymsrv()
+        {
+            const string sympathWithDll = "symsrv*symstore.dll*https://msdl.microsoft.com/download/symbols";
+            (string cache, string[] servers) = SymbolGroup.EnumerateEntries(sympathWithDll);
+            Assert.Null(cache);
+            Assert.Equal(["https://msdl.microsoft.com/download/symbols"], servers);
+
+            const string sympathWithoutDll = "symsrv*https://msdl.microsoft.com/download/symbols";
+            (cache, servers) = SymbolGroup.EnumerateEntries(sympathWithoutDll);
+            Assert.Null(cache);
+            Assert.Equal(["https://msdl.microsoft.com/download/symbols"], servers);
+        }
+
+        [Fact]
+        public void TestServerWithCache()
+        {
+            const string sympath = "srv*d:\\cache*https://msdl.microsoft.com/download/symbols";
+            (string cache, string[] servers) = SymbolGroup.EnumerateEntries(sympath);
+            Assert.Equal("d:\\cache", cache);
+            Assert.Equal(["https://msdl.microsoft.com/download/symbols"], servers);
+
+            const string sympathWithDll = "srv*d:\\cache*https://msdl.microsoft.com/download/symbols*https://msdl.microsoft.com/download/symbols2";
+            (cache, servers) = SymbolGroup.EnumerateEntries(sympathWithDll);
+            Assert.Equal("d:\\cache", cache);
+            Assert.Equal(["https://msdl.microsoft.com/download/symbols", "https://msdl.microsoft.com/download/symbols2"], servers);
+
+            const string sympathWithoutCache = "srv*https://msdl.microsoft.com/download/symbols*https://msdl.microsoft.com/download/symbols2";
+            (cache, servers) = SymbolGroup.EnumerateEntries(sympathWithoutCache);
+            Assert.Null(cache);
+            Assert.Equal(["https://msdl.microsoft.com/download/symbols", "https://msdl.microsoft.com/download/symbols2"], servers);
+        }
+
+        [Fact]
+        public void TestCache()
+        {
+            const string sympath = "cache*d:\\cache";
+            (string cache, string[] servers) = SymbolGroup.EnumerateEntries(sympath);
+            Assert.Equal("d:\\cache", cache);
+            Assert.Empty(servers);
+
+            const string sympathWithDll = "cache*d:\\cache*https://msdl.microsoft.com/download/symbols";
+            (cache, servers) = SymbolGroup.EnumerateEntries(sympathWithDll);
+            Assert.Equal("d:\\cache", cache);
+            Assert.Equal(["https://msdl.microsoft.com/download/symbols"], servers);
+
+            const string sympathWithDll2 = "cache*d:\\cache*https://msdl.microsoft.com/download/symbols*https://msdl.microsoft.com/download/symbols2";
+            (cache, servers) = SymbolGroup.EnumerateEntries(sympathWithDll2);
+            Assert.Equal("d:\\cache", cache);
+            Assert.Equal(["https://msdl.microsoft.com/download/symbols", "https://msdl.microsoft.com/download/symbols2"], servers);
+        }
+    }
+}

src/Microsoft.Diagnostics.Runtime/Implementation/SymbolGroup.cs

@@ -112,7 +112,7 @@ public static IFileLocator CreateFromSymbolPath(string symbolPath, TokenCredenti
 
                 foreach (string server in Servers)
                 {
-                    if (server.StartsWith("http:", StringComparison.OrdinalIgnoreCase) || server.StartsWith("https:", StringComparison.OrdinalIgnoreCase))
+                    if (IsUrl(server))
                     {
                         SymbolServer symSvr = new(cache, server, credential);
                         locators.Add(symSvr);
@@ -147,49 +147,67 @@ public static IFileLocator CreateFromSymbolPath(string symbolPath, TokenCredenti
             return new SymbolGroup(locators);
         }
 
-        private static (string? Cache, string[] Servers) EnumerateEntries(string part)
+        internal static (string? Cache, string[] Servers) EnumerateEntries(string part)
         {
-            if (!part.Contains('*'))
-                return (null, new string[] { part });
-
-            string[] split = part.Split('*');
-            DebugOnly.Assert(split.Length > 1);
-
-            if (split[0].Equals("cache"))
-                return (split[1], split.Skip(2).ToArray());
-
-
-            if (split[0].Equals("symsrv", StringComparison.OrdinalIgnoreCase))
+            string? cache = null;
+            List<string> servers = [];
+            foreach (string entry in EnumerateParts(part))
             {
-                // We don't really support this, but we'll make it work...ish.
-                // Convert symsrv*symstore.dll*DownStream*server -> srv*DownStream*server
-
-                if (split.Length < 3)
-                    return (null, new string[] { part });
-
-                split = new string[] { "srv" }.Concat(split.Skip(2)).ToArray();
+                if (cache is null && servers.Count == 0 && !IsUrl(entry))
+                    cache = entry;
+                else
+                    servers.Add(entry);
             }
 
+            return (cache, servers.ToArray());
+        }
 
-            if (split[0].Equals("svr", StringComparison.OrdinalIgnoreCase) || split[0].Equals("srv", StringComparison.OrdinalIgnoreCase))
+        private static IEnumerable<string> EnumerateParts(string path)
+        {
+            bool possiblySkipNextDll = false;
+            int curr = 0;
+            for (int i = 0; i < path.Length; i++)
             {
-                string? cache = split[1];
-
-                if (string.IsNullOrWhiteSpace(cache))
-                    cache = null;
-
-                // e.g. "svr*http://symbols.com/"
-                if (split.Length == 2)
+                if (path[i] == '*')
                 {
-                    if (cache is null)
-                        return (split[1], split.Skip(2).ToArray());
+                    ReadOnlySpan<char> part = path.AsSpan(curr, i - curr).Trim();
+                    curr = i + 1;
 
-                    return (null, new string[] { cache });
+                    if (part.Equals("cache".AsSpan(), StringComparison.OrdinalIgnoreCase)
+                        || part.Equals("svr".AsSpan(), StringComparison.OrdinalIgnoreCase)
+                        || part.Equals("srv".AsSpan(), StringComparison.OrdinalIgnoreCase))
+                    {
+                        // Don't yield this.
+                    }
+                    else if (part.Equals("symsrv".AsSpan(), StringComparison.OrdinalIgnoreCase))
+                    {
+                        possiblySkipNextDll = true;
+                    }
+                    else
+                    {
+                        bool skip = possiblySkipNextDll && part.EndsWith(".dll".AsSpan(), StringComparison.OrdinalIgnoreCase);
+                        possiblySkipNextDll = false;
+
+                        if (!skip && part.Length > 0)
+                            yield return part.ToString();
+                    }
                 }
             }
 
-            // Ok, so we have * but it didn't start with srv or svr, so what now?
-            return (null, split.Where(s => !string.IsNullOrWhiteSpace(s)).ToArray());
+            if (curr < path.Length)
+            {
+                ReadOnlySpan<char> part = path.AsSpan(curr).Trim();
+                bool skip = possiblySkipNextDll && part.EndsWith(".dll".AsSpan(), StringComparison.OrdinalIgnoreCase);
+                if (!skip && part.Length > 0)
+                    yield return part.ToString();
+            }
+        }
+
+        private static bool IsUrl(string path)
+        {
+            bool result = Uri.TryCreate(path, UriKind.Absolute, out Uri? uriResult)
+                          && (uriResult.Scheme == Uri.UriSchemeHttp || uriResult.Scheme == Uri.UriSchemeHttps);
+            return result;
         }
     }
 }

src/Microsoft.Diagnostics.Runtime/Implementation/SymbolServer.cs

@@ -32,7 +32,7 @@ internal SymbolServer(FileSymbolCache cache, string server, TokenCredential? cre
                 throw new ArgumentNullException(nameof(cache));
 
             _cache = cache;
-            _accessToken = default;Server = server;
+            Server = server;
 
             if (IsSymweb(server))
             {