From dcc959ddcb1e9ae3cc3f428dc238805f9ce43308 Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Tue, 9 Apr 2019 23:13:51 -0600 Subject: [PATCH] Fix issue with reference code lens not working with UNC paths (#908) * Fix issue with reference code lens not working with UNC paths * Refactor to avoid extra Insert ops. Simplifies macOS/Linux code path. * Still need to unescape / on macOs & Linux * Replace compile-time check for .NET Core with runtime check --- .../Workspace/Workspace.cs | 70 ++++++++++++++----- .../Session/ScriptFileTests.cs | 13 ++-- 2 files changed, 62 insertions(+), 21 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/Workspace.cs b/src/PowerShellEditorServices/Workspace/Workspace.cs index 677d94a92..b9ca0185a 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. @@ -650,42 +650,78 @@ 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 systems, append the escapedPath and undo the over-aggressive + // escaping of / done by Uri.EscapeDataString. + docUriStrBld.Append(escapedPath).Replace("%2F", "/"); + } + + 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"); } - // ' is not always encoded. I've seen this in Windows PowerShell. - return docUriStrBld.Replace("'", "%27").Insert(0, fileUriPrefix).ToString(); + return docUriStrBld.ToString(); } #endregion diff --git a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs index 31543b1d5..2e1cb987d 100644 --- a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs +++ b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs @@ -593,9 +593,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 { @@ -604,9 +609,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);