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.

Sign up for GitHub

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

Jump to bottom

Surprising behavior on Windows with drive roots in the middle of a path #188

Open
jakepetroules opened this issue Jul 15, 2024 · 1 comment
Open

Surprising behavior on Windows with drive roots in the middle of a path #188

jakepetroules opened this issue Jul 15, 2024 · 1 comment

Comments

@jakepetroules
Copy link
Member

Consider the following (tested with v1.3.1 tag):

let fp = FilePath("Sources/E:\\source")
print(fp.isAbsolute) // false
print(fp.isRelative) // true
print(fp.root) // nil
print(Array(fp.components)) // ["Sources", "E:", "source"]

print(FilePath("Sources").appending(fp.components)) // SystemPackage/FilePathParsing.swift:351: Assertion failed
print(FilePath("Sources").pushing(fp)) // Sources\Sources\E:\source

print(FilePath("Sources").appending(FilePath.Component("C:"))) // SystemPackage/FilePathString.swift:299: Fatal error: FilePath.Component must be created from exactly one non-root component
print(FilePath("Sources").appending("C:")) // Sources

It's not clear to me that some of these assertions are expected, or even how a seemingly bogus path like Sources/E:\\source should be treated by the APIs. Should the FilePath constructor have asserted right away on this path if containing prohibited characters per https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions? Should there be an alternative validating constructor that returns nil or throws in this case?
@lorentey
Copy link
Member

Trapping in FilePath.appending does not seem right to me. Trapping in an assertion (as opposed to precondition) failure is definitely incorrect -- this operation either needs to succeed, or it must produce a guaranteed runtime error.

If Source/E:\\sources is not a valid path on Windows, then I think FilePath("Sources/E:\\source") should not successfully create a file path. FilePath.init(_: String) is a non-failable initializer, so the only way to report an error is to trap. If that isn't acceptable, then we have to make a choice between deprecating it, or coming up (and clearly documenting!) a mechanism to turn it into a valid path.

For what it's worth, Foo\0Bar is clearly an invalid path on UNIX-compatible systems, but FilePath("Foo\0Bar") appears to accept it, silently truncating the input string before the NUL character. If we take this (terrible) precedent as the intended behavior, then one potential solution on Windows would be to filter out inappropriate characters or to replace them with some bogus substitute.

The state of FilePath makes me deeply unhappy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@lorentey @jakepetroules