Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
carlreinke opened this issue Dec 2, 2024 · 4 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.IO untriaged New issue has not been triaged by the area owner

Comments

@carlreinke
Copy link
Contributor

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

@carlreinke carlreinke added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Dec 2, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Dec 2, 2024
Copy link
Contributor

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

@carlreinke carlreinke changed the title [API Proposal]: Path.ContainsInvalidPathChars and Path.ContainsInvalidFileNameChars [API Proposal]: Path.ContainsInvalidPathChar and Path.ContainsInvalidFileNameChar Dec 2, 2024
@Symbai
Copy link

Symbai commented Dec 2, 2024

Dupe of #25011

@MihaZupan
Copy link
Member

Related: #82606

@carlreinke
Copy link
Contributor Author

carlreinke commented Dec 2, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.IO untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

3 participants