dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.47k stars 4.77k forks source link

[API Proposal]: Path.ContainsInvalidPathChar and Path.ContainsInvalidFileNameChar #110301

Open carlreinke opened 1 day ago

carlreinke commented 1 day ago

Background and motivation

Path.GetInvalidFileNameChars() and Path.GetInvalidPathChars() each make a copy and aren't even very convenient for one of the most common operation you want to do with them: ask if your string contains one.

API Proposal

namespace System.IO;

public partial static class Path
{
    public static bool ContainsInvalidPathChar(ReadOnlySpan<char> path);
    public static bool ContainsInvalidFileNameChar(ReadOnlySpan<char> fileName);
}

API Usage

if (Path.ContainsInvalidPathNameChar(pipeName) || pipeName.EndsWith(Path.DirectorySeparatorChar))
    throw new PlatformNotSupportedException(SR.PlatformNotSupported_InvalidPipeNameChars);
if (Path.ContainsInvalidFileNameChar(pipeName))
{
    throw new PlatformNotSupportedException(SR.PlatformNotSupported_InvalidPipeNameChars);
}

(Instead of caching in a static and using .AsSpan().ContainsAny(...).)

Alternative Designs

Maybe expose SearchValues. (Could do this in addition to the above proposal.)

namespace System.IO;

public partial static class Path
{
    public static SearchValues<char> InvalidPathCharSearchValues { get; }
    public static SearchValues<char> InvalidFileNameCharSearchValues { get; }
}

Risks

No response

dotnet-policy-service[bot] commented 1 day ago

Tagging subscribers to this area: @dotnet/area-system-io See info in area-owners.md if you want to be subscribed.

Symbai commented 21 hours ago

Dupe of https://github.com/dotnet/runtime/issues/25011

MihaZupan commented 18 hours ago

Related: #82606

carlreinke commented 16 hours ago

Regarding #25011, the problem with this API is that a filename being "portable" doesn't make it a valid filename, and seems likely that the users of such an API will have reasonable expectations of a guarantee that the API cannot provide. (Though it might work reasonably well in practice.)

I expect that users of GetInvalidFileNameChars() have a similar expectation that removing/replacing the invalid chars will produce a valid filename. ContainsInvalidFileNameChar() would have less of a problem in this regard in that it cannot be used to try to transform an invalid filename into a valid filename. It does still have the problem that users might expect that the filename is valid when false is returned, but it's no worse than GetInvalidFileNameChars().

Regarding #82606, the problem with this API is that replacing all the invalid chars in a path/filename doesn't necessarily make a valid path/filename. (To be fair, I've written the exact code that the proposed API would replace, and it works reasonably well in practice, but it's hardly robust.)

GetInvalidPathChars()/GetInvalidFileNameChars() only tells you what the OS will definitely not accept in a path/filename; it doesn't tell you what the OS/filesystem will accept. The only way to actually know if the path/filename is valid is to try to open/create it.

This proposal doesn't add any new capability, it only makes it cheaper/simpler to use the existing capability for the only thing that it can reliably be used for — a quick check of whether the OS is definitely not going to accept the path/filename.