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

std::fs incorrect behavior when dealing with UNIX domain socket on Windows #109106

Closed
fanzeyi opened this issue Mar 13, 2023 · 2 comments · Fixed by #109235
Closed

std::fs incorrect behavior when dealing with UNIX domain socket on Windows #109106

fanzeyi opened this issue Mar 13, 2023 · 2 comments · Fixed by #109235
Assignees
Labels
A-io Area: std::io, std::fs, std::net and std::path C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@fanzeyi
Copy link

fanzeyi commented Mar 13, 2023

I tried this code:

use std::path::Path;
use std::env::args_os;

fn main() {
    let p = args_os().skip(1).next().expect("to have a path argument");
    let p: &Path = p.as_ref();

    dbg!(p.display());
    dbg!(p.exists());
    dbg!(p.metadata());
    dbg!(p.symlink_metadata());
    dbg!(p.is_file());
    dbg!(p.is_dir());
    dbg!(p.is_symlink());
}

I expected to see this happen:

The API should report the given path exists.

Instead, this happened:

When passed a path to a UNIX domain socket on Windows, I get:

[src\main.rs:22] p.display() = "<REDACTED>\\socket"
[src\main.rs:23] p.exists() = false
[src\main.rs:24] p.metadata() = Err(
    Os {
        code: 1920,
        kind: Uncategorized,
        message: "The file cannot be accessed by the system.",
    },
)
[src\main.rs:25] p.symlink_metadata() = Ok(
    Metadata {
        file_type: FileType(
            FileType {
                attributes: 1056,
                reparse_tag: 2147483683,
            },
        ),
        is_dir: false,
        is_file: true,
        permissions: Permissions(
            FilePermissions {
                attrs: 1056,
            },
        ),
        modified: Ok(
            SystemTime {
                intervals: 133232173107398736,
            },
        ),
        accessed: Ok(
            SystemTime {
                intervals: 133232173107398736,
            },
        ),
        created: Ok(
            SystemTime {
                intervals: 133172157242066860,
            },
        ),
        ..
    },
)
[src\main.rs:26] p.is_file() = false
[src\main.rs:27] p.is_dir() = false
[src\main.rs:28] p.is_symlink() = false

Meta

rustc --version --verbose:

rustc --version --verbose
rustc 1.68.0 (2c8cc3432 2023-03-06)
binary: rustc
commit-hash: 2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74
commit-date: 2023-03-06
host: x86_64-pc-windows-msvc
release: 1.68.0
LLVM version: 15.0.6

Analysis

I think this is simply an unintended consequence of supporting symlinks on Windows. It is interesting that symlink_metadata seems to be producing a correct result here (IMO this metadata should be returned by metadata()).

The differences between stat (metadata) and lstat (symlink_metadata) is the ReparsePoint flag.

pub fn stat(path: &Path) -> io::Result<FileAttr> {
metadata(path, ReparsePoint::Follow)
}
pub fn lstat(path: &Path) -> io::Result<FileAttr> {
metadata(path, ReparsePoint::Open)
}

On Windows, UNIX domain sockets are implemented as reparse points just like symlink. However Rust is only setting the FILE_FLAG_OPEN_REPARSE_POINT flag when we are testing for symlink. Without this flag CreateFileW is going to return the access error above.

I think the issue is that, the standard library didn't consider that the possibility to have other type of files that are reparse points but not symlinks.

I'm not sure what is the best fix for this, but this is kinda surprising.

@fanzeyi fanzeyi added the C-bug Category: This is a bug. label Mar 13, 2023
@fanzeyi fanzeyi changed the title std::fs incorect when dealing with UNIX domain socket on Windows std::fs incorrect behavior when dealing with UNIX domain socket on Windows Mar 13, 2023
@the8472 the8472 added O-windows Operating system: Windows A-io Area: std::io, std::fs, std::net and std::path T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 13, 2023
@ChrisDenton
Copy link
Member

This is indeed a long standing issue. You also get the same thing with app aliases, etc. The fix I've been considering would be to add a fallback to stat so that if ERROR_CANT_ACCESS_FILE is encountered then it uses lstat. Though it would then need to discard the result if the reparse point tag has the name surrogate bit set (which, incidentally, is what is_symlink tests for).

If anyone is interested in trying to implement this then feel very free. I'd be happy to provide guidance if you assign me as the reviewer of your PR.

@ChrisDenton ChrisDenton added the E-help-wanted Call for participation: Help is requested to fix this issue. label Mar 14, 2023
@chaitanyav
Copy link
Contributor

@ChrisDenton I can give this a try. @rustbot claim

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 17, 2023
fallback to lstat when stat fails on Windows

Fixes rust-lang#109106
`@ChrisDenton` please let me know if this is the expected behavior for stat on windows
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 17, 2023
fallback to lstat when stat fails on Windows

Fixes rust-lang#109106
``@ChrisDenton`` please let me know if this is the expected behavior for stat on windows
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 17, 2023
fallback to lstat when stat fails on Windows

Fixes rust-lang#109106
```@ChrisDenton``` please let me know if this is the expected behavior for stat on windows
@bors bors closed this as completed in 2a3c0e3 Mar 18, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 15, 2023
Make `try_exists` return `Ok(true)` for Windows Unix Sockets

This is a follow up to rust-lang#109106 but for[ `fs::try_exists`](https://doc.rust-lang.org/std/fs/fn.try_exists.html), which doesn't need to get the metadata of a file (which can fail even if a file exists).

`fs::try_exists` currently fails on Windows if encountering a Unix Domain Socket (UDS). This PR fixes it by checking for an error code that's returned when there's a failure to use a reparse point.

## Reparse points

A reparse point is a way to invoke a filesystem filter on a file instead of the file being opened normally. This is used to implement symbolic links (by redirecting to a different path) but also to implement other types of special files such as Unix domain sockets. If the reparse point is not a link type then opening it with `CreateFileW` may fail with `ERROR_CANT_ACCESS_FILE` because the filesystem filter does not implement that operation. This differs from resolving links which may fail with errors such as `ERROR_FILE_NOT_FOUND` or `ERROR_CANT_RESOLVE_FILENAME`.

So `ERROR_CANT_ACCESS_FILE` means that the file exists but that we can't open it normally. Still, the file does exist on the filesystem so `try_exists` should report that as `Ok(true)`.

r? libs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-io Area: std::io, std::fs, std::net and std::path C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants