Skip to content

Commit 5b6a5f9

Browse files
committed
Additional refactorings for ExecuteCommand and ExecuteCommandMonitor to handle relative paths and quotation marks.
1 parent c3b26b1 commit 5b6a5f9

File tree

8 files changed

+505
-289
lines changed

8 files changed

+505
-289
lines changed

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2.1.7
1+
2.1.8

src/VirtualClient/VirtualClient.Contracts.UnitTests/PlatformSpecificsTests.cs

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
namespace VirtualClient.Contracts
55
{
66
using System;
7+
using System.Collections;
8+
using System.Collections.Generic;
79
using System.IO;
810
using System.Reflection;
911
using System.Runtime.InteropServices;
@@ -246,6 +248,124 @@ public void TheListOfSupportedProcessorArchitecturesMatchesExpected(Architecture
246248
}
247249
}
248250

251+
[Test]
252+
[Platform(Include ="Win")]
253+
public void CanResolveRelativePathsCorrectly()
254+
{
255+
IDictionary<string, string> paths = new Dictionary<string, string>
256+
{
257+
{ @"script.exe", @"script.exe" },
258+
{ @"S:\Path\Contains\None", @"S:\Path\Contains\None" },
259+
{ @".\packages", Path.GetFullPath(@".\packages") },
260+
{ @"..\packages", Path.GetFullPath(@"..\packages") },
261+
{ @"..\..\directory\..\packages", Path.GetFullPath(@"..\..\directory\..\packages") },
262+
{ @".\scripts\script.exe", Path.GetFullPath(@".\scripts\script.exe") },
263+
{ @".\scripts\script.exe --any=option --flag", $"{Path.GetFullPath(@".\scripts\script.exe")} --any=option --flag" },
264+
{ @""".\scripts\script.exe --any=option --flag""", $"\"{Path.GetFullPath(@".\scripts\script.exe")} --any=option --flag\"" },
265+
{ @""".\scripts\script.exe"" --any=option --flag", $"\"{Path.GetFullPath(@".\scripts\script.exe")}\" --any=option --flag" }
266+
};
267+
268+
foreach (var entry in paths)
269+
{
270+
string relativePath = entry.Key;
271+
string expectedPath = entry.Value;
272+
string actualPath = PlatformSpecifics.ResolveRelativePaths(relativePath);
273+
274+
Assert.AreEqual(expectedPath, actualPath, $"Relative path did not resolve correctly: '{relativePath}'");
275+
}
276+
}
277+
278+
[Test]
279+
[Platform(Include = "Win")]
280+
public void CanResolveRelativePathsCorrectly_More_Advanced_Cases()
281+
{
282+
IDictionary<string, string> paths = new Dictionary<string, string>
283+
{
284+
{
285+
$@"cmd -c "".\scripts\script.exe --any=option --flag""",
286+
$@"cmd -c ""{Path.GetFullPath(@".\scripts\script.exe")} --any=option --flag"""
287+
},
288+
{
289+
$@"cmd -c ""'.\scripts\script.exe' --any=option --flag""",
290+
$@"cmd -c ""'{Path.GetFullPath(@".\scripts\script.exe")}' --any=option --flag"""
291+
},
292+
{
293+
$@"..\..\any\path\anycommand.exe --this=true --that=123 --log-path=.\another\path.log ..\and\one\other.thing .\",
294+
$@"{Path.GetFullPath(@"..\..\any\path\anycommand.exe")} --this=true --that=123 --log-path={Path.GetFullPath(@".\another\path.log")} {Path.GetFullPath(@"..\and\one\other.thing")} {Path.GetFullPath(@".\")}"
295+
},
296+
{
297+
$@"..\..\any\path\anycommand.exe --who=..\..\Any\Path\AnyCommand.exe",
298+
$@"{Path.GetFullPath(@"..\..\any\path\anycommand.exe")} --who={Path.GetFullPath(@"..\..\Any\Path\AnyCommand.exe")}"
299+
}
300+
};
301+
302+
foreach (var entry in paths)
303+
{
304+
string relativePath = entry.Key;
305+
string expectedPath = entry.Value;
306+
string actualPath = PlatformSpecifics.ResolveRelativePaths(relativePath);
307+
308+
Assert.AreEqual(expectedPath, actualPath, $"Relative path did not resolve correctly: '{relativePath}'");
309+
}
310+
}
311+
312+
[Test]
313+
[TestCase("anycommand", "anycommand", null)]
314+
[TestCase("anycommand ", "anycommand", null)]
315+
[TestCase("./anycommand", "./anycommand", null)]
316+
[TestCase("./anycommand --argument=value", "./anycommand", "--argument=value")]
317+
[TestCase("./anycommand --argument=value --argument2 value2", "./anycommand", "--argument=value --argument2 value2")]
318+
[TestCase("./anycommand --argument=value --argument2 value2 --flag", "./anycommand", "--argument=value --argument2 value2 --flag")]
319+
[TestCase("./anycommand --argument=value --argument2 value2 --flag ", "./anycommand", "--argument=value --argument2 value2 --flag")]
320+
[TestCase("../../anycommand --argument=value --argument2 value2 --flag ", "../../anycommand", "--argument=value --argument2 value2 --flag")]
321+
[TestCase("/home/user/anycommand", "/home/user/anycommand", null)]
322+
[TestCase("/home/user/anycommand --argument=value --argument2 value2", "/home/user/anycommand", "--argument=value --argument2 value2")]
323+
[TestCase("\"/home/user/anycommand\"", "\"/home/user/anycommand\"", null)]
324+
[TestCase("\"/home/user/dir with space/anycommand\" --argument=value --argument2 value2", "\"/home/user/dir with space/anycommand\"", "--argument=value --argument2 value2")]
325+
[TestCase("sudo anycommand", "sudo", "anycommand")]
326+
[TestCase("sudo ./anycommand", "sudo", "./anycommand")]
327+
[TestCase("sudo /home/user/anycommand", "sudo", "/home/user/anycommand")]
328+
[TestCase("sudo /home/user/anycommand --argument=value --argument2 value2", "sudo", "/home/user/anycommand --argument=value --argument2 value2")]
329+
[TestCase("sudo \"/home/user/dir with space/anycommand\"", "sudo", "\"/home/user/dir with space/anycommand\"")]
330+
[TestCase("sudo \"/home/user/dir with space/anycommand\" --argument=value --argument2 value2", "sudo", "\"/home/user/dir with space/anycommand\" --argument=value --argument2 value2")]
331+
public void CorrectlyIdentifiesThePartsOfTheCommandOnUnixSystems(string fullCommand, string expectedCommand, string expectedCommandArguments)
332+
{
333+
Assert.IsTrue(PlatformSpecifics.TryGetCommandParts(fullCommand, out string actualCommand, out string actualCommandArguments));
334+
Assert.AreEqual(expectedCommand, actualCommand);
335+
Assert.AreEqual(expectedCommandArguments, actualCommandArguments);
336+
}
337+
338+
[Test]
339+
[TestCase("bash -c \"/home/user/anyscript.sh\"", "bash", "-c \"/home/user/anyscript.sh\"")]
340+
[TestCase("bash -c \"/home/user/anyscript.sh --argument=value --argument2 value2\"", "bash", "-c \"/home/user/anyscript.sh --argument=value --argument2 value2\"")]
341+
[TestCase("bash -c \"/home/dir with space/anyscript.sh\"", "bash", "-c \"/home/dir with space/anyscript.sh\"")]
342+
[TestCase("sudo bash -c \"/home/user/anyscript.sh --argument=value --argument2 value2\"", "sudo", "bash -c \"/home/user/anyscript.sh --argument=value --argument2 value2\"")]
343+
public void CorrectlyIdentifiesThePartsOfBashCommandsOnUnixSystems(string fullCommand, string expectedCommand, string expectedCommandArguments)
344+
{
345+
Assert.IsTrue(PlatformSpecifics.TryGetCommandParts(fullCommand, out string actualCommand, out string actualCommandArguments));
346+
Assert.AreEqual(expectedCommand, actualCommand);
347+
Assert.AreEqual(expectedCommandArguments, actualCommandArguments);
348+
}
349+
350+
[Test]
351+
[TestCase("anycommand.exe", "anycommand.exe", null)]
352+
[TestCase("anycommand.exe ", "anycommand.exe", null)]
353+
[TestCase(".\\anycommand.exe", ".\\anycommand.exe", null)]
354+
[TestCase(".\\anycommand.exe --argument=value --argument2 value2", ".\\anycommand.exe", "--argument=value --argument2 value2")]
355+
[TestCase(".\\anycommand.exe --argument=value --argument2 value2 --flag", ".\\anycommand.exe", "--argument=value --argument2 value2 --flag")]
356+
[TestCase(".\\anycommand.exe --argument=value --argument2 value2 --flag ", ".\\anycommand.exe", "--argument=value --argument2 value2 --flag")]
357+
[TestCase(".\\anycommand.exe --argument=value", ".\\anycommand.exe", "--argument=value")]
358+
[TestCase("..\\..\\anycommand.exe --argument=value", "..\\..\\anycommand.exe", "--argument=value")]
359+
[TestCase("C:\\Users\\User\\anycommand.exe", "C:\\Users\\User\\anycommand.exe", null)]
360+
[TestCase("C:\\Users\\User\\anycommand.exe --argument=value --argument2 value2", "C:\\Users\\User\\anycommand.exe", "--argument=value --argument2 value2")]
361+
[TestCase("\"C:\\Users\\User\\Dir With Space\\anycommand.exe\"--argument=value --argument2 value2", "\"C:\\Users\\User\\Dir With Space\\anycommand.exe\"", "--argument=value --argument2 value2")]
362+
public void CorrectlyIdentifiesThePartsOfTheCommandOnWindowsSystems(string fullCommand, string expectedCommand, string expectedCommandArguments)
363+
{
364+
Assert.IsTrue(PlatformSpecifics.TryGetCommandParts(fullCommand, out string actualCommand, out string actualCommandArguments));
365+
Assert.AreEqual(expectedCommand, actualCommand);
366+
Assert.AreEqual(expectedCommandArguments, actualCommandArguments);
367+
}
368+
249369
[Test]
250370
[TestCase(@"\packages", @"\packages")]
251371
[TestCase(@"/packages", @"\packages")]

src/VirtualClient/VirtualClient.Contracts/PlatformSpecifics.cs

Lines changed: 106 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ public class PlatformSpecifics
3737
/// </summary>
3838
public static readonly string WinArm64 = PlatformSpecifics.GetPlatformArchitectureName(PlatformID.Win32NT, Architecture.Arm64);
3939

40+
/// <summary>
41+
/// Regular expression for identifying text containing relative paths within.
42+
/// </summary>
43+
public static readonly Regex RelativePathExpression = new Regex(@"\.{1,}[\\\/]{1,2}[\x21\x23-\x7E]*", RegexOptions.Compiled);
44+
4045
/// <summary>
4146
/// Initializes a new version of the <see cref="PlatformSpecifics"/> class.
4247
/// </summary>
@@ -161,32 +166,6 @@ public PlatformSpecifics(PlatformID platform, Architecture architecture, string
161166
/// </summary>
162167
internal static bool RunningInContainer { get; set; } = PlatformSpecifics.IsRunningInContainer();
163168

164-
/// <summary>
165-
/// Get the logged in user/username. On Windows systems, the user is discoverable even when running as Administrator.
166-
/// On Linux systems, the user can be discovered using certain environment variables when running under sudo/root.
167-
/// </summary>
168-
public string GetLoggedInUser()
169-
{
170-
string loggedInUserName = Environment.UserName;
171-
if (string.Equals(loggedInUserName, "root"))
172-
{
173-
loggedInUserName = this.GetEnvironmentVariable(EnvironmentVariable.SUDO_USER);
174-
if (string.Equals(loggedInUserName, "root") || string.IsNullOrEmpty(loggedInUserName))
175-
{
176-
loggedInUserName = this.GetEnvironmentVariable(EnvironmentVariable.VC_SUDO_USER);
177-
if (string.IsNullOrEmpty(loggedInUserName))
178-
{
179-
throw new EnvironmentSetupException(
180-
$"Unable to determine logged in username. The expected environment variables '{EnvironmentVariable.SUDO_USER}' and " +
181-
$"'{EnvironmentVariable.VC_SUDO_USER}' do not exist or are set to 'root' (i.e. potentially when running as sudo/root).",
182-
ErrorReason.EnvironmentIsInsufficent);
183-
}
184-
}
185-
}
186-
187-
return loggedInUserName;
188-
}
189-
190169
/// <summary>
191170
/// Returns the platform + architecture name used by the Virtual Client to represent a
192171
/// common supported platform and architecture (e.g. win-x64, win-arm64, linux-x64, linux-arm64);
@@ -282,6 +261,31 @@ public static bool IsRunningInContainer()
282261
return (Convert.ToBoolean(Environment.GetEnvironmentVariable("DOTNET_RUNNING_IN_CONTAINER")) == true || File.Exists("/.dockerenv"));
283262
}
284263

264+
/// <summary>
265+
/// Resolves any relative paths in the text as full paths
266+
/// (e.g. "../path/to/script.sh --log-dir=../path/to/logs" -> "/home/user/path/to/script.sh --log-dir=/home/user/path/to/logs").
267+
/// </summary>
268+
public static string ResolveRelativePaths(string text)
269+
{
270+
string resolved = text;
271+
if (!string.IsNullOrWhiteSpace(resolved))
272+
{
273+
MatchCollection relativePathMatches = PlatformSpecifics.RelativePathExpression.Matches(resolved);
274+
if (relativePathMatches?.Any() == true)
275+
{
276+
foreach (Match match in relativePathMatches)
277+
{
278+
// Ensure that relative working directory paths are fully expanded. Preserve case-sensitivity
279+
// to avoid anomalies on Linux.
280+
string relativeDirectory = match.Value;
281+
resolved = resolved.Replace(relativeDirectory, Path.GetFullPath(relativeDirectory), StringComparison.Ordinal);
282+
}
283+
}
284+
}
285+
286+
return resolved;
287+
}
288+
285289
/// <summary>
286290
/// Standardizes/normalizes the path based upon the platform/OS ensuring
287291
/// a valid path is
@@ -357,6 +361,57 @@ public static void ThrowIfNotSupported(Architecture architecture)
357361
}
358362
}
359363

364+
/// <summary>
365+
/// Returns true if the command parts can be determined and outputs the parts.
366+
/// </summary>
367+
/// <param name="fullCommand">The full comamnd and arguments (e.g. sudo lshw -c disk).</param>
368+
/// <param name="command">The command to execute.</param>
369+
/// <param name="commandArguments">The arguments to pass to the command.</param>
370+
public static bool TryGetCommandParts(string fullCommand, out string command, out string commandArguments)
371+
{
372+
fullCommand.ThrowIfNullOrWhiteSpace(nameof(fullCommand));
373+
374+
command = null;
375+
commandArguments = null;
376+
377+
string effectiveFullCommand = fullCommand.Trim();
378+
Match commandMatch = null;
379+
380+
// Note:
381+
// \x22 = quotation mark
382+
if (effectiveFullCommand.StartsWith('"'))
383+
{
384+
// e.g.
385+
// "/home/user/dir/anycommand"
386+
// "/home/user/dir with space/anycommand"
387+
//
388+
// ...directories having spaces in the name
389+
// "/home/user/dir/anycommand" --argument=value --argument2=value2
390+
// "/home/user/dir with space/anycommand" --argument=value --argument2=value2
391+
commandMatch = Regex.Match(effectiveFullCommand, @"^(\x22[\x20\x21\x23-\x7E]+\x22)", RegexOptions.IgnoreCase);
392+
}
393+
else
394+
{
395+
// e.g.
396+
// /home/user/dir/anycommand
397+
// /home/user/dir/anycommand --argument=value --argument2=value2
398+
commandMatch = Regex.Match(effectiveFullCommand, @"^([\x21\x23-\x7E]+)", RegexOptions.IgnoreCase);
399+
}
400+
401+
if (commandMatch.Success)
402+
{
403+
command = commandMatch.Groups[1].Value?.Trim();
404+
commandArguments = fullCommand.Substring(commandMatch.Groups[1].Value.Trim().Length)?.Trim();
405+
406+
if (string.IsNullOrWhiteSpace(commandArguments))
407+
{
408+
commandArguments = null;
409+
}
410+
}
411+
412+
return command != null;
413+
}
414+
360415
/// <summary>
361416
/// Combines the path segments into a valid path for the platform/OS.
362417
/// </summary>
@@ -391,6 +446,31 @@ public virtual string GetEnvironmentVariable(string variableName, EnvironmentVar
391446
return Environment.GetEnvironmentVariable(variableName, target);
392447
}
393448

449+
/// <summary>
450+
/// Get the logged in user/username. On Windows systems, the user is discoverable even when running as Administrator.
451+
/// On Linux systems, the user can be discovered using certain environment variables when running under sudo/root.
452+
/// </summary>
453+
public string GetLoggedInUser()
454+
{
455+
string loggedInUserName = Environment.UserName;
456+
if (string.Equals(loggedInUserName, "root"))
457+
{
458+
loggedInUserName = this.GetEnvironmentVariable(EnvironmentVariable.SUDO_USER);
459+
if (string.IsNullOrWhiteSpace(loggedInUserName))
460+
{
461+
loggedInUserName = this.GetEnvironmentVariable(EnvironmentVariable.VC_SUDO_USER);
462+
if (string.IsNullOrEmpty(loggedInUserName))
463+
{
464+
// When the user is "root" and running a command with "sudo", there will be
465+
// no "SUDO_USER" environment variable.
466+
return "root";
467+
}
468+
}
469+
}
470+
471+
return loggedInUserName;
472+
}
473+
394474
/// <summary>
395475
/// Combines the path segments provided with path where the log files/objects are stored.
396476
/// </summary>

src/VirtualClient/VirtualClient.Contracts/VirtualClientComponent.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ public Guid? ClientRequestId
179179
/// The content path template to use when uploading content
180180
/// to target storage resources. When not defined the default template will be used.
181181
/// </summary>
182-
public string ContentPathTemplate { get; internal set; }
182+
public string ContentPathTemplate { get; set; }
183183

184184
/// <summary>
185185
/// The CPU/processor architecture (e.g. amd64, arm).

0 commit comments

Comments
 (0)