Skip to content

Commit dcc959d

Browse files
committed
Fix issue with reference code lens not working with UNC paths (PowerShell#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
1 parent 641b018 commit dcc959d

File tree

2 files changed

+62
-21
lines changed

2 files changed

+62
-21
lines changed

src/PowerShellEditorServices/Workspace/Workspace.cs

+53-17
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,7 @@ private static string UnescapeDriveColon(string fileUri)
639639
/// <returns>The file system path encoded as a DocumentUri.</returns>
640640
public static string ConvertPathToDocumentUri(string path)
641641
{
642-
const string fileUriPrefix = "file:///";
642+
const string fileUriPrefix = "file:";
643643
const string untitledUriPrefix = "untitled:";
644644

645645
// If path is already in document uri form, there is nothing to convert.
@@ -650,42 +650,78 @@ public static string ConvertPathToDocumentUri(string path)
650650
}
651651

652652
string escapedPath = Uri.EscapeDataString(path);
653-
var docUriStrBld = new StringBuilder(escapedPath);
653+
654+
// Max capacity of the StringBuilder will be the current escapedPath length
655+
// plus extra chars for file:///.
656+
var docUriStrBld = new StringBuilder(escapedPath.Length + fileUriPrefix.Length + 3);
657+
docUriStrBld.Append(fileUriPrefix).Append("//");
654658

655659
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
656660
{
657-
// VSCode file URIs on Windows need the drive letter lowercase.
658-
// Search original path for colon since a char search (no string culture involved)
659-
// is faster than a string search.
661+
// VSCode file URIs on Windows need the drive letter to be lowercase. Search the
662+
// original path for colon since a char search (no string culture involved) is
663+
// faster than a string search. If found, then lowercase the associated drive letter.
660664
if (path.Contains(':'))
661665
{
662-
// Start at index 1 to avoid an index out of range check when accessing index - 1.
663-
// Also, if the colon is at index 0 there is no drive letter before it to lower case.
664-
for (int i = 1; i < docUriStrBld.Length - 2; i++)
666+
// A valid, drive-letter based path converted to URI form needs to be prefixed
667+
// with a / to indicate the path is an absolute path.
668+
docUriStrBld.Append("/");
669+
int prefixLen = docUriStrBld.Length;
670+
671+
docUriStrBld.Append(escapedPath);
672+
673+
// Uri.EscapeDataString goes a bit far, encoding \ chars. Also, VSCode wants / instead of \.
674+
docUriStrBld.Replace("%5C", "/");
675+
676+
// Find the first colon after the "file:///" prefix, skipping the first char after
677+
// the prefix since a Windows path cannot start with a colon. End the check at
678+
// less than docUriStrBld.Length - 2 since we need to look-ahead two characters.
679+
for (int i = prefixLen + 1; i < docUriStrBld.Length - 2; i++)
665680
{
666681
if ((docUriStrBld[i] == '%') && (docUriStrBld[i + 1] == '3') && (docUriStrBld[i + 2] == 'A'))
667682
{
668683
int driveLetterIndex = i - 1;
669684
char driveLetter = char.ToLowerInvariant(docUriStrBld[driveLetterIndex]);
670-
docUriStrBld.Replace(path[driveLetterIndex], driveLetter, driveLetterIndex, 1);
685+
docUriStrBld.Replace(docUriStrBld[driveLetterIndex], driveLetter, driveLetterIndex, 1);
671686
break;
672687
}
673688
}
674689
}
690+
else
691+
{
692+
// This is a Windows path without a drive specifier, must be either a relative or UNC path.
693+
int prefixLen = docUriStrBld.Length;
694+
695+
docUriStrBld.Append(escapedPath);
696+
697+
// Uri.EscapeDataString goes a bit far, encoding \ chars. Also, VSCode wants / instead of \.
698+
docUriStrBld.Replace("%5C", "/");
675699

676-
// Uri.EscapeDataString goes a bit far, encoding \ chars. Also, VSCode wants / instead of \.
677-
docUriStrBld.Replace("%5C", "/");
700+
// The proper URI form for a UNC path is file://server/share. In the case of a UNC
701+
// path, remove the path's leading // because the file:// prefix already provides it.
702+
if ((docUriStrBld.Length > prefixLen + 1) &&
703+
(docUriStrBld[prefixLen] == '/') &&
704+
(docUriStrBld[prefixLen + 1] == '/'))
705+
{
706+
docUriStrBld.Remove(prefixLen, 2);
707+
}
708+
}
678709
}
679710
else
680711
{
681-
// Because we will prefix later with file:///, remove the initial encoded / if this is an absolute path.
682-
// See https://docs.microsoft.com/en-us/dotnet/api/system.uri?view=netframework-4.7.2#implicit-file-path-support
683-
// Uri.EscapeDataString goes a bit far, encoding / chars.
684-
docUriStrBld.Replace("%2F", string.Empty, 0, 3).Replace("%2F", "/");
712+
// On non-Windows systems, append the escapedPath and undo the over-aggressive
713+
// escaping of / done by Uri.EscapeDataString.
714+
docUriStrBld.Append(escapedPath).Replace("%2F", "/");
715+
}
716+
717+
if (!Utils.IsNetCore)
718+
{
719+
// ' is not encoded by Uri.EscapeDataString in Windows PowerShell 5.x.
720+
// This is apparently a difference between .NET Framework and .NET Core.
721+
docUriStrBld.Replace("'", "%27");
685722
}
686723

687-
// ' is not always encoded. I've seen this in Windows PowerShell.
688-
return docUriStrBld.Replace("'", "%27").Insert(0, fileUriPrefix).ToString();
724+
return docUriStrBld.ToString();
689725
}
690726

691727
#endregion

test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs

+9-4
Original file line numberDiff line numberDiff line change
@@ -593,9 +593,14 @@ public void DocumentUriRetunsCorrectStringForAbsolutePath()
593593
scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion);
594594
Assert.Equal("file:///c%3A/Users/AmosBurton/projects/Rocinate/ProtoMolecule.ps1", scriptFile.DocumentUri);
595595

596-
path = @"c:\Users\BobbyDraper\projects\Rocinate\foo's_~#-[@] +,;=%.ps1";
596+
path = @"c:\Users\BobbieDraper\projects\Rocinate\foo's_~#-[@] +,;=%.ps1";
597597
scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion);
598-
Assert.Equal("file:///c%3A/Users/BobbyDraper/projects/Rocinate/foo%27s_~%23-%5B%40%5D%20%2B%2C%3B%3D%25.ps1", scriptFile.DocumentUri);
598+
Assert.Equal("file:///c%3A/Users/BobbieDraper/projects/Rocinate/foo%27s_~%23-%5B%40%5D%20%2B%2C%3B%3D%25.ps1", scriptFile.DocumentUri);
599+
600+
// Test UNC path
601+
path = @"\\ClarissaMao\projects\Rocinate\foo's_~#-[@] +,;=%.ps1";
602+
scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion);
603+
Assert.Equal("file://ClarissaMao/projects/Rocinate/foo%27s_~%23-%5B%40%5D%20%2B%2C%3B%3D%25.ps1", scriptFile.DocumentUri);
599604
}
600605
else
601606
{
@@ -604,9 +609,9 @@ public void DocumentUriRetunsCorrectStringForAbsolutePath()
604609
scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion);
605610
Assert.Equal("file:///home/AlexKamal/projects/Rocinate/ProtoMolecule.ps1", scriptFile.DocumentUri);
606611

607-
path = "/home/BobbyDraper/projects/Rocinate/foo's_~#-[@] +,;=%.ps1";
612+
path = "/home/BobbieDraper/projects/Rocinate/foo's_~#-[@] +,;=%.ps1";
608613
scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion);
609-
Assert.Equal("file:///home/BobbyDraper/projects/Rocinate/foo%27s_~%23-%5B%40%5D%20%2B%2C%3B%3D%25.ps1", scriptFile.DocumentUri);
614+
Assert.Equal("file:///home/BobbieDraper/projects/Rocinate/foo%27s_~%23-%5B%40%5D%20%2B%2C%3B%3D%25.ps1", scriptFile.DocumentUri);
610615

611616
path = "/home/NaomiNagata/projects/Rocinate/Proto:Mole:cule.ps1";
612617
scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion);

0 commit comments

Comments
 (0)