diff --git a/src/TraceEvent/Symbols/SymbolReader.cs b/src/TraceEvent/Symbols/SymbolReader.cs index 18c86a5e1..e107c22b4 100644 --- a/src/TraceEvent/Symbols/SymbolReader.cs +++ b/src/TraceEvent/Symbols/SymbolReader.cs @@ -1726,15 +1726,36 @@ internal bool GetUrlForFilePathUsingSourceLink(string buildTimeFilePath, out str if (_sourceLinkMapping != null) { - foreach (Tuple map in _sourceLinkMapping) + // First, try to find an exact match for the buildTimeFilePath (non-wildcard entries) + foreach (Tuple map in _sourceLinkMapping) { string path = map.Item1; - string urlReplacement = map.Item2; + string urlTemplate = map.Item2; + bool isWildcard = map.Item3; - if (buildTimeFilePath.StartsWith(path, StringComparison.OrdinalIgnoreCase)) + if (!isWildcard && buildTimeFilePath.Equals(path, StringComparison.OrdinalIgnoreCase)) { + // Exact match found - this is a direct file mapping without wildcards + relativeFilePath = ""; + url = urlTemplate; // Use the URL as-is for exact matches + return true; + } + } + + // If no exact match, try prefix matching (wildcard patterns only) + // Per spec rule 2: only paths that had a wildcard (*) should be used for prefix matching + foreach (Tuple map in _sourceLinkMapping) + { + string path = map.Item1; + string urlTemplate = map.Item2; + bool isWildcard = map.Item3; + + // Only use wildcard patterns for prefix matching + if (isWildcard && buildTimeFilePath.StartsWith(path, StringComparison.OrdinalIgnoreCase)) + { + // Prefix match - extract the relative path and substitute into URL relativeFilePath = buildTimeFilePath.Substring(path.Length, buildTimeFilePath.Length - path.Length).Replace('\\', '/'); - url = urlReplacement.Replace("*", string.Join("/", relativeFilePath.Split('/').Select(Uri.EscapeDataString))); + url = urlTemplate.Replace("*", string.Join("/", relativeFilePath.Split('/').Select(Uri.EscapeDataString))); return true; } } @@ -1746,11 +1767,12 @@ internal bool GetUrlForFilePathUsingSourceLink(string buildTimeFilePath, out str } /// - /// Parses SourceLink information and returns a list of filepath -> url Prefix tuples. + /// Parses SourceLink information and returns a list of (filepath, url, isWildcard) tuples. + /// Supports both wildcard patterns ("path\\*" -> "url/*") and exact path mappings ("path\\file.h" -> "url"). /// - private List> ParseSourceLinkJson(IEnumerable sourceLinkContents) + private List> ParseSourceLinkJson(IEnumerable sourceLinkContents) { - List> ret = null; + List> ret = null; foreach (string sourceLinkJson in sourceLinkContents) { // TODO this is not right for corner cases (e.g. file paths with " or , } in them) @@ -1760,24 +1782,26 @@ private List> ParseSourceLinkJson(IEnumerable sour string mappings = m.Groups[1].Value; while (!string.IsNullOrWhiteSpace(mappings)) { - m = Regex.Match(m.Groups[1].Value, "^\\s*\"(.*?)\"\\s*:\\s*\"(.*?)\"\\s*,?(.*)", RegexOptions.Singleline); + m = Regex.Match(mappings, "^\\s*\"(.*?)\"\\s*:\\s*\"(.*?)\"\\s*,?(.*)", RegexOptions.Singleline); if (m.Success) { if (ret == null) { - ret = new List>(); + ret = new List>(); } string pathSpec = m.Groups[1].Value.Replace("\\\\", "\\"); - if (pathSpec.EndsWith("*")) - { - pathSpec = pathSpec.Substring(0, pathSpec.Length - 1); // Remove the * - ret.Add(new Tuple(pathSpec, m.Groups[2].Value)); - } - else + string urlSpec = m.Groups[2].Value; + bool isWildcard = pathSpec.EndsWith("*"); + + // Support both wildcard patterns and exact path mappings + if (isWildcard) { - _log.WriteLine("Warning: {0} does not end in *, skipping this mapping.", pathSpec); + // Wildcard pattern: remove the * from the path + pathSpec = pathSpec.Substring(0, pathSpec.Length - 1); } + // Add the mapping with wildcard flag + ret.Add(new Tuple(pathSpec, urlSpec, isWildcard)); mappings = m.Groups[3].Value; } @@ -1799,7 +1823,7 @@ private List> ParseSourceLinkJson(IEnumerable sour private string _pdbPath; private SymbolReader _reader; - private List> _sourceLinkMapping; // Used by SourceLink to map build paths to URLs (see GetUrlForFilePath) + private List> _sourceLinkMapping; // Used by SourceLink to map build paths to URLs (path, url, isWildcard) private bool _sourceLinkMappingInited; // Lazy init flag. #endregion } diff --git a/src/TraceEvent/TraceEvent.Tests/Symbols/SymbolReaderTests.cs b/src/TraceEvent/TraceEvent.Tests/Symbols/SymbolReaderTests.cs index 08ff2108f..2a17ca2cc 100644 --- a/src/TraceEvent/TraceEvent.Tests/Symbols/SymbolReaderTests.cs +++ b/src/TraceEvent/TraceEvent.Tests/Symbols/SymbolReaderTests.cs @@ -153,6 +153,53 @@ public void SourceLinkUrlsAreEscaped() } } + [Fact] + public void SourceLinkSupportsWildcardAndExactPathMappings() + { + // Create a test symbol module that returns SourceLink JSON with both wildcard and exact path mappings + var testModule = new TestSymbolModuleWithSourceLink(_symbolReader); + + // Test wildcard pattern matching + bool result1 = testModule.GetUrlForFilePathUsingSourceLink( + @"C:\src\myproject\subfolder\file.cs", + out string url1, + out string relativePath1); + + Assert.True(result1, "Should match wildcard pattern"); + Assert.Equal("https://raw.githubusercontent.com/org/repo/commit/subfolder/file.cs", url1); + Assert.Equal("subfolder/file.cs", relativePath1); + + // Test exact path matching + bool result2 = testModule.GetUrlForFilePathUsingSourceLink( + @"c:\external\sdk\inc\header.h", + out string url2, + out string relativePath2); + + Assert.True(result2, "Should match exact path"); + Assert.Equal("https://example.com/blobs/ABC123?download=true&filename=header.h", url2); + Assert.Equal("", relativePath2); + + // Test another wildcard pattern with escaped characters + bool result3 = testModule.GetUrlForFilePathUsingSourceLink( + @"C:\src\myproject\some folder\another file.cs", + out string url3, + out string relativePath3); + + Assert.True(result3, "Should match wildcard pattern with spaces"); + Assert.Equal("https://raw.githubusercontent.com/org/repo/commit/some%20folder/another%20file.cs", url3); + Assert.Equal("some folder/another file.cs", relativePath3); + + // Test non-matching path + bool result4 = testModule.GetUrlForFilePathUsingSourceLink( + @"C:\other\path\file.cs", + out string url4, + out string relativePath4); + + Assert.False(result4, "Should not match any pattern"); + Assert.Null(url4); + Assert.Null(relativePath4); + } + /// /// Tests that the checksum matching allows for different line endings. /// Open the PDB and try to retrieve the source code for one of the files, @@ -370,31 +417,31 @@ public void MsfzFileDetectionWorks() var tempDir = Path.GetTempPath(); var testFile = Path.Combine(tempDir, "test_msfz.pdb"); var nonMsfzFile = Path.Combine(tempDir, "test_non_msfz.pdb"); - + try { // Write MSFZ header followed by some dummy data var msfzHeader = "Microsoft MSFZ Container"; var headerBytes = Encoding.UTF8.GetBytes(msfzHeader); var dummyData = new byte[] { 0x01, 0x02, 0x03, 0x04 }; - + using (var stream = File.Create(testFile)) { stream.Write(headerBytes, 0, headerBytes.Length); stream.Write(dummyData, 0, dummyData.Length); } - + // Use reflection to call the private IsMsfzFile method - var method = typeof(SymbolReader).GetMethod("IsMsfzFile", + var method = typeof(SymbolReader).GetMethod("IsMsfzFile", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); - + var result = (bool)method.Invoke(_symbolReader, new object[] { testFile }); - + Assert.True(result, "File with MSFZ header should be detected as MSFZ file"); - + // Test with non-MSFZ file File.WriteAllText(nonMsfzFile, "This is not an MSFZ file"); - + result = (bool)method.Invoke(_symbolReader, new object[] { nonMsfzFile }); Assert.False(result, "File without MSFZ header should not be detected as MSFZ file"); } @@ -412,30 +459,30 @@ public void MsfzFileMovesToCorrectSubdirectory() { var tempDir = Path.Combine(Path.GetTempPath(), "msfz_test_" + Guid.NewGuid().ToString("N")); Directory.CreateDirectory(tempDir); - + try { var testFile = Path.Combine(tempDir, "test.pdb"); - + // Create MSFZ file var msfzHeader = "Microsoft MSFZ Container"; var headerBytes = Encoding.UTF8.GetBytes(msfzHeader); var dummyData = new byte[] { 0x01, 0x02, 0x03, 0x04 }; - + using (var stream = File.Create(testFile)) { stream.Write(headerBytes, 0, headerBytes.Length); stream.Write(dummyData, 0, dummyData.Length); } - + // Since MSFZ logic is now integrated into GetFileFromServer, // this test validates the MSFZ detection logic which remains the same - var isMsfzMethod = typeof(SymbolReader).GetMethod("IsMsfzFile", + var isMsfzMethod = typeof(SymbolReader).GetMethod("IsMsfzFile", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); - + var isMsfz = (bool)isMsfzMethod.Invoke(_symbolReader, new object[] { testFile }); Assert.True(isMsfz, "File should be detected as MSFZ file"); - + // The file moving functionality is now tested through integration tests // since it's part of the GetFileFromServer method } @@ -451,39 +498,40 @@ public void HttpRequestIncludesMsfzAcceptHeader() { // This test verifies that our HttpRequestMessage creation includes the MSFZ accept header // We'll create a minimal test by checking the private method behavior indirectly - + var tempDir = Path.Combine(Path.GetTempPath(), "msfz_http_test_" + Guid.NewGuid().ToString("N")); Directory.CreateDirectory(tempDir); var targetPath = Path.Combine(tempDir, "test.pdb"); - + try { // Configure intercepting handler to capture the request with MSFZ content - _handler.AddIntercept(new Uri("https://test.example.com/test.pdb"), HttpMethod.Get, HttpStatusCode.OK, () => { + _handler.AddIntercept(new Uri("https://test.example.com/test.pdb"), HttpMethod.Get, HttpStatusCode.OK, () => + { var msfzContent = "Microsoft MSFZ Container\x00\x01\x02\x03"; return new StringContent(msfzContent, Encoding.UTF8, "application/msfz0"); }); - + // This will trigger an HTTP request that should include the Accept header - var method = typeof(SymbolReader).GetMethod("GetPhysicalFileFromServer", + var method = typeof(SymbolReader).GetMethod("GetPhysicalFileFromServer", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); - - var result = (bool)method.Invoke(_symbolReader, new object[] { - "https://test.example.com", - "test.pdb", - targetPath, - null + + var result = (bool)method.Invoke(_symbolReader, new object[] { + "https://test.example.com", + "test.pdb", + targetPath, + null }); - + // Verify that the download was successful Assert.True(result, "GetPhysicalFileFromServer should succeed with MSFZ content"); - + // In the new architecture, GetPhysicalFileFromServer just downloads the file // The MSFZ moving logic is handled by GetFileFromServer Assert.True(File.Exists(targetPath), "Downloaded file should exist at target path"); - + // Verify the content is MSFZ - var isMsfzMethod = typeof(SymbolReader).GetMethod("IsMsfzFile", + var isMsfzMethod = typeof(SymbolReader).GetMethod("IsMsfzFile", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); var isMsfz = (bool)isMsfzMethod.Invoke(_symbolReader, new object[] { targetPath }); Assert.True(isMsfz, "Downloaded file should be detected as MSFZ"); @@ -577,5 +625,37 @@ protected override Task SendAsync(HttpRequestMessage reques return base.SendAsync(request, cancellationToken); } } + + /// + /// A test symbol module that provides SourceLink JSON for testing. + /// + private class TestSymbolModuleWithSourceLink : ManagedSymbolModule + { + public TestSymbolModuleWithSourceLink(SymbolReader reader) + : base(reader, "test.pdb") + { + } + + public override SourceLocation SourceLocationForManagedCode(uint methodMetadataToken, int ilOffset) + { + // Not used in this test + return null; + } + + protected override IEnumerable GetSourceLinkJson() + { + // Return SourceLink JSON with both wildcard and exact path mappings + // This mimics the example from issue #2350 + return new[] + { + @"{ + ""documents"": { + ""C:\\src\\myproject\\*"": ""https://raw.githubusercontent.com/org/repo/commit/*"", + ""c:\\external\\sdk\\inc\\header.h"": ""https://example.com/blobs/ABC123?download=true&filename=header.h"" + } + }" + }; + } + } } -} +} \ No newline at end of file