Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 956aefe

Browse files
artkpvatsushikan
artkpv
authored and
atsushikan
committed
Command line tokenization reconciling (#25857)
* Command line tokenization reconciling Using PasteArguments from Environment to avoid code duplication while getting command line. Issue #21267. Command Line tokenization is done all over the place and inconsistently. * Fixing no-op bug. Clean up (style) * Paste arguments cross platform fix: avoiding arg[0] check on Unix. refs #25857 * Fixing code style. Adding unit tests. Refs #25857 * (UWP CoreCLR x64 build fix)
1 parent b93ec32 commit 956aefe

File tree

9 files changed

+229
-127
lines changed

9 files changed

+229
-127
lines changed

‎src/Common/src/System/PasteArguments.Unix.cs‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Collections.Generic;
6+
using System.Runtime.CompilerServices;
7+
using System.Text;
8+
9+
namespace System
10+
{
11+
internal static partial class PasteArguments
12+
{
13+
/// <summary>
14+
/// Repastes a set of arguments into a linear string that parses back into the originals under pre- or post-2008 VC parsing rules.
15+
/// On Unix: the rules for parsing the executable name (argv[0]) are ignored.
16+
/// </summary>
17+
internal static string Paste(IEnumerable<string> arguments, bool pasteFirstArgumentUsingArgV0Rules)
18+
{
19+
var stringBuilder = new StringBuilder();
20+
foreach (string argument in arguments)
21+
{
22+
AppendArgument(stringBuilder, argument);
23+
}
24+
return stringBuilder.ToString();
25+
}
26+
27+
}
28+
}

‎src/Common/src/System/PasteArguments.Windows.cs‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Collections.Generic;
6+
using System.Runtime.CompilerServices;
7+
using System.Text;
8+
9+
namespace System
10+
{
11+
internal static partial class PasteArguments
12+
{
13+
/// <summary>
14+
/// Repastes a set of arguments into a linear string that parses back into the originals under pre- or post-2008 VC parsing rules.
15+
/// The rules for parsing the executable name (argv[0]) are special, so you must indicate whether the first argument actually is argv[0].
16+
/// </summary>
17+
internal static string Paste(IEnumerable<string> arguments, bool pasteFirstArgumentUsingArgV0Rules)
18+
{
19+
var stringBuilder = new StringBuilder();
20+
21+
foreach (string argument in arguments)
22+
{
23+
if (pasteFirstArgumentUsingArgV0Rules)
24+
{
25+
pasteFirstArgumentUsingArgV0Rules = false;
26+
27+
// Special rules for argv[0]
28+
// - Backslash is a normal character.
29+
// - Quotes used to include whitespace characters.
30+
// - Parsing ends at first whitespace outside quoted region.
31+
// - No way to get a literal quote past the parser.
32+
33+
bool hasWhitespace = false;
34+
foreach (char c in argument)
35+
{
36+
if (c == Quote)
37+
{
38+
throw new ApplicationException("The argv[0] argument cannot include a double quote.");
39+
}
40+
if (char.IsWhiteSpace(c))
41+
{
42+
hasWhitespace = true;
43+
}
44+
}
45+
if (argument.Length == 0 || hasWhitespace)
46+
{
47+
stringBuilder.Append(Quote);
48+
stringBuilder.Append(argument);
49+
stringBuilder.Append(Quote);
50+
}
51+
else
52+
{
53+
stringBuilder.Append(argument);
54+
}
55+
}
56+
else
57+
{
58+
AppendArgument(stringBuilder, argument);
59+
}
60+
}
61+
62+
return stringBuilder.ToString();
63+
}
64+
65+
}
66+
}

‎src/Common/src/System/PasteArguments.cs‎

Lines changed: 54 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -7,119 +7,77 @@
77

88
namespace System
99
{
10-
internal static class PasteArguments
10+
internal static partial class PasteArguments
1111
{
12-
/// <summary>
13-
/// Repastes a set of arguments into a linear string that parses back into the originals under pre- or post-2008 VC parsing rules.
14-
/// The rules for parsing the executable name (argv[0]) are special, so you must indicate whether the first argument actually is argv[0].
15-
/// </summary>
16-
public static string Paste(IEnumerable<string> arguments, bool pasteFirstArgumentUsingArgV0Rules)
12+
private static void AppendArgument(StringBuilder stringBuilder, string argument)
1713
{
18-
var stringBuilder = new StringBuilder();
14+
if (stringBuilder.Length != 0)
15+
{
16+
stringBuilder.Append(' ');
17+
}
1918

20-
foreach (string argument in arguments)
19+
// Parsing rules for non-argv[0] arguments:
20+
// - Backslash is a normal character except followed by a quote.
21+
// - 2N backslashes followed by a quote ==> N literal backslashes followed by unescaped quote
22+
// - 2N+1 backslashes followed by a quote ==> N literal backslashes followed by a literal quote
23+
// - Parsing stops at first whitespace outside of quoted region.
24+
// - (post 2008 rule): A closing quote followed by another quote ==> literal quote, and parsing remains in quoting mode.
25+
if (argument.Length != 0 && ContainsNoWhitespaceOrQuotes(argument))
26+
{
27+
// Simple case - no quoting or changes needed.
28+
stringBuilder.Append(argument);
29+
}
30+
else
2131
{
22-
if (pasteFirstArgumentUsingArgV0Rules)
32+
stringBuilder.Append(Quote);
33+
int idx = 0;
34+
while (idx < argument.Length)
2335
{
24-
pasteFirstArgumentUsingArgV0Rules = false;
25-
26-
// Special rules for argv[0]
27-
// - Backslash is a normal character.
28-
// - Quotes used to include whitespace characters.
29-
// - Parsing ends at first whitespace outside quoted region.
30-
// - No way to get a literal quote past the parser.
31-
32-
bool hasWhitespace = false;
33-
foreach (char c in argument)
36+
char c = argument[idx++];
37+
if (c == Backslash)
3438
{
35-
if (c == Quote)
39+
int numBackSlash = 1;
40+
while (idx < argument.Length && argument[idx] == Backslash)
3641
{
37-
throw new ApplicationException("The argv[0] argument cannot include a double quote.");
42+
idx++;
43+
numBackSlash++;
3844
}
39-
if (char.IsWhiteSpace(c))
45+
46+
if (idx == argument.Length)
4047
{
41-
hasWhitespace = true;
48+
// We'll emit an end quote after this so must double the number of backslashes.
49+
stringBuilder.Append(Backslash, numBackSlash * 2);
50+
}
51+
else if (argument[idx] == Quote)
52+
{
53+
// Backslashes will be followed by a quote. Must double the number of backslashes.
54+
stringBuilder.Append(Backslash, numBackSlash * 2 + 1);
55+
stringBuilder.Append(Quote);
56+
idx++;
57+
}
58+
else
59+
{
60+
// Backslash will not be followed by a quote, so emit as normal characters.
61+
stringBuilder.Append(Backslash, numBackSlash);
4262
}
43-
}
44-
if (argument.Length == 0 || hasWhitespace)
45-
{
46-
stringBuilder.Append(Quote);
47-
stringBuilder.Append(argument);
48-
stringBuilder.Append(Quote);
49-
}
50-
else
51-
{
52-
stringBuilder.Append(argument);
53-
}
54-
}
55-
else
56-
{
57-
if (stringBuilder.Length != 0)
58-
{
59-
stringBuilder.Append(' ');
60-
}
6163

62-
// Parsing rules for non-argv[0] arguments:
63-
// - Backslash is a normal character except followed by a quote.
64-
// - 2N backslashes followed by a quote ==> N literal backslashes followed by unescaped quote
65-
// - 2N+1 backslashes followed by a quote ==> N literal backslashes followed by a literal quote
66-
// - Parsing stops at first whitespace outside of quoted region.
67-
// - (post 2008 rule): A closing quote followed by another quote ==> literal quote, and parsing remains in quoting mode.
68-
if (argument.Length != 0 && ContainsNoWhitespaceOrQuotes(argument))
69-
{
70-
// Simple case - no quoting or changes needed.
71-
stringBuilder.Append(argument);
64+
continue;
7265
}
73-
else
66+
67+
if (c == Quote)
7468
{
69+
// Escape the quote so it appears as a literal. This also guarantees that we won't end up generating a closing quote followed
70+
// by another quote (which parses differently pre-2008 vs. post-2008.)
71+
stringBuilder.Append(Backslash);
7572
stringBuilder.Append(Quote);
76-
int idx = 0;
77-
while (idx < argument.Length)
78-
{
79-
char c = argument[idx++];
80-
if (c == Backslash)
81-
{
82-
int numBackSlash = 1;
83-
while (idx < argument.Length && argument[idx] == Backslash)
84-
{
85-
idx++;
86-
numBackSlash++;
87-
}
88-
if (idx == argument.Length)
89-
{
90-
// We'll emit an end quote after this so must double the number of backslashes.
91-
stringBuilder.Append(Backslash, numBackSlash * 2);
92-
}
93-
else if (argument[idx] == Quote)
94-
{
95-
// Backslashes will be followed by a quote. Must double the number of backslashes.
96-
stringBuilder.Append(Backslash, numBackSlash * 2 + 1);
97-
stringBuilder.Append(Quote);
98-
idx++;
99-
}
100-
else
101-
{
102-
// Backslash will not be followed by a quote, so emit as normal characters.
103-
stringBuilder.Append(Backslash, numBackSlash);
104-
}
105-
continue;
106-
}
107-
if (c == Quote)
108-
{
109-
// Escape the quote so it appears as a literal. This also guarantees that we won't end up generating a closing quote followed
110-
// by another quote (which parses differently pre-2008 vs. post-2008.)
111-
stringBuilder.Append(Backslash);
112-
stringBuilder.Append(Quote);
113-
continue;
114-
}
115-
stringBuilder.Append(c);
116-
}
117-
stringBuilder.Append(Quote);
73+
continue;
11874
}
75+
76+
stringBuilder.Append(c);
11977
}
120-
}
12178

122-
return stringBuilder.ToString();
79+
stringBuilder.Append(Quote);
80+
}
12381
}
12482

12583
private static bool ContainsNoWhitespaceOrQuotes(string s)

‎src/Common/tests/Common.Tests.csproj‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,15 @@
6868
<Compile Include="$(CommonPath)\System\Security\IdentityHelper.cs">
6969
<Link>Common\System\Security\IdentityHelper.cs</Link>
7070
</Compile>
71+
<Compile Include="..\src\System\PasteArguments.cs">
72+
<Link>Common\System\PasteArguments.cs</Link>
73+
</Compile>
7174
<Compile Include="Tests\Interop\procfsTests.cs" />
7275
<Compile Include="Tests\System\AssertExtensionTests.cs" />
7376
<Compile Include="Tests\System\CharArrayHelpersTests.cs" />
7477
<Compile Include="Tests\System\IO\StringParserTests.cs" />
7578
<Compile Include="Tests\System\MarvinTests.cs" />
79+
<Compile Include="Tests\System\PasteArgumentsTests.cs" />
7680
<Compile Include="Tests\System\Security\IdentityHelperTests.cs" />
7781
<Compile Include="Tests\System\Text\ValueStringBuilderTests.cs" />
7882
<Compile Include="Tests\System\StringExtensions.Tests.cs" />
@@ -118,6 +122,9 @@
118122
<Link>Common\System\IO\Win32Marshal.cs</Link>
119123
</Compile>
120124
<Compile Include="Tests\System\IO\Win32Marshal.Tests.cs" />
125+
<Compile Include="..\src\System\PasteArguments.Windows.cs">
126+
<Link>Common\System\PasteArguments.Windows.cs</Link>
127+
</Compile>
121128
</ItemGroup>
122129
<ItemGroup Condition="'$(TargetsUnix)'=='true'">
123130
<Compile Include="..\src\System\IO\PathInternal.Unix.cs">
@@ -129,6 +136,9 @@
129136
<Compile Include="$(CommonPath)\Interop\Unix\Interop.Libraries.cs">
130137
<Link>Common\Interop\Unix\Interop.Libraries.cs</Link>
131138
</Compile>
139+
<Compile Include="..\src\System\PasteArguments.Unix.cs">
140+
<Link>Common\System\PasteArguments.Unix.cs</Link>
141+
</Compile>
132142
</ItemGroup>
133143
<ItemGroup>
134144
<Folder Include="CommonTest\System\" />

‎src/Common/tests/Tests/System/PasteArgumentsTests.cs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
using System;
2+
3+
using Xunit;
4+
5+
namespace Tests.System
6+
{
7+
public class PasteArgumentsTests
8+
{
9+
[Theory]
10+
[InlineData(@"app.exe arg1 arg2", new[] {"app.exe", "arg1", "arg2"})]
11+
[InlineData(@"""app name.exe"" arg1 arg2", new[] {"app name.exe", "arg1", "arg2"})]
12+
[InlineData(@"app.exe \\ arg2", new[] {"app.exe", @"\\", "arg2"})]
13+
[InlineData(@"app.exe ""\"""" arg2", new[] {"app.exe", @"""", "arg2"})] // literal double quotation mark character
14+
[InlineData(@"app.exe ""\\\"""" arg2", new[] {"app.exe", @"\""", "arg2"})] // 2N+1 backslashes before quote rule
15+
[InlineData(@"app.exe ""\\\\\"""" arg2", new[] {"app.exe", @"\\""", "arg2"})] // 2N backslashes before quote rule
16+
public void Pastes(string pasteExpected, string[] arguments)
17+
{
18+
Assert.Equal(pasteExpected, PasteArguments.Paste(arguments, pasteFirstArgumentUsingArgV0Rules: true));
19+
}
20+
21+
[Theory]
22+
[InlineData(@"""dir/app\""name.exe""", new[] {@"dir/app""name.exe"})] // no throwing on quotes, escaping quotes
23+
[InlineData(@"""dir/app\\\""name.exe""", new[] {@"dir/app\""name.exe"})] // escaping a backslash
24+
[InlineData(@"""dir/app\\\\\""name.exe""", new[] {@"dir/app\\""name.exe"})] // escaping backslashes
25+
[PlatformSpecific(TestPlatforms.AnyUnix)]
26+
public void Paste_Argv0Rules_Ignored_onUnix(string pasteExpected, string[] arguments)
27+
{
28+
Assert.Equal(pasteExpected, PasteArguments.Paste(arguments, pasteFirstArgumentUsingArgV0Rules: true));
29+
}
30+
31+
[Theory]
32+
[InlineData(@"dir/app""name.exe")] // throws
33+
[InlineData(@"dir/app\""name.exe")] // throws and ignores a backslash
34+
[InlineData(@"dir/app\\""name.exe")] // throws and ignores backslashes
35+
[PlatformSpecific(TestPlatforms.Windows)]
36+
public void Paste_Argv0Rules_ThrowsIfQuotes_OnWindows(string argv0)
37+
{
38+
Assert.Throws<ApplicationException>(() => PasteArguments.Paste(new []{argv0}, pasteFirstArgumentUsingArgV0Rules: true));
39+
}
40+
}
41+
}

‎src/CoreFx.Private.TestUtilities/src/CoreFx.Private.TestUtilities.csproj‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@
7070
<Compile Include="$(CommonPath)\Interop\Windows\kernel32\Interop.GetCurrentProcess_IntPtr.cs">
7171
<Link>Common\Interop\Windows\kernel32\Interop.GetCurrentProcess_IntPtr.cs</Link>
7272
</Compile>
73+
<Compile Include="..\..\Common\src\System\PasteArguments.Windows.cs">
74+
<Link>Common\System\PasteArguments.Windows.cs</Link>
75+
</Compile>
7376
<Compile Include="$(CommonPath)\Interop\Windows\advapi32\Interop.OpenProcessToken_SafeAccessTokenHandle.cs">
7477
<Link>Common\Interop\Windows\advapi32\Interop.OpenProcessToken_SafeAccessTokenHandle.cs</Link>
7578
</Compile>
@@ -97,6 +100,9 @@
97100
<Compile Include="$(CommonPath)\Interop\Unix\System.Security.Cryptography.Native\Interop.OpenSslVersion.cs">
98101
<Link>Common\Interop\Unix\System.Security.Cryptography.Native\Interop.OpenSslVersion.cs</Link>
99102
</Compile>
103+
<Compile Include="..\..\Common\src\System\PasteArguments.Unix.cs">
104+
<Link>Common\System\PasteArguments.Unix.cs</Link>
105+
</Compile>
100106
<Compile Include="System\AdminHelpers.Unix.cs" />
101107
<Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.GetEUid.cs">
102108
<Link>Common\Interop\Unix\Interop.GetEUid.cs</Link>

‎src/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,5 +54,17 @@
5454
<Name>RemoteExecutorConsoleApp</Name>
5555
</ProjectReference>
5656
</ItemGroup>
57+
<!-- WINDOWS: Shared CoreCLR and .NET Native -->
58+
<ItemGroup Condition="'$(TargetsWindows)' == 'true'">
59+
<Compile Include="..\..\Common\src\System\PasteArguments.Windows.cs">
60+
<Link>Common\System\PasteArguments.Windows.cs</Link>
61+
</Compile>
62+
</ItemGroup>
63+
<!-- UNIX -->
64+
<ItemGroup Condition=" '$(TargetsUnix)' == 'true' ">
65+
<Compile Include="..\..\Common\src\System\PasteArguments.Unix.cs">
66+
<Link>Common\System\PasteArguments.Unix.cs</Link>
67+
</Compile>
68+
</ItemGroup>
5769
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
58-
</Project>
70+
</Project>

0 commit comments

Comments
 (0)