[wasm][debugger] Revert don't need to escape special characters anymo…

…re (#78320) * Trying to fix again * Removing comment * trying to fix CI * implementing escaped string for dotnet:// as discussed with @radical * addressing radical suggestion * fix CI * implementing test with colon as discussed with @radical. * fix on windows * pushing @radical changes and addressing @radical comments * using ilona suggestion about conditionalfact * Reverting one suggestion. * Fixing behavior on mac
dotnet · Dec 8, 2022 · 2480f01 · 2480f01
1 parent 41eab87
commit 2480f01
Show file tree

Hide file tree

Showing 8 changed files with 110 additions and 55 deletions.
diff --git a/.gitignore b/.gitignore
@@ -363,3 +363,5 @@ src/coreclr/System.Private.CoreLib/common
 # Temporary artifacts from local libraries stress builds
 .dotnet-daily/
 run-stress-*
+debugger-test-with-colon-in-source-name.csproj
+test:.cs
diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs b/src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
@@ -99,10 +99,10 @@ public bool IsMatch(SourceFile sourceFile)
             {
                 string urlRegex = request?["urlRegex"].Value<string>();
                 var regex = new Regex(urlRegex);
-                return regex.IsMatch(sourceFile.Url.ToString()) || regex.IsMatch(sourceFile.DocUrl);
+                return regex.IsMatch(sourceFile.Url.ToString()) || regex.IsMatch(sourceFile.FilePath);
             }
 
-            return sourceFile.Url.ToString() == url || sourceFile.DotNetUrl == url;
+            return sourceFile.Url.ToString() == url || sourceFile.DotNetUrlEscaped == url;
         }
 
         public bool TryResolve(SourceFile sourceFile)
@@ -117,7 +117,7 @@ public bool TryResolve(SourceFile sourceFile)
                 return false;
 
             Assembly = sourceFile.AssemblyName;
-            File = sourceFile.DebuggerFileName;
+            File = sourceFile.FilePath;
             Line = line.Value;
             Column = column.Value;
             return true;
@@ -327,7 +327,7 @@ internal sealed class MethodInfo
 
         public SourceId SourceId => Source.SourceId;
 
-        public string SourceName => Source.DebuggerFileName;
+        public string SourceName => Source.FilePath;
 
         public string Name { get; }
         public MethodDebugInformation DebugInformation;
@@ -1200,34 +1200,54 @@ internal void UpdatePdbInformation(Stream streamToReadFrom)
     }
     internal sealed class SourceFile
     {
+        private static readonly Regex regexForEscapeFileName = new(@"([:/])", RegexOptions.Compiled);
         private Dictionary<int, MethodInfo> methods;
         private AssemblyInfo assembly;
         private Document doc;
         private DocumentHandle docHandle;
-        private string url;
         internal List<int> BreakableLines { get; }
 
-        internal SourceFile(AssemblyInfo assembly, int id, DocumentHandle docHandle, Uri sourceLinkUri, string url)
+        public string FilePath { get; init; }
+        public string FileUriEscaped { get; init; }
+        public string DotNetUrlEscaped { get; init; }
+
+        public Uri Url { get; init; }
+        public Uri SourceLinkUri { get; init; }
+
+        public int Id { get; }
+        public string AssemblyName => assembly.Name;
+        public SourceId SourceId => new SourceId(assembly.Id, this.Id);
+        public IEnumerable<MethodInfo> Methods => this.methods.Values;
+
+        internal SourceFile(AssemblyInfo assembly, int id, DocumentHandle docHandle, Uri sourceLinkUri, string documentName)
         {
             this.methods = new Dictionary<int, MethodInfo>();
             this.SourceLinkUri = sourceLinkUri;
             this.assembly = assembly;
             this.Id = id;
             this.doc = assembly.pdbMetadataReader.GetDocument(docHandle);
             this.docHandle = docHandle;
-            this.url = url;
-            this.DebuggerFileName = url.Replace("\\", "/").Replace(":", "");
             this.BreakableLines = new List<int>();
 
-            this.SourceUri = new Uri((Path.IsPathRooted(url) ? "file://" : "") + url, UriKind.RelativeOrAbsolute);
-            if (SourceUri.IsFile && File.Exists(SourceUri.LocalPath))
-            {
-                this.Url = this.SourceUri.ToString();
-            }
-            else
+            this.FilePath = documentName;
+
+            string escapedDocumentName = EscapePathForUri(documentName.Replace("\\", "/"));
+            this.FileUriEscaped = $"file://{(OperatingSystem.IsWindows() ? "/" : "")}{escapedDocumentName}";
+            this.DotNetUrlEscaped = $"dotnet://{assembly.Name}/{escapedDocumentName}";
+            this.Url = new Uri(File.Exists(documentName) ? FileUriEscaped : DotNetUrlEscaped, UriKind.Absolute);
+        }
+
+        private static string EscapePathForUri(string path)
+        {
+            var builder = new StringBuilder();
+            foreach (var part in regexForEscapeFileName.Split(path))
             {
-                this.Url = DotNetUrl;
+                if (part == ":" || part == "/")
+                    builder.Append(part);
+                else
+                    builder.Append(Uri.EscapeDataString(part));
             }
+            return builder.ToString();
         }
 
         internal void AddMethod(MethodInfo mi)
@@ -1238,35 +1258,21 @@ internal void AddMethod(MethodInfo mi)
             }
         }
 
-        public string DebuggerFileName { get; }
-        public string Url { get; }
-        public int Id { get; }
-        public string AssemblyName => assembly.Name;
-        public string DotNetUrl => $"dotnet://{assembly.Name}/{DebuggerFileName}";
-
-        public SourceId SourceId => new SourceId(assembly.Id, this.Id);
-        public Uri SourceLinkUri { get; }
-        public Uri SourceUri { get; }
-
-        public IEnumerable<MethodInfo> Methods => this.methods.Values;
-
-        public string DocUrl => url;
-
         public (int startLine, int startColumn, int endLine, int endColumn) GetExtents()
         {
             MethodInfo start = Methods.OrderBy(m => m.StartLocation.Line).ThenBy(m => m.StartLocation.Column).First();
             MethodInfo end = Methods.OrderByDescending(m => m.EndLocation.Line).ThenByDescending(m => m.EndLocation.Column).First();
             return (start.StartLocation.Line, start.StartLocation.Column, end.EndLocation.Line, end.EndLocation.Column);
         }
 
-        private async Task<MemoryStream> GetDataAsync(Uri uri, CancellationToken token)
+        private static async Task<MemoryStream> GetDataAsync(Uri uri, CancellationToken token)
         {
             var mem = new MemoryStream();
             try
             {
                 if (uri.IsFile && File.Exists(uri.LocalPath))
                 {
-                    using (FileStream file = File.Open(SourceUri.LocalPath, FileMode.Open))
+                    using (FileStream file = File.Open(uri.LocalPath, FileMode.Open))
                     {
                         await file.CopyToAsync(mem, token).ConfigureAwait(false);
                         mem.Position = 0;
@@ -1340,7 +1346,7 @@ where reader.GetGuid(cdi.Kind) == PortableCustomDebugInfoKinds.EmbeddedSource
                 }
             }
 
-            foreach (Uri url in new[] { SourceUri, SourceLinkUri })
+            foreach (Uri url in new[] { new Uri(FileUriEscaped), SourceLinkUri })
             {
                 MemoryStream mem = await GetDataAsync(url, token).ConfigureAwait(false);
                 if (mem != null && mem.Length > 0 && (!checkHash || CheckPdbHash(ComputePdbHash(mem))))
@@ -1358,11 +1364,11 @@ public object ToScriptSource(int executionContextId, object executionContextAuxD
             return new
             {
                 scriptId = SourceId.ToString(),
-                url = Url,
+                url = Url.OriginalString,
                 executionContextId,
                 executionContextAuxData,
                 //hash:  should be the v8 hash algo, managed implementation is pending
-                dotNetUrl = DotNetUrl,
+                dotNetUrl = DotNetUrlEscaped
             };
         }
     }
@@ -1614,7 +1620,7 @@ public IEnumerable<SourceLocation> FindBreakpointLocations(BreakpointRequest req
             request.TryResolve(this);
 
             AssemblyInfo asm = assemblies.FirstOrDefault(a => a.Name.Equals(request.Assembly, StringComparison.OrdinalIgnoreCase));
-            SourceFile sourceFile = asm?.Sources?.SingleOrDefault(s => s.DebuggerFileName.Equals(request.File, StringComparison.OrdinalIgnoreCase));
+            SourceFile sourceFile = asm?.Sources?.SingleOrDefault(s => s.FilePath.Equals(request.File, StringComparison.OrdinalIgnoreCase));
 
             if (sourceFile == null)
                 yield break;
@@ -1680,6 +1686,6 @@ static List<MethodInfo> FindMethodsContainingLine(SourceFile sourceFile, int lin
             }
         }
 
-        public string ToUrl(SourceLocation location) => location != null ? GetFileById(location.Id).Url : "";
+        public string ToUrl(SourceLocation location) => location != null ? GetFileById(location.Id).Url.OriginalString : "";
     }
 }
diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs
@@ -838,7 +838,7 @@ internal override async Task OnSourceFileAdded(SessionId sessionId, SourceFile s
             isBlackBoxed = false,
             introductionType = "scriptElement",
             resourceType = "source",
-            dotNetUrl = source.DotNetUrl
+            dotNetUrl = source.DotNetUrlEscaped
         });
         JObject sourcesJObj;
         if (!string.IsNullOrEmpty(ctx.GlobalName))
@@ -1014,8 +1014,7 @@ internal override async Task<bool> OnGetScriptSource(MessageId msg_id, string sc
 
         try
         {
-            var uri = new Uri(src_file.Url);
-            string source = $"// Unable to find document {src_file.SourceUri}";
+            string source = $"// Unable to find document {src_file.FileUriEscaped}";
 
             using (Stream data = await src_file.GetSourceAsync(checkHash: false, token: token))
             {
@@ -1032,7 +1031,7 @@ internal override async Task<bool> OnGetScriptSource(MessageId msg_id, string sc
             var o = JObject.FromObject(new
             {
                 source = $"// Unable to read document ({e.Message})\n" +
-                $"Local path: {src_file?.SourceUri}\n" +
+                $"Local path: {src_file?.FileUriEscaped}\n" +
                 $"SourceLink path: {src_file?.SourceLinkUri}\n",
                 from = script_id
             });

diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
@@ -630,7 +630,7 @@ internal async Task<Result> GetMethodLocation(MessageId id, JObject args, Cancel
                 return Result.Err($"Method '{typeName}:{methodName}' not found.");
             }
 
-            string src_url = methodInfo.Assembly.Sources.Single(sf => sf.SourceId == methodInfo.SourceId).Url;
+            string src_url = methodInfo.Assembly.Sources.Single(sf => sf.SourceId == methodInfo.SourceId).Url.ToString();
 
             return Result.OkFromObject(new
             {
@@ -1330,7 +1330,7 @@ private async Task OnSetEntrypointBreakpoint(SessionId sessionId, JObject args,
                     logger.LogDebug($"Could not source file {method.SourceName} for method {method.Name} in assembly {assemblyName}");
                     return;
                 }
-                string bpId = $"auto:{method.StartLocation.Line}:{method.StartLocation.Column}:{sourceFile.DotNetUrl}";
+                string bpId = $"auto:{method.StartLocation.Line}:{method.StartLocation.Column}:{sourceFile.DotNetUrlEscaped}";
                 BreakpointRequest request = new(bpId, JObject.FromObject(new
                 {
                     lineNumber = method.StartLocation.Line,
@@ -1746,8 +1746,7 @@ internal virtual async Task<bool> OnGetScriptSource(MessageId msg_id, string scr
 
             try
             {
-                var uri = new Uri(src_file.Url);
-                string source = $"// Unable to find document {src_file.SourceUri}";
+                string source = $"// Unable to find document {src_file.FileUriEscaped}";
 
                 using (Stream data = await src_file.GetSourceAsync(checkHash: false, token: token))
                 {
@@ -1764,7 +1763,7 @@ internal virtual async Task<bool> OnGetScriptSource(MessageId msg_id, string scr
                 var o = new
                 {
                     scriptSource = $"// Unable to read document ({e.Message})\n" +
-                    $"Local path: {src_file?.SourceUri}\n" +
+                    $"Local path: {src_file?.FileUriEscaped}\n" +
                     $"SourceLink path: {src_file?.SourceLinkUri}\n"
                 };
 

diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj b/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk" InitialTargets="CreateProjectWithColonInSourceName">
   <PropertyGroup>
     <TargetFramework>$(AspNetCoreAppCurrent)</TargetFramework>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
@@ -116,4 +116,34 @@
                       Overwrite="true" />
   </Target>
 
+  <Target Name="CreateProjectWithColonInSourceName"
+    Condition="!$([MSBuild]::IsOSPlatform('windows'))">
+    <PropertyGroup>
+        <CsprojContent>
+            <Project Sdk="Microsoft.NET.Sdk">
+            </Project>
+        </CsprojContent>
+        <CsContent>
+            namespace DebuggerTests
+            {
+                public class CheckColonInSourceName
+                {
+                    public static void Evaluate()
+                    {
+                        var a = 123%3B
+                    }
+                }
+            }
+        </CsContent>
+    </PropertyGroup>
+    <WriteLinesToFile
+        File="../tests/debugger-test-with-colon-in-source-name/debugger-test-with-colon-in-source-name.csproj"
+        Lines="$(CsprojContent)"
+        Overwrite="true"/>
+    <WriteLinesToFile
+        File="../tests/debugger-test-with-colon-in-source-name/test:.cs"
+        Lines="$(CsContent)"
+        Overwrite="true"/>
+  </Target>
+
 </Project>
diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs
@@ -961,20 +961,36 @@ await EvaluateAndCheck(
         [Theory]
         [InlineData(
             "DebuggerTests.CheckSpecialCharactersInPath",
-            "dotnet://debugger-test-special-char-in-path.dll/test#.cs")]
+            "dotnet://debugger-test-special-char-in-path.dll/test%23.cs",
+            "debugger-test-special-char-in-path-%23%40/test%23.cs")]
         [InlineData(
             "DebuggerTests.CheckSNonAsciiCharactersInPath",
-            "dotnet://debugger-test-special-char-in-path.dll/non-ascii-test-\u0105\u0142.cs")]
+            "dotnet://debugger-test-special-char-in-path.dll/non-ascii-test-%C4%85%C5%82%C3%85.cs",
+            "debugger-test-special-char-in-path-%23%40/non-ascii-test-%C4%85%C5%82%C3%85.cs")]
         public async Task SetBreakpointInProjectWithSpecialCharactersInPath(
-            string classWithNamespace, string expectedFileLocation)
+            string classWithNamespace, string expectedFileLocation, string expectedFileNameEscaped)
         {
             var bp = await SetBreakpointInMethod("debugger-test-special-char-in-path.dll", classWithNamespace, "Evaluate", 1);
-            await EvaluateAndCheck(
+            var ret = await EvaluateAndCheck(
                 $"window.setTimeout(function() {{ invoke_static_method ('[debugger-test-special-char-in-path] {classWithNamespace}:Evaluate'); }}, 1);",
                 expectedFileLocation,
                 bp.Value["locations"][0]["lineNumber"].Value<int>(),
                 bp.Value["locations"][0]["columnNumber"].Value<int>(),
                 $"{classWithNamespace}.Evaluate");
+            Assert.EndsWith(expectedFileNameEscaped, ret["callFrames"][0]["url"].Value<string>(), StringComparison.InvariantCulture);
+        }
+
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsLinux))]
+        public async Task SetBreakpointInProjectWithColonInSourceName()
+        {
+            var bp = await SetBreakpointInMethod("debugger-test-with-colon-in-source-name.dll", "DebuggerTests.CheckColonInSourceName", "Evaluate", 1);
+            var ret = await EvaluateAndCheck(
+                $"window.setTimeout(function() {{ invoke_static_method ('[debugger-test-with-colon-in-source-name] DebuggerTests.CheckColonInSourceName:Evaluate'); }}, 1);",
+                "dotnet://debugger-test-with-colon-in-source-name.dll/test:.cs",
+                bp.Value["locations"][0]["lineNumber"].Value<int>(),
+                bp.Value["locations"][0]["columnNumber"].Value<int>(),
+                $"DebuggerTests.CheckColonInSourceName.Evaluate");
+            Assert.EndsWith("debugger-test-with-colon-in-source-name/test:.cs", ret["callFrames"][0]["url"].Value<string>(), StringComparison.InvariantCulture);
         }
 
         [Theory]
@@ -1096,13 +1112,14 @@ await EvaluateAndCheck(
         [ConditionalFact(nameof(RunningOnChrome))]
         public async Task SetBreakpointInProjectWithChineseCharactereInPath()
         {
-            var bp = await SetBreakpointInMethod("debugger-test-chinese-char-in-path-\u3128.dll", "DebuggerTests.CheckChineseCharacterInPath", "Evaluate", 1);
-            await EvaluateAndCheck(
-                $"window.setTimeout(function() {{ invoke_static_method ('[debugger-test-chinese-char-in-path-\u3128] DebuggerTests.CheckChineseCharacterInPath:Evaluate'); }}, 1);",
-                "dotnet://debugger-test-chinese-char-in-path-\u3128.dll/test.cs",
+            var bp = await SetBreakpointInMethod("debugger-test-chinese-char-in-path-ㄨ.dll", "DebuggerTests.CheckChineseCharacterInPath", "Evaluate", 1);
+            var ret = await EvaluateAndCheck(
+                $"window.setTimeout(function() {{ invoke_static_method ('[debugger-test-chinese-char-in-path-ㄨ] DebuggerTests.CheckChineseCharacterInPath:Evaluate'); }}, 1);",
+                "dotnet://debugger-test-chinese-char-in-path-ㄨ.dll/test.cs",
                 bp.Value["locations"][0]["lineNumber"].Value<int>(),
                 bp.Value["locations"][0]["columnNumber"].Value<int>(),
                 $"DebuggerTests.CheckChineseCharacterInPath.Evaluate");
+            Assert.EndsWith("debugger-test-chinese-char-in-path-%E3%84%A8/test.cs", ret["callFrames"][0]["url"].Value<string>(), StringComparison.InvariantCulture);
         }
 
         [Fact]

diff --git a/...cial-char-in-path-#@/non-ascii-test-ął.cs → ...ial-char-in-path-#@/non-ascii-test-ąłÅ.cs b/...cial-char-in-path-#@/non-ascii-test-ął.cs → ...ial-char-in-path-#@/non-ascii-test-ąłÅ.cs
diff --git a/src/mono/wasm/debugger/tests/debugger-test/debugger-test.csproj b/src/mono/wasm/debugger/tests/debugger-test/debugger-test.csproj
@@ -28,6 +28,7 @@
     <ProjectReference Include="..\debugger-test-with-source-link\debugger-test-with-source-link.csproj" ReferenceOutputAssembly="false" Private="true" />
     <ProjectReference Include="..\debugger-test-without-debug-symbols-to-load\debugger-test-without-debug-symbols-to-load.csproj" Private="true" />
     <ProjectReference Include="..\debugger-test-with-non-user-code-class\debugger-test-with-non-user-code-class.csproj" Private="true" />
+    <ProjectReference Condition="!$([MSBuild]::IsOSPlatform('windows'))" Include="..\debugger-test-with-colon-in-source-name\debugger-test-with-colon-in-source-name.csproj" Private="true" />
     <ProjectReference Include="..\debugger-test-vb\debugger-test-vb.vbproj" Private="true" />
     <!-- loaded by *tests*, and not the test app -->
     <ProjectReference Include="..\lazy-debugger-test-embedded\lazy-debugger-test-embedded.csproj" ReferenceOutputAssembly="false" Private="true" />
@@ -63,6 +64,7 @@
       <WasmAssembliesToBundle Include="$(OutDir)\debugger-test-with-source-link.dll" />
       <WasmAssembliesToBundle Include="$(OutDir)\debugger-test-without-debug-symbols-to-load.dll" />
       <WasmAssembliesToBundle Include="$(OutDir)\debugger-test-with-non-user-code-class.dll" />
+      <WasmAssembliesToBundle Condition="!$([MSBuild]::IsOSPlatform('windows'))" Include="$(OutDir)\debugger-test-with-colon-in-source-name.dll" />
       <WasmAssembliesToBundle Include="$(OutDir)\debugger-test-vb.dll" />
       <WasmAssembliesToBundle Include="$(MicrosoftNetCoreAppRuntimePackRidDir)\lib\$(NetCoreappCurrent)\System.Runtime.InteropServices.JavaScript.dll" />
 
@@ -105,7 +107,7 @@
                                       Condition="$([System.String]::new('%(PublishItemsOutputGroupOutputs.Identity)').EndsWith('.dpdb'))" />
     </ItemGroup>
   </Target>
-  
+
   <ItemGroup>
     <ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices\gen\Microsoft.Interop.SourceGeneration\Microsoft.Interop.SourceGeneration.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
     <ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.InteropServices.JavaScript\gen\JSImportGenerator\JSImportGenerator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />