Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
340 changes: 0 additions & 340 deletions src/libraries/Common/tests/Tests/System/IO/PathInternal.Tests.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public static string GetFullPath(string path)

// We would ideally use realpath to do this, but it resolves symlinks, requires that the file actually exist,
// and turns it into a full path, which we only want if fullCheck is true.
string collapsedString = PathInternal.RemoveRelativeSegments(path, PathInternal.GetRootLength(path));
string collapsedString = Path.RemoveRedundantSegments(path);

Debug.Assert(collapsedString.Length < path.Length || collapsedString.ToString() == path,
"Either we've removed characters, or the string should be unmodified from the input path.");
Expand Down
21 changes: 14 additions & 7 deletions src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public static string GetFullPath(string path, string basePath)
// them properly. As such we need to manually remove segments and not use GetFullPath().

return PathInternal.IsDevice(combinedPath.AsSpan())
? PathInternal.RemoveRelativeSegments(combinedPath, PathInternal.GetRootLength(combinedPath.AsSpan()))
? RemoveRedundantSegments(combinedPath.AsSpan())
: GetFullPath(combinedPath);
}

Expand Down Expand Up @@ -213,11 +213,18 @@ public static bool IsPathRooted(ReadOnlySpan<char> path)
if (PathInternal.IsEffectivelyEmpty(path.AsSpan()))
return null;

ReadOnlySpan<char> result = GetPathRoot(path.AsSpan());
if (path!.Length == result.Length)
return PathInternal.NormalizeDirectorySeparators(path);
int rootLength = PathInternal.GetRootLength(path.AsSpan());
if (rootLength <= 0)
return null;

return PathInternal.NormalizeDirectorySeparators(result.ToString());
Span<char> destination = stackalloc char[rootLength];
// Fails when path already normalized (or empty, but won't happen here)
if (PathInternal.TryNormalizeDirectorySeparators(path.AsSpan(0, rootLength), destination, out int charsWritten))
{
return destination.Slice(0, charsWritten).ToString();
}
// Reaching here means the path was already normalized
return path!.Substring(0, rootLength);
}

/// <remarks>
Expand All @@ -228,8 +235,8 @@ public static ReadOnlySpan<char> GetPathRoot(ReadOnlySpan<char> path)
if (PathInternal.IsEffectivelyEmpty(path))
return ReadOnlySpan<char>.Empty;

int pathRoot = PathInternal.GetRootLength(path);
return pathRoot <= 0 ? ReadOnlySpan<char>.Empty : path.Slice(0, pathRoot);
int rootLength = PathInternal.GetRootLength(path);
return rootLength <= 0 ? ReadOnlySpan<char>.Empty : path.Slice(0, rootLength);
}

/// <summary>Gets whether the system is case-sensitive.</summary>
Expand Down
107 changes: 106 additions & 1 deletion src/libraries/System.Private.CoreLib/src/System/IO/Path.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,18 @@ public static partial class Path
return null;

int end = GetDirectoryNameOffset(path.AsSpan());
return end >= 0 ? PathInternal.NormalizeDirectorySeparators(path.Substring(0, end)) : null;

if (end >= 0)
{
Span<char> destination = stackalloc char[end];
// Fails when empty or already normalized
if (PathInternal.TryNormalizeDirectorySeparators(path.AsSpan(0, end), destination, out int charsWritten))
{
return destination.Slice(0, charsWritten).ToString();
}
return path.Substring(0, end);
}
return null;
}

/// <summary>
Expand Down Expand Up @@ -926,5 +937,99 @@ private static string GetRelativePath(string relativeTo, string path, StringComp
/// Returns true if the path ends in a directory separator.
/// </summary>
public static bool EndsInDirectorySeparator(string path) => PathInternal.EndsInDirectorySeparator(path);

/// <summary>
/// Removes any redundant segments found in the specified path.
/// </summary>
/// <param name="path">A string containing an absolute or relative path that may or may not contain redundant segments.</param>
/// <returns><paramref name="path" /> without any redundant segments.</returns>
[return: NotNullIfNotNull("path")]
public static string? RemoveRedundantSegments(string? path)
{
if (path == null)
return null;

if (PathInternal.IsEffectivelyEmpty(path.AsSpan()))
return string.Empty;

ValueStringBuilder sb = new ValueStringBuilder(path.Length);

ReadOnlySpan<char> root = GetPathRoot(path.AsSpan());
bool isFullyQualified = IsPathFullyQualified(path);

if (!PathInternal.TryRemoveRedundantSegments(path.AsSpan(), root.Length, isFullyQualified, ref sb))
{
sb.Dispose();
return path;
}

return sb.ToString(); // Disposes
}

/// <summary>
/// Removes any redundant segments found in the specified path.
/// </summary>
/// <param name="path">A read-only span containing an absolute or relative path that may or may not contain redundant segments.</param>
/// <returns><paramref name="path" /> without any redundant segments.</returns>
public static string RemoveRedundantSegments(ReadOnlySpan<char> path)
{
if (PathInternal.IsEffectivelyEmpty(path))
return string.Empty;

ValueStringBuilder sb = new ValueStringBuilder(path.Length);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ValueStringBuilder is meant to be used with stackallocated buffer for sizes up to certain limit. Getting a pooled buffer has too much overhead for small buffers.


string result;

ReadOnlySpan<char> root = GetPathRoot(path);
bool isFullyQualified = IsPathFullyQualified(path);

if (!PathInternal.TryRemoveRedundantSegments(path, root.Length, isFullyQualified, ref sb))
{
result = path.ToString();
sb.Dispose();
}
else
{
result = sb.ToString(); // Disposes
}

return result;
}

/// <summary>
/// Tries to remove any redundant segments found in the specified path.
/// </summary>
/// <param name="path">A read-only span containing an absolute or relative path that may or may not contain redundant segments.</param>
/// <param name="destination">When the method returns <see langword="true" />, a span containing <paramref name="path" /> without any redundant segments. When the method returns <see langword="false" />, the operation failed.</param>
/// <param name="charsWritten">When the method returns <see langword="true" />, contains the total number of characters written in <paramref name="destination" />; when the method returns <see langword="false" />, the value is zero.</param>
/// <returns><see langword="true" /> if the operation succeeds; <see langword="false" /> if the operation fails.</returns>
/// <remarks>The method returns `false` when the path is empty or when the destination span's length is less than the resulting path's length.</remarks>
public static bool TryRemoveRedundantSegments(ReadOnlySpan<char> path, Span<char> destination, out int charsWritten)
{
charsWritten = 0;

if (PathInternal.IsEffectivelyEmpty(path))
return false;

ValueStringBuilder sb = new ValueStringBuilder(path.Length);

bool result = false;

ReadOnlySpan<char> root = GetPathRoot(path);
bool isFullyQualified = IsPathFullyQualified(path);

if (PathInternal.TryRemoveRedundantSegments(path, root.Length, isFullyQualified, ref sb))
{
result = sb.TryCopyTo(destination, out charsWritten); // Disposes
}
else if (path.TryCopyTo(destination))
{
charsWritten = path.Length;
result = true;
sb.Dispose();
}

return result;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ internal static bool IsDirectorySeparator(char c)
/// Like the current NormalizePath this will not try and analyze periods/spaces within directory segments.
/// </summary>
/// <remarks>
/// The only callers that used to use Path.Normalize(fullCheck=false) were Path.GetDirectoryName() and Path.GetPathRoot(). Both usages do
/// The only callers that used to use Path.Normalize(fullCheck=false) were Path.GetDirectoryName() and Path.GetPathRoot(string). Both usages do
/// not need trimming of trailing whitespace here.
///
/// GetPathRoot() could technically skip normalizing separators after the second segment- consider as a future optimization.
Expand All @@ -341,6 +341,22 @@ internal static bool IsDirectorySeparator(char c)
if (string.IsNullOrEmpty(path))
return path;

Span<char> destination = stackalloc char[path.Length];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unbounded stackallocs are prohibited in dotnet/runtime libraries

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephentoub I don't see record of a discussion about trying to catch this with an analyzer. It's not generally possible to be sure (either way) whether it's unbounded. But an analyzer that forced a pattern of passing a constant value would be doable.

Looking at all our stackallocs, almost all of them already pass a constant directly or via a constant field or local. The few exceptions are not self evidently safe by eyeball, eg.,

        private static unsafe int EncryptDecryptHelper(OP op, ISSPIInterface secModule, SafeDeleteContext context, Span<SecurityBuffer> input, uint sequenceNumber)
        {
            Interop.SspiCli.SecBufferDesc sdcInOut = new Interop.SspiCli.SecBufferDesc(input.Length);
            Span<Interop.SspiCli.SecBuffer> unmanagedBuffer = stackalloc Interop.SspiCli.SecBuffer[input.Length];

and it would be easy for one of them to be wrong. Would it be reasonable to have an analyzer that required them to be rewritten in constant terms, or at least something the analyzer could recognize like

            Span<Interop.SspiCli.SecBuffer> unmanagedBuffer = stackalloc Interop.SspiCli.SecBuffer[Math.Min(input.Length, BufferSize)];

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to #35382

if (!TryNormalizeDirectorySeparators(path.AsSpan(), destination, out int charsWritten))
{
return path;
}

return destination.Slice(0, charsWritten).ToString();
}

internal static bool TryNormalizeDirectorySeparators(ReadOnlySpan<char> path, Span<char> destination, out int charsWritten)
{
charsWritten = 0;

if (IsEffectivelyEmpty(path))
return false;

char current;

// Make a pass to see if we need to normalize so we can potentially skip allocating
Expand All @@ -360,7 +376,7 @@ internal static bool IsDirectorySeparator(char c)
}

if (normalized)
return path;
return false;

var builder = new ValueStringBuilder(stackalloc char[MaxShortPath]);

Expand Down Expand Up @@ -391,7 +407,7 @@ internal static bool IsDirectorySeparator(char c)
builder.Append(current);
}

return builder.ToString();
return builder.TryCopyTo(destination, out charsWritten);
}

/// <summary>
Expand All @@ -411,5 +427,6 @@ internal static bool IsEffectivelyEmpty(ReadOnlySpan<char> path)
}
return true;
}

}
}
Loading