Skip to content

Commit 1168018

Browse files
JustinGroteJustinGrote
committed
Simplify logic by using quotes up front
1 parent 2bf6b66 commit 1168018

File tree

7 files changed

+90
-191
lines changed

7 files changed

+90
-191
lines changed

‎src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,16 +156,19 @@ private static PSCommand BuildPSCommandFromArguments(string command, IReadOnlyLi
156156
return new PSCommand().AddCommand(command);
157157
}
158158

159-
// We are forced to use a hack here so that we can reuse PowerShell's parameter binding
159+
// HACK: We use AddScript instead of AddArgument/AddParameter to reuse Powershell parameter binding logic.
160+
// We quote the command parameter so that expressions can still be used in the arguments.
160161
var sb = new StringBuilder()
161162
.Append("& ")
162-
.Append(StringEscaping.SingleQuoteAndEscape(command));
163+
.Append('"')
164+
.Append(command)
165+
.Append('"');
163166

164167
foreach (string arg in arguments)
165168
{
166169
sb
167170
.Append(' ')
168-
.Append(StringEscaping.EscapePowershellArgument(arg));
171+
.Append(ArgumentEscaping.Escape(arg));
169172
}
170173

171174
return new PSCommand().AddScript(sb.ToString());

‎src/PowerShellEditorServices/Utility/ArgumentUtils.cs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
using System.Text;
2+
using System.Management.Automation.Language;
3+
4+
namespace Microsoft.PowerShell.EditorServices.Utility
5+
{
6+
internal static class ArgumentEscaping
7+
{
8+
/// <summary>
9+
/// Escape a powershell argument while still making it evaluatable in AddScript.
10+
///
11+
/// NOTE: This does not "santize" parameters, e.g. a pipe in one argument might affect another argument.
12+
/// This is intentional to give flexibility to specifying arguments.
13+
/// It also does not try to fix invalid PowerShell syntax, e.g. a single quote in a string literal.
14+
/// </summary>
15+
public static string Escape(string Arg)
16+
{
17+
// if argument is a scriptblock return as-is
18+
if (Arg.StartsWith("{") && Arg.EndsWith("}"))
19+
{
20+
return Arg;
21+
}
22+
23+
// If argument has a space enclose it in quotes unless it is already quoted
24+
if (Arg.Contains(" "))
25+
{
26+
if (Arg.StartsWith("\"") && Arg.EndsWith("\"") || Arg.StartsWith("'") && Arg.EndsWith("'"))
27+
{
28+
return Arg;
29+
}
30+
else
31+
{
32+
return "\"" + Arg + "\"";
33+
}
34+
}
35+
36+
return Arg;
37+
}
38+
}
39+
}

‎src/PowerShellEditorServices/Utility/PathUtils.cs

Lines changed: 6 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -38,48 +38,22 @@ public static string NormalizePathSeparators(string path)
3838
return string.IsNullOrWhiteSpace(path) ? path : path.Replace(AlternatePathSeparator, DefaultPathSeparator);
3939
}
4040

41-
public static string WildcardEscape(string path)
42-
{
43-
return WildcardPattern.Escape(path);
44-
}
45-
4641
/// <summary>
4742
/// Return the given path with all PowerShell globbing characters escaped,
4843
/// plus optionally the whitespace.
4944
/// </summary>
5045
/// <param name="path">The path to process.</param>
5146
/// <param name="escapeSpaces">Specify True to escape spaces in the path, otherwise False.</param>
52-
/// <returns>The path with [ and ] escaped.</returns>
47+
/// <returns>The path with *, ?, [, and ] escaped, including spaces if required</returns>
5348
internal static string WildcardEscapePath(string path, bool escapeSpaces = false)
5449
{
55-
var sb = new StringBuilder();
56-
for (int i = 0; i < path.Length; i++)
57-
{
58-
char curr = path[i];
59-
switch (curr)
60-
{
61-
// Escape '[', ']', '?' and '*' with '`'
62-
case '[':
63-
case ']':
64-
case '*':
65-
case '?':
66-
case '`':
67-
sb.Append('`').Append(curr);
68-
break;
50+
var wildcardEscapedPath = WildcardPattern.Escape(path);
6951

70-
default:
71-
// Escape whitespace if required
72-
if (escapeSpaces && char.IsWhiteSpace(curr))
73-
{
74-
sb.Append('`').Append(curr);
75-
break;
76-
}
77-
sb.Append(curr);
78-
break;
79-
}
52+
if (escapeSpaces)
53+
{
54+
wildcardEscapedPath = wildcardEscapedPath.Replace(" ", "` ");
8055
}
81-
82-
return sb.ToString();
56+
return wildcardEscapedPath;
8357
}
8458
}
8559
}

‎src/PowerShellEditorServices/Utility/StringEscaping.cs

Lines changed: 0 additions & 60 deletions
This file was deleted.

‎test/PowerShellEditorServices.Test.Shared/scriptassets/[Truly] b&d Name_4_script.ps1 renamed to ‎test/PowerShellEditorServices.Test.Shared/scriptassets/[Truly] b&d N$(){}me_4_script.ps1

File renamed without changes.

‎test/PowerShellEditorServices.Test/Session/PathEscapingTests.cs

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,12 @@
22
// Licensed under the MIT License.
33

44
using Xunit;
5-
using System.IO;
6-
using Microsoft.PowerShell.EditorServices.Services;
75
using Microsoft.PowerShell.EditorServices.Utility;
86

97
namespace Microsoft.PowerShell.EditorServices.Test.Session
108
{
119
public class PathEscapingTests
1210
{
13-
private const string ScriptAssetPath = @"..\..\..\..\PowerShellEditorServices.Test.Shared\scriptassets";
14-
1511
[Trait("Category", "PathEscaping")]
1612
[Theory]
1713
[InlineData("DebugTest.ps1", "DebugTest.ps1")]
@@ -53,70 +49,5 @@ public void CorrectlyWildcardEscapesPaths_Spaces(string unescapedPath, string es
5349
string extensionEscapedPath = PathUtils.WildcardEscapePath(unescapedPath, escapeSpaces: true);
5450
Assert.Equal(escapedPath, extensionEscapedPath);
5551
}
56-
57-
[Trait("Category", "PathEscaping")]
58-
[Theory]
59-
[InlineData("DebugTest.ps1", "'DebugTest.ps1'")]
60-
[InlineData("../../DebugTest.ps1", "'../../DebugTest.ps1'")]
61-
[InlineData("C:\\Users\\me\\Documents\\DebugTest.ps1", "'C:\\Users\\me\\Documents\\DebugTest.ps1'")]
62-
[InlineData("/home/me/Documents/weird&folder/script.ps1", "'/home/me/Documents/weird&folder/script.ps1'")]
63-
[InlineData("./path/with some/spaces", "'./path/with some/spaces'")]
64-
[InlineData("C:\\path\\with[some]brackets\\file.ps1", "'C:\\path\\with[some]brackets\\file.ps1'")]
65-
[InlineData("C:\\look\\an*\\here.ps1", "'C:\\look\\an*\\here.ps1'")]
66-
[InlineData("/Users/me/Documents/?here.ps1", "'/Users/me/Documents/?here.ps1'")]
67-
[InlineData("/Brackets [and s]paces/path.ps1", "'/Brackets [and s]paces/path.ps1'")]
68-
[InlineData("/file path/that isn't/normal/", "'/file path/that isn`'t/normal/'")]
69-
[InlineData("/CJK.chars/脚本/hello.ps1", "'/CJK.chars/脚本/hello.ps1'")]
70-
[InlineData("/CJK chars/脚本/[hello].ps1", "'/CJK chars/脚本/[hello].ps1'")]
71-
[InlineData("C:\\Animal s\\утка\\quack.ps1", "'C:\\Animal s\\утка\\quack.ps1'")]
72-
[InlineData("C:\\&nimals\\утка\\qu*ck?.ps1", "'C:\\&nimals\\утка\\qu*ck?.ps1'")]
73-
[InlineData("../../Quote'InPathTest.ps1", "'../../Quote`'InPathTest.ps1'")]
74-
75-
public void CorrectlyQuoteEscapesPaths(string unquotedPath, string expectedQuotedPath)
76-
{
77-
string extensionQuotedPath = StringEscaping.SingleQuoteAndEscape(unquotedPath).ToString();
78-
Assert.Equal(expectedQuotedPath, extensionQuotedPath);
79-
}
80-
81-
[Trait("Category", "PathEscaping")]
82-
[Theory]
83-
[InlineData("DebugTest.ps1", "'DebugTest.ps1'")]
84-
[InlineData("../../DebugTest.ps1", "'../../DebugTest.ps1'")]
85-
[InlineData("C:\\Users\\me\\Documents\\DebugTest.ps1", "'C:\\Users\\me\\Documents\\DebugTest.ps1'")]
86-
[InlineData("/home/me/Documents/weird&folder/script.ps1", "'/home/me/Documents/weird&folder/script.ps1'")]
87-
[InlineData("./path/with some/spaces", "'./path/with some/spaces'")]
88-
[InlineData("C:\\path\\with[some]brackets\\file.ps1", "'C:\\path\\with`[some`]brackets\\file.ps1'")]
89-
[InlineData("C:\\look\\an*\\here.ps1", "'C:\\look\\an`*\\here.ps1'")]
90-
[InlineData("/Users/me/Documents/?here.ps1", "'/Users/me/Documents/`?here.ps1'")]
91-
[InlineData("/Brackets [and s]paces/path.ps1", "'/Brackets `[and s`]paces/path.ps1'")]
92-
[InlineData("/file path/that isn't/normal/", "'/file path/that isn`'t/normal/'")]
93-
[InlineData("/CJK.chars/脚本/hello.ps1", "'/CJK.chars/脚本/hello.ps1'")]
94-
[InlineData("/CJK chars/脚本/[hello].ps1", "'/CJK chars/脚本/`[hello`].ps1'")]
95-
[InlineData("C:\\Animal s\\утка\\quack.ps1", "'C:\\Animal s\\утка\\quack.ps1'")]
96-
[InlineData("C:\\&nimals\\утка\\qu*ck?.ps1", "'C:\\&nimals\\утка\\qu`*ck`?.ps1'")]
97-
public void CorrectlyFullyEscapesPaths(string unescapedPath, string escapedPath)
98-
{
99-
string extensionEscapedPath = StringEscaping.SingleQuoteAndEscape(PathUtils.WildcardEscapePath(unescapedPath)).ToString();
100-
Assert.Equal(escapedPath, extensionEscapedPath);
101-
}
102-
103-
[Trait("Category", "PathEscaping")]
104-
[Theory]
105-
[InlineData("NormalScript.ps1")]
106-
[InlineData("Bad&name4script.ps1")]
107-
[InlineData("[Truly] b&d Name_4_script.ps1")]
108-
public void CanDotSourcePath(string rawFileName)
109-
{
110-
string fullPath = Path.Combine(ScriptAssetPath, rawFileName);
111-
string quotedPath = StringEscaping.SingleQuoteAndEscape(fullPath).ToString();
112-
113-
var psCommand = new System.Management.Automation.PSCommand().AddScript($". {quotedPath}");
114-
115-
using (var pwsh = System.Management.Automation.PowerShell.Create())
116-
{
117-
pwsh.Commands = psCommand;
118-
pwsh.Invoke();
119-
}
120-
}
12152
}
12253
}

‎test/PowerShellEditorServices.Test/Utility/ArgumentEscapingTests.cs

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using Xunit;
22
using Microsoft.PowerShell.EditorServices.Utility;
3+
using System.IO;
34
using System.Management.Automation;
45
using System.Linq;
56

@@ -9,46 +10,57 @@ public class ArgumentEscapingTests
910
{
1011
[Trait("Category", "ArgumentEscaping")]
1112
[Theory]
12-
[InlineData("/path/to/file", "/path/to/file")]
13-
[InlineData("'/path/to/file'", "'/path/to/file'")]
14-
[InlineData("not|allowed|pipeline", "'not|allowed|pipeline'")]
15-
[InlineData("doublequote\"inmiddle", "'doublequote\"inmiddle'")]
16-
[InlineData("am&persand", "'am&persand'")]
17-
[InlineData("semicolon;", "'semicolon;'")]
18-
[InlineData(":colon", "':colon'")]
19-
[InlineData(" has space s", "' has space s'")]
20-
[InlineData("[brackets]areOK", "[brackets]areOK")]
21-
[InlineData("$(expressionsAreOK)", "$(expressionsAreOK)")]
22-
[InlineData("{scriptBlocksAreOK}", "{scriptBlocksAreOK}")]
23-
[InlineData("'quote ' in middle of argument'", "'quote `' in middle of argument'")]
24-
13+
[InlineData(" has spaces", "\" has spaces\"")]
14+
[InlineData("-Parameter", "-Parameter")]
15+
[InlineData("' single quote left alone'", "' single quote left alone'")]
16+
[InlineData("\"double quote left alone\"", "\"double quote left alone\"")]
17+
[InlineData("/path/to/fi le", "\"/path/to/fi le\"")]
18+
[InlineData("'/path/to/fi le'", "'/path/to/fi le'")]
19+
[InlineData("|pipeline", "|pipeline")]
20+
[InlineData("am&pe rsand", "\"am&pe rsand\"")]
21+
[InlineData("semicolon ;", "\"semicolon ;\"")]
22+
[InlineData(": colon", "\": colon\"")]
23+
[InlineData("$(expressions should be quoted)", "\"$(expressions should be quoted)\"")]
24+
[InlineData("{scriptBlocks should not have escaped-spaces}", "{scriptBlocks should not have escaped-spaces}")]
25+
[InlineData("-Parameter test", "\"-Parameter test\"")] //This is invalid, but should be obvious enough looking at the PSIC invocation
2526
public void CorrectlyEscapesPowerShellArguments(string Arg, string expectedArg)
2627
{
27-
string quotedArg = StringEscaping.EscapePowershellArgument(Arg);
28+
string quotedArg = ArgumentEscaping.Escape(Arg);
2829
Assert.Equal(expectedArg, quotedArg);
2930
}
3031

3132
[Trait("Category", "ArgumentEscaping")]
3233
[Theory]
33-
[InlineData("/path/to/file", "/path/to/file")]
34-
[InlineData("'/path/to/file'", "/path/to/file")]
35-
[InlineData("not|allowed|pipeline", "not|allowed|pipeline")]
36-
[InlineData("doublequote\"inmiddle", "doublequote\"inmiddle")]
37-
[InlineData("am&persand", "am&persand")]
38-
[InlineData("semicolon;", "semicolon;")]
39-
[InlineData(":colon", ":colon")]
40-
[InlineData(" has space s", " has space s")]
41-
[InlineData("[brackets]areOK", "[brackets]areOK")]
42-
[InlineData("$(echo 'expressionsAreOK')", "expressionsAreOK")]
43-
// [InlineData("{scriptBlocksAreOK}", "{scriptBlocksAreOK}")]
44-
public void CanEvaluateArgumentsSafely(string Arg, string expectedOutput)
34+
[InlineData("/path/t o/file", "/path/t o/file")]
35+
[InlineData("/path/with/$(echo 'expression')inline", "/path/with/expressioninline")]
36+
[InlineData("/path/with/$(echo 'expression') inline", "/path/with/expression inline")]
37+
[InlineData("am&per sand", "am&per sand")]
38+
[InlineData("'inner\"\"quotes'", "inner\"\"quotes")]
39+
public void CanEvaluateArguments(string Arg, string expectedOutput)
4540
{
46-
var escapedArg = StringEscaping.EscapePowershellArgument(Arg);
41+
var escapedArg = ArgumentEscaping.Escape(Arg);
4742
var psCommand = new PSCommand().AddScript($"& Write-Output {escapedArg}");
4843
using var pwsh = System.Management.Automation.PowerShell.Create();
4944
pwsh.Commands = psCommand;
5045
var scriptOutput = pwsh.Invoke<string>().First();
5146
Assert.Equal(expectedOutput, scriptOutput);
5247
}
48+
49+
[Trait("Category", "ArgumentEscaping")]
50+
[Theory]
51+
[InlineData("NormalScript.ps1")]
52+
[InlineData("Bad&name4script.ps1")]
53+
[InlineData("[Truly] b&d Name_4_script.ps1")]
54+
public void CanDotSourcePath(string rawFileName)
55+
{
56+
var ScriptAssetPath = @"..\..\..\..\PowerShellEditorServices.Test.Shared\scriptassets";
57+
var fullPath = Path.Combine(ScriptAssetPath, rawFileName);
58+
var escapedPath = PathUtils.WildcardEscapePath(fullPath).ToString();
59+
var psCommand = new PSCommand().AddScript($"& \"{escapedPath}\"");
60+
61+
using var pwsh = System.Management.Automation.PowerShell.Create();
62+
pwsh.Commands = psCommand;
63+
pwsh.Invoke();
64+
}
5365
}
5466
}

0 commit comments

Comments
 (0)