Skip to content

Dir::open("a_file") succeeds #27

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

Open
pjw91 opened this issue Nov 23, 2019 · 2 comments · May be fixed by #43
Open

Dir::open("a_file") succeeds #27

pjw91 opened this issue Nov 23, 2019 · 2 comments · May be fixed by #43

Comments

@pjw91
Copy link

pjw91 commented Nov 23, 2019

Should Dir::open succeeds when it's given a file?

@tailhook
Copy link
Owner

By looking at history, it looks like it might be a deliberate decision at some point (there is a test which is reversed). But unfortunately it was early on for the project, there is no issue to cite for a motivation. I need to think about it more.

In case we want to fix it, what's your use case? Just to make sure I understand the problem.

@pjw91
Copy link
Author

pjw91 commented Nov 23, 2019

When I first looked at the code (0.1.18, via docs.rs), I noticed that the O_PATH is used instead of O_DIRECTORY (on Linux).

Then I noticed that it was changed to O_DIRECTORY then to O_PATH again (on master branch, by #23).

The flag O_DIRECTORY is specified in POSIX.1-2008 and added in Linux 2.1.126, but not in POSIX.1-2001, just like O_CLOEXEC (added in Linux 2.6.23), and O_NOFOLLOW. [ref]

Thus, if O_CLOEXEC is available, O_DIRECTORY is available, too.

Moreover, IMHO, on those platforms where O_DIRECTORY is unavailable, this crate should check if the given path is a directory or not. Otherwise the behavior of this crate is inconsistent across different platforms.

cehteh added a commit to cehteh/openat that referenced this issue Apr 28, 2021
This should resolve tailhook#27. It does not fix it by adding a expensive stat() check on opening, but by
giving the opportunity to explicitly check a Dir handle. When really required a programmer can
enforce consistency between platforms by using 'is_dir()'. Failing to do so won't cause any
harm because improper handles (on platforms without O_DIRECTORY) would report errors at later
time.

This also removes the test_open_file() test from the testsuite, as it depended on presence of
O_DIRECTORY which makes he testsuite non deterministic testing only where its oblivious that
the OS wouldn't open files as directories.
This was referenced Apr 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants