From: Thays Grazia Date: Thu, 8 Dec 2022 19:04:44 +0000 (-0300) Subject: [wasm][debugger] Revert don't need to escape special characters anymore (#78320) X-Git-Tag: accepted/tizen/unified/riscv/20231226.055536~5094 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2480f0104160f8218d55568771b57c7f9c0d02f5;p=platform%2Fupstream%2Fdotnet%2Fruntime.git [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 --- diff --git a/.gitignore b/.gitignore index fd60fdc93e4..232c8b44cde 100644 --- 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 index b2abc224dc3..1e0b19dd435 100644 --- 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(); 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 methods; private AssemblyInfo assembly; private Document doc; private DocumentHandle docHandle; - private string url; internal List 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 Methods => this.methods.Values; + + internal SourceFile(AssemblyInfo assembly, int id, DocumentHandle docHandle, Uri sourceLinkUri, string documentName) { this.methods = new Dictionary(); 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(); - 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,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 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 GetDataAsync(Uri uri, CancellationToken token) + private static async Task 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 : ""; } } diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs index ecea374bb19..6d721631b8f 100644 --- 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 }); diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 85fb14db231..bdcc6ef05f5 100644 --- 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" }; diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj b/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj index 79e5adf0bdc..8789247541c 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj +++ b/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj @@ -1,4 +1,4 @@ - + $(AspNetCoreAppCurrent) true @@ -116,4 +116,34 @@ Overwrite="true" /> + + + + + + + + namespace DebuggerTests + { + public class CheckColonInSourceName + { + public static void Evaluate() + { + var a = 123%3B + } + } + } + + + + + + diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs index 1e0fc83b939..17decf2cb0f 100644 --- 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(), bp.Value["locations"][0]["columnNumber"].Value(), $"{classWithNamespace}.Evaluate"); + Assert.EndsWith(expectedFileNameEscaped, ret["callFrames"][0]["url"].Value(), 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(), + bp.Value["locations"][0]["columnNumber"].Value(), + $"DebuggerTests.CheckColonInSourceName.Evaluate"); + Assert.EndsWith("debugger-test-with-colon-in-source-name/test:.cs", ret["callFrames"][0]["url"].Value(), 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(), bp.Value["locations"][0]["columnNumber"].Value(), $"DebuggerTests.CheckChineseCharacterInPath.Evaluate"); + Assert.EndsWith("debugger-test-chinese-char-in-path-%E3%84%A8/test.cs", ret["callFrames"][0]["url"].Value(), StringComparison.InvariantCulture); } [Fact] diff --git "a/src/mono/wasm/debugger/tests/debugger-test-special-char-in-path-#@/non-ascii-test-\304\205\305\202.cs" "b/src/mono/wasm/debugger/tests/debugger-test-special-char-in-path-#@/non-ascii-test-\304\205\305\202.cs" deleted file mode 100644 index db51e5895ab..00000000000 --- "a/src/mono/wasm/debugger/tests/debugger-test-special-char-in-path-#@/non-ascii-test-\304\205\305\202.cs" +++ /dev/null @@ -1,10 +0,0 @@ -namespace DebuggerTests -{ - public class CheckSNonAsciiCharactersInPath - { - public static void Evaluate() - { - var a = 123; - } - } -} diff --git "a/src/mono/wasm/debugger/tests/debugger-test-special-char-in-path-#@/non-ascii-test-\304\205\305\202\303\205.cs" "b/src/mono/wasm/debugger/tests/debugger-test-special-char-in-path-#@/non-ascii-test-\304\205\305\202\303\205.cs" new file mode 100644 index 00000000000..db51e5895ab --- /dev/null +++ "b/src/mono/wasm/debugger/tests/debugger-test-special-char-in-path-#@/non-ascii-test-\304\205\305\202\303\205.cs" @@ -0,0 +1,10 @@ +namespace DebuggerTests +{ + public class CheckSNonAsciiCharactersInPath + { + public static void Evaluate() + { + var a = 123; + } + } +} diff --git a/src/mono/wasm/debugger/tests/debugger-test/debugger-test.csproj b/src/mono/wasm/debugger/tests/debugger-test/debugger-test.csproj index 099f5da10b1..f312b67ebca 100644 --- 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 @@ + @@ -63,6 +64,7 @@ + @@ -105,7 +107,7 @@ Condition="$([System.String]::new('%(PublishItemsOutputGroupOutputs.Identity)').EndsWith('.dpdb'))" /> - +