[wasm][debugger] Revert don't need to escape special characters anymore (#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
2022-12-08 16:04:44 -03:00 · 2022-12-08 16:04:44 -03:00 · 2480f01041
parent 41eab87a25
commit 2480f01041
8 changed files with 110 additions and 55 deletions
--- 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
--- a/src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
+++ b/src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
@ -99,10 +99,10 @@ namespace Microsoft.WebAssembly.Diagnostics
            {
                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 @@ namespace Microsoft.WebAssembly.Diagnostics
                return false;

            Assembly = sourceFile.AssemblyName;
-            File = sourceFile.DebuggerFileName;
+            File = sourceFile.FilePath;
            Line = line.Value;
            Column = column.Value;
            return true;
@ -327,7 +327,7 @@ namespace Microsoft.WebAssembly.Diagnostics

        public SourceId SourceId => Source.SourceId;

-        public string SourceName => Source.DebuggerFileName;
+        public string SourceName => Source.FilePath;

        public string Name { get; }
        public MethodDebugInformation DebugInformation;
@ -1200,14 +1200,26 @@ namespace Microsoft.WebAssembly.Diagnostics
    }
    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;
@ -1215,19 +1227,27 @@ namespace Microsoft.WebAssembly.Diagnostics
            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.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 = this.SourceUri.ToString();
-            }
-            else
-            {
-                this.Url = DotNetUrl;
+                if (part == ":" || part == "/")
+                    builder.Append(part);
+                else
+                    builder.Append(Uri.EscapeDataString(part));
            }
+            return builder.ToString();
        }

        internal void AddMethod(MethodInfo mi)
@ -1238,20 +1258,6 @@ namespace Microsoft.WebAssembly.Diagnostics
            }
        }

-        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();
@ -1259,14 +1265,14 @@ namespace Microsoft.WebAssembly.Diagnostics
            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 @@ namespace Microsoft.WebAssembly.Diagnostics
                }
            }

-            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 @@ namespace Microsoft.WebAssembly.Diagnostics
            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 @@ namespace Microsoft.WebAssembly.Diagnostics
            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 @@ namespace Microsoft.WebAssembly.Diagnostics
            }
        }

-        public string ToUrl(SourceLocation location) => location != null ? GetFileById(location.Id).Url : "";
+        public string ToUrl(SourceLocation location) => location != null ? GetFileById(location.Id).Url.OriginalString : "";
    }
 }
--- a/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs
+++ b/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs
@ -838,7 +838,7 @@ internal sealed class FirefoxMonoProxy : MonoProxy
            isBlackBoxed = false,
            introductionType = "scriptElement",
            resourceType = "source",
-            dotNetUrl = source.DotNetUrl
+            dotNetUrl = source.DotNetUrlEscaped
        });
        JObject sourcesJObj;
        if (!string.IsNullOrEmpty(ctx.GlobalName))
@ -1014,8 +1014,7 @@ internal sealed class FirefoxMonoProxy : MonoProxy

        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 sealed class FirefoxMonoProxy : MonoProxy
            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
            });
--- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
+++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
@ -630,7 +630,7 @@ namespace Microsoft.WebAssembly.Diagnostics
                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 @@ namespace Microsoft.WebAssembly.Diagnostics
                    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 @@ namespace Microsoft.WebAssembly.Diagnostics

            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 @@ namespace Microsoft.WebAssembly.Diagnostics
                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"
                };

--- 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>
--- a/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs
+++ b/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs
@ -961,20 +961,36 @@ namespace DebuggerTests
        [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 @@ namespace DebuggerTests
        [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]
--- a/src/mono/wasm/debugger/tests/debugger-test-special-char-in-path-#@/non-ascii-test-ąłÅ.cs
+++ b/src/mono/wasm/debugger/tests/debugger-test-special-char-in-path-#@/non-ascii-test-ąłÅ.cs
--- 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" />