From 130d242ff3b32cbd788cdc8ef67616d07fd11004 Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Thu, 4 Apr 2019 20:49:40 -0600 Subject: [PATCH 1/4] Fix issue with reference code lens not working with UNC paths --- src/PowerShellEditorServices/Workspace/Workspace.cs | 12 ++++++++++-- .../Session/ScriptFileTests.cs | 13 +++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/Workspace.cs b/src/PowerShellEditorServices/Workspace/Workspace.cs index 677d94a92..01c6a77e7 100644 --- a/src/PowerShellEditorServices/Workspace/Workspace.cs +++ b/src/PowerShellEditorServices/Workspace/Workspace.cs @@ -639,7 +639,7 @@ private static string UnescapeDriveColon(string fileUri) /// The file system path encoded as a DocumentUri. public static string ConvertPathToDocumentUri(string path) { - const string fileUriPrefix = "file:///"; + const string fileUriPrefix = "file:"; const string untitledUriPrefix = "untitled:"; // If path is already in document uri form, there is nothing to convert. @@ -685,7 +685,15 @@ public static string ConvertPathToDocumentUri(string path) } // ' is not always encoded. I've seen this in Windows PowerShell. - return docUriStrBld.Replace("'", "%27").Insert(0, fileUriPrefix).ToString(); + docUriStrBld.Replace("'", "%27"); + + // Insert /// unless path is a UNC path in which case the proper URI form is file://server/share. + if ((docUriStrBld.Length < 2) || ((docUriStrBld[0] != '/') && (docUriStrBld[1] != '/'))) + { + docUriStrBld.Insert(0, "///"); + } + + return docUriStrBld.Insert(0, fileUriPrefix).ToString(); } #endregion diff --git a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs index 4961fbd5c..09c2a03bb 100644 --- a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs +++ b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs @@ -595,9 +595,14 @@ public void DocumentUriRetunsCorrectStringForAbsolutePath() scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion); Assert.Equal("file:///c%3A/Users/AmosBurton/projects/Rocinate/ProtoMolecule.ps1", scriptFile.DocumentUri); - path = @"c:\Users\BobbyDraper\projects\Rocinate\foo's_~#-[@] +,;=%.ps1"; + path = @"c:\Users\BobbieDraper\projects\Rocinate\foo's_~#-[@] +,;=%.ps1"; scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion); - Assert.Equal("file:///c%3A/Users/BobbyDraper/projects/Rocinate/foo%27s_~%23-%5B%40%5D%20%2B%2C%3B%3D%25.ps1", scriptFile.DocumentUri); + Assert.Equal("file:///c%3A/Users/BobbieDraper/projects/Rocinate/foo%27s_~%23-%5B%40%5D%20%2B%2C%3B%3D%25.ps1", scriptFile.DocumentUri); + + // Test UNC path + path = @"\\ClarissaMao\projects\Rocinate\foo's_~#-[@] +,;=%.ps1"; + scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion); + Assert.Equal("file://ClarissaMao/projects/Rocinate/foo%27s_~%23-%5B%40%5D%20%2B%2C%3B%3D%25.ps1", scriptFile.DocumentUri); } else { @@ -606,9 +611,9 @@ public void DocumentUriRetunsCorrectStringForAbsolutePath() scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion); Assert.Equal("file:///home/AlexKamal/projects/Rocinate/ProtoMolecule.ps1", scriptFile.DocumentUri); - path = "/home/BobbyDraper/projects/Rocinate/foo's_~#-[@] +,;=%.ps1"; + path = "/home/BobbieDraper/projects/Rocinate/foo's_~#-[@] +,;=%.ps1"; scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion); - Assert.Equal("file:///home/BobbyDraper/projects/Rocinate/foo%27s_~%23-%5B%40%5D%20%2B%2C%3B%3D%25.ps1", scriptFile.DocumentUri); + Assert.Equal("file:///home/BobbieDraper/projects/Rocinate/foo%27s_~%23-%5B%40%5D%20%2B%2C%3B%3D%25.ps1", scriptFile.DocumentUri); path = "/home/NaomiNagata/projects/Rocinate/Proto:Mole:cule.ps1"; scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion); From eb07203a64b868a13a44913e92acddd81e0082bf Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Sun, 7 Apr 2019 16:26:45 -0600 Subject: [PATCH 2/4] Refactor to avoid extra Insert ops. Simplifies macOS/Linux code path. --- .../Workspace/Workspace.cs | 70 +++++++++++++------ 1 file changed, 48 insertions(+), 22 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/Workspace.cs b/src/PowerShellEditorServices/Workspace/Workspace.cs index 01c6a77e7..267e16db2 100644 --- a/src/PowerShellEditorServices/Workspace/Workspace.cs +++ b/src/PowerShellEditorServices/Workspace/Workspace.cs @@ -650,50 +650,76 @@ public static string ConvertPathToDocumentUri(string path) } string escapedPath = Uri.EscapeDataString(path); - var docUriStrBld = new StringBuilder(escapedPath); + + // Max capacity of the StringBuilder will be the current escapedPath length + // plus extra chars for file:///. + var docUriStrBld = new StringBuilder(escapedPath.Length + fileUriPrefix.Length + 3); + docUriStrBld.Append(fileUriPrefix).Append("//"); if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - // VSCode file URIs on Windows need the drive letter lowercase. - // Search original path for colon since a char search (no string culture involved) - // is faster than a string search. + // VSCode file URIs on Windows need the drive letter to be lowercase. Search the + // original path for colon since a char search (no string culture involved) is + // faster than a string search. If found, then lowercase the associated drive letter. if (path.Contains(':')) { - // Start at index 1 to avoid an index out of range check when accessing index - 1. - // Also, if the colon is at index 0 there is no drive letter before it to lower case. - for (int i = 1; i < docUriStrBld.Length - 2; i++) + // A valid, drive-letter based path converted to URI form needs to be prefixed + // with a / to indicate the path is an absolute path. + docUriStrBld.Append("/"); + int prefixLen = docUriStrBld.Length; + + docUriStrBld.Append(escapedPath); + + // Uri.EscapeDataString goes a bit far, encoding \ chars. Also, VSCode wants / instead of \. + docUriStrBld.Replace("%5C", "/"); + + // Find the first colon after the "file:///" prefix, skipping the first char after + // the prefix since a Windows path cannot start with a colon. End the check at + // less than docUriStrBld.Length - 2 since we need to look-ahead two characters. + for (int i = prefixLen + 1; i < docUriStrBld.Length - 2; i++) { if ((docUriStrBld[i] == '%') && (docUriStrBld[i + 1] == '3') && (docUriStrBld[i + 2] == 'A')) { int driveLetterIndex = i - 1; char driveLetter = char.ToLowerInvariant(docUriStrBld[driveLetterIndex]); - docUriStrBld.Replace(path[driveLetterIndex], driveLetter, driveLetterIndex, 1); + docUriStrBld.Replace(docUriStrBld[driveLetterIndex], driveLetter, driveLetterIndex, 1); break; } } } + else + { + // This is a Windows path without a drive specifier, must be either a relative or UNC path. + int prefixLen = docUriStrBld.Length; + + docUriStrBld.Append(escapedPath); + + // Uri.EscapeDataString goes a bit far, encoding \ chars. Also, VSCode wants / instead of \. + docUriStrBld.Replace("%5C", "/"); - // Uri.EscapeDataString goes a bit far, encoding \ chars. Also, VSCode wants / instead of \. - docUriStrBld.Replace("%5C", "/"); + // The proper URI form for a UNC path is file://server/share. In the case of a UNC + // path, remove the path's leading // because the file:// prefix already provides it. + if ((docUriStrBld.Length > prefixLen + 1) && + (docUriStrBld[prefixLen] == '/') && + (docUriStrBld[prefixLen + 1] == '/')) + { + docUriStrBld.Remove(prefixLen, 2); + } + } } else { - // Because we will prefix later with file:///, remove the initial encoded / if this is an absolute path. - // See https://docs.microsoft.com/en-us/dotnet/api/system.uri?view=netframework-4.7.2#implicit-file-path-support - // Uri.EscapeDataString goes a bit far, encoding / chars. - docUriStrBld.Replace("%2F", string.Empty, 0, 3).Replace("%2F", "/"); + // On non-Windows system, simply append the escaped path. + docUriStrBld.Append(escapedPath); } - // ' is not always encoded. I've seen this in Windows PowerShell. +#if !CoreCLR + // ' is not encoded by Uri.EscapeDataString in Windows PowerShell 5.x. + // This is apparently a difference between .NET Framework and .NET Core. docUriStrBld.Replace("'", "%27"); +#endif - // Insert /// unless path is a UNC path in which case the proper URI form is file://server/share. - if ((docUriStrBld.Length < 2) || ((docUriStrBld[0] != '/') && (docUriStrBld[1] != '/'))) - { - docUriStrBld.Insert(0, "///"); - } - - return docUriStrBld.Insert(0, fileUriPrefix).ToString(); + return docUriStrBld.ToString(); } #endregion From 6fa07db2f9165d0e856bef613dced19c2d890831 Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Sun, 7 Apr 2019 16:48:48 -0600 Subject: [PATCH 3/4] Still need to unescape / on macOs & Linux --- src/PowerShellEditorServices/Workspace/Workspace.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/Workspace.cs b/src/PowerShellEditorServices/Workspace/Workspace.cs index 267e16db2..aac6bd995 100644 --- a/src/PowerShellEditorServices/Workspace/Workspace.cs +++ b/src/PowerShellEditorServices/Workspace/Workspace.cs @@ -709,8 +709,9 @@ public static string ConvertPathToDocumentUri(string path) } else { - // On non-Windows system, simply append the escaped path. - docUriStrBld.Append(escapedPath); + // On non-Windows systems, append the escapedPath and undo the over-aggressive + // escaping of / done by Uri.EscapeDataString. + docUriStrBld.Append(escapedPath).Replace("%2F", "/"); } #if !CoreCLR From 0a89523facc2e474d0792a34fffb73f074cdcb1a Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Tue, 9 Apr 2019 22:52:39 -0600 Subject: [PATCH 4/4] Replace compile-time check for .NET Core with runtime check --- src/PowerShellEditorServices/Workspace/Workspace.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/Workspace.cs b/src/PowerShellEditorServices/Workspace/Workspace.cs index aac6bd995..b9ca0185a 100644 --- a/src/PowerShellEditorServices/Workspace/Workspace.cs +++ b/src/PowerShellEditorServices/Workspace/Workspace.cs @@ -714,11 +714,12 @@ public static string ConvertPathToDocumentUri(string path) docUriStrBld.Append(escapedPath).Replace("%2F", "/"); } -#if !CoreCLR - // ' is not encoded by Uri.EscapeDataString in Windows PowerShell 5.x. - // This is apparently a difference between .NET Framework and .NET Core. - docUriStrBld.Replace("'", "%27"); -#endif + if (!Utils.IsNetCore) + { + // ' is not encoded by Uri.EscapeDataString in Windows PowerShell 5.x. + // This is apparently a difference between .NET Framework and .NET Core. + docUriStrBld.Replace("'", "%27"); + } return docUriStrBld.ToString(); }