Skip to content

Commit

Permalink
fix: parameter validation in Path.GetFullPath (#334)
Browse files Browse the repository at this point in the history
Throw correct exceptions in `Path.GetFullPath` when the parameter is
`null`, empty or invalid.

---------

Co-authored-by: Valentin Breuß <v.breuss@tig.at>
  • Loading branch information
vbreuss and vbtig authored Jul 23, 2023
1 parent 64ced94 commit 3cabe7d
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ public FileAttributes Attributes
/// <inheritdoc cref="IFileSystemInfo.CreateAsSymbolicLink(string)" />
public void CreateAsSymbolicLink(string pathToTarget)
{
if (!Execute.IsWindows && string.IsNullOrWhiteSpace(FullName))
{
return;
}

FullName.EnsureValidFormat(_fileSystem);
pathToTarget.ThrowCommonExceptionsIfPathToTargetIsInvalid(_fileSystem);
if (_fileSystem.Storage.TryAddContainer(Location, InMemoryContainer.NewFile,
Expand Down
6 changes: 2 additions & 4 deletions Source/Testably.Abstractions.Testing/FileSystem/PathMock.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.IO;
using Testably.Abstractions.Helpers;
using Testably.Abstractions.Testing.Helpers;
#if FEATURE_FILESYSTEM_NET7
using System.Diagnostics.CodeAnalysis;
using Testably.Abstractions.Testing.Storage;
Expand Down Expand Up @@ -34,10 +35,7 @@ public override bool Exists([NotNullWhen(true)] string? path)
/// <inheritdoc cref="IPath.GetFullPath(string)" />
public override string GetFullPath(string path)
{
if (string.IsNullOrEmpty(path))
{
return string.Empty;
}
path.EnsureValidArgument(FileSystem, nameof(path));

return Path.GetFullPath(Path.Combine(
_fileSystem.Storage.CurrentDirectory,
Expand Down
18 changes: 17 additions & 1 deletion Source/Testably.Abstractions.Testing/Helpers/PathHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal static bool HasIllegalCharacters(this string path, IFileSystem fileSyst
() => false);
}

internal static bool IsUncPath([NotNullWhen(true)]this string? path)
internal static bool IsUncPath([NotNullWhen(true)] this string? path)
{
if (path == null)
{
Expand Down Expand Up @@ -70,6 +70,22 @@ internal static void ThrowCommonExceptionsIfPathToTargetIsInvalid(
CheckPathCharacters(pathToTarget, fileSystem, nameof(pathToTarget), -2147024713);
}

/// <summary>
/// Get the <see cref="IPath.GetFullPath(string)" /> of the <paramref name="path" />
/// if the <paramref name="path" /> is not <see langword="null" /> or white space.<br />
/// Otherwise an empty string if the <paramref name="path" /> is <see langword="null" />
/// or the <paramref name="path" /> itself.
/// </summary>
internal static string GetFullPathOrWhiteSpace(this string? path, IFileSystem fileSystem)
{
if (string.IsNullOrWhiteSpace(path))
{
return path ?? string.Empty;
}

return fileSystem.Path.GetFullPath(path);
}

private static void CheckPathArgument([NotNull] string? path, string paramName,
bool includeIsEmptyCheck)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public static MockFileSystem Changing(
changeDescription => changeDescription.Matches(
fileSystemType,
WatcherChangeTypes.Changed,
handler.FileSystem.Path.GetFullPath(path),
path.GetFullPathOrWhiteSpace(handler.FileSystem),
searchPattern,
predicate));

Expand Down Expand Up @@ -78,7 +78,7 @@ public static MockFileSystem Creating(
changeDescription => changeDescription.Matches(
fileSystemType,
WatcherChangeTypes.Created,
handler.FileSystem.Path.GetFullPath(path),
path.GetFullPathOrWhiteSpace(handler.FileSystem),
searchPattern,
predicate));

Expand Down Expand Up @@ -114,7 +114,7 @@ public static MockFileSystem Deleting(
changeDescription => changeDescription.Matches(
fileSystemType,
WatcherChangeTypes.Deleted,
handler.FileSystem.Path.GetFullPath(path),
path.GetFullPathOrWhiteSpace(handler.FileSystem),
searchPattern,
predicate));
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public static Notification.IAwaitableCallback<ChangeDescription>
changeDescription => changeDescription.Matches(
fileSystemType,
WatcherChangeTypes.Changed,
handler.FileSystem.Path.GetFullPath(path),
path.GetFullPathOrWhiteSpace(handler.FileSystem),
searchPattern,
predicate));

Expand Down Expand Up @@ -80,7 +80,7 @@ public static Notification.IAwaitableCallback<ChangeDescription>
changeDescription => changeDescription.Matches(
fileSystemType,
WatcherChangeTypes.Created,
handler.FileSystem.Path.GetFullPath(path),
path.GetFullPathOrWhiteSpace(handler.FileSystem),
searchPattern,
predicate));

Expand Down Expand Up @@ -117,7 +117,7 @@ public static Notification.IAwaitableCallback<ChangeDescription>
changeDescription => changeDescription.Matches(
fileSystemType,
WatcherChangeTypes.Deleted,
handler.FileSystem.Path.GetFullPath(path),
path.GetFullPathOrWhiteSpace(handler.FileSystem),
searchPattern,
predicate));
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.Diagnostics;
using System.IO;
using Testably.Abstractions.Testing.Helpers;

Expand Down Expand Up @@ -85,14 +84,11 @@ public override int GetHashCode()
public IStorageLocation? GetParent()
{
string? parentPath = Path.GetDirectoryName(FullPath);
if (Path.GetPathRoot(FullPath) == FullPath)
if (Path.GetPathRoot(FullPath) == FullPath || parentPath == null)
{
return null;
}

Debug.Assert(parentPath != null,
"When parentPath is null, FullPath must be null or a root path!");

return New(Drive,
parentPath,
GetFriendlyNameParent(parentPath));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ public IEnumerable<IStorageDrive> GetDrives()
drive = _fileSystem.Storage.MainDrive;
}

return InMemoryLocation.New(drive, _fileSystem.Path.GetFullPath(path), path);
return InMemoryLocation.New(drive, path.GetFullPathOrWhiteSpace(_fileSystem), path);
}

/// <inheritdoc cref="IStorage.GetOrAddDrive(string)" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;

namespace Testably.Abstractions.Tests.FileSystem.Path;

// ReSharper disable once PartialTypeWithSinglePart
public abstract partial class ExceptionTests<TFileSystem>
: FileSystemTestBase<TFileSystem>
where TFileSystem : IFileSystem
{
[SkippableTheory]
[MemberData(nameof(GetPathCallbacks), parameters: "")]
public void Operations_WhenValueIsEmpty_ShouldThrowArgumentException(
Expression<Action<IPath>> callback, string paramName,
bool ignoreParamCheck)
{
Exception? exception = Record.Exception(() =>
{
callback.Compile().Invoke(FileSystem.Path);
});

exception.Should().BeException<ArgumentException>(
hResult: -2147024809,
paramName: ignoreParamCheck || Test.IsNetFramework ? null : paramName,
because:
$"\n{callback}\n has empty parameter for '{paramName}' (ignored: {ignoreParamCheck})");
}

[SkippableTheory]
[MemberData(nameof(GetPathCallbacks), parameters: " ")]
public void Operations_WhenValueIsWhitespace_ShouldThrowArgumentException(
Expression<Action<IPath>> callback, string paramName,
bool ignoreParamCheck)
{
Skip.IfNot(Test.RunsOnWindows);

Exception? exception = Record.Exception(() =>
{
callback.Compile().Invoke(FileSystem.Path);
});

exception.Should().BeException<ArgumentException>(
hResult: -2147024809,
paramName: ignoreParamCheck || Test.IsNetFramework ? null : paramName,
because:
$"\n{callback}\n has whitespace parameter for '{paramName}' (ignored: {ignoreParamCheck})");
}

[SkippableTheory]
[MemberData(nameof(GetPathCallbacks), parameters: (string?)null)]
public void Operations_WhenValueIsNull_ShouldThrowArgumentNullException(
Expression<Action<IPath>> callback, string paramName,
bool ignoreParamCheck)
{
Exception? exception = Record.Exception(() =>
{
callback.Compile().Invoke(FileSystem.Path);
});

exception.Should().BeException<ArgumentNullException>(
paramName: ignoreParamCheck ? null : paramName,
because:
$"\n{callback}\n has `null` parameter for '{paramName}' (ignored: {ignoreParamCheck})");
}

#region Helpers

public static IEnumerable<object?[]> GetPathCallbacks(string? path)
=> GetPathCallbackTestParameters(path!)
.Where(item => item.TestType.HasFlag(path.ToTestType()))
.Select(item => new object?[]
{
item.Callback,
item.ParamName,
item.TestType.HasFlag(ExceptionTestHelper.TestTypes
.IgnoreParamNameCheck)
});

private static IEnumerable<(ExceptionTestHelper.TestTypes TestType, string? ParamName,
Expression<Action<IPath>> Callback)>
GetPathCallbackTestParameters(string value)
{
yield return (ExceptionTestHelper.TestTypes.AllExceptInvalidPath, "path", path
=> path.GetFullPath(value));
#if FEATURE_PATH_RELATIVE
yield return (ExceptionTestHelper.TestTypes.AllExceptInvalidPath, "relativeTo", path
=> path.GetRelativePath(value, "foo"));
yield return (ExceptionTestHelper.TestTypes.AllExceptInvalidPath, "path", path
=> path.GetRelativePath("foo", value));
yield return (ExceptionTestHelper.TestTypes.Null, "path", path
=> path.IsPathFullyQualified(value));
#endif
}

#endregion
}

0 comments on commit 3cabe7d

Please sign in to comment.