Skip to content

libstd/io/fs.rs: avoid reading directories as files on *BSD #19303

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

Closed
wants to merge 2 commits into from

Conversation

nodakai
Copy link
Contributor

@nodakai nodakai commented Nov 25, 2014

On *BSD systems, we can open(2) a directory and directly read(2) from it due to an old tradition. We should avoid doing so by explicitly calling fstat(2) to check the type of the opened file.

Opening a directory as a module file can't always be avoided. Even when there's no "path" attribute trick involved, there can always be a directory named my_module.rs.

Incidentally, remove unnecessary mutability of &self from io::fs::File::stat().

unreachable!()

let bytes = File::open(path).and_then(|f|
if cfg!(any(target_os = "freebsd", target_os = "dragonfly")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We typically try to not house logic such as this in the compiler itself but rather put it in the standard library. As std::io::File is a cross-platform abstraction and we generally try to enforce the same behavior across platforms. If read_to_end() is only available on freebsd/dragonfly, I'd personally be a little more in favor of putting the error all the way over into std::io.

Exposing the platform-specific behavior will be done through std::sys in time as we expose the raw underlying system primitives for this sort of operation.

@nodakai nodakai force-pushed the libsyntax-reject-dirs branch from 5fe5c79 to 5d1865d Compare December 2, 2014 05:37
@nodakai
Copy link
Contributor Author

nodakai commented Dec 2, 2014

@alexcrichton I restricted the change to std::io::File. File::new() calls fstat(2) on unix other than linux or android (ie. all *BSD). If it finds that the file descriptor was for a directory, Err is returned and the descriptor is closed.

},
Err(e) => Err(e)
}
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to be somewhat nitpickity, but could you move this to std::io instead? We're generally trying to keep the std::sys bindings a low-level as possible (no worries about cross-platform errors like this) while pushing the compatibility layer to std::io instead.

@nodakai nodakai force-pushed the libsyntax-reject-dirs branch from 5d1865d to 6080b7d Compare December 2, 2014 22:20
@nodakai nodakai changed the title libsyntax/parse: avoid reading directories as files on *BSD libstd/io/fs.rs: avoid reading directories as files on *BSD Dec 2, 2014
@nodakai
Copy link
Contributor Author

nodakai commented Dec 2, 2014

@alexcrichton The check logic was moved to libstd/io/fs.rs.

@nodakai nodakai force-pushed the libsyntax-reject-dirs branch 2 times, most recently from 8c1b2fb to e7e2d7a Compare December 3, 2014 10:14
@nodakai
Copy link
Contributor Author

nodakai commented Dec 3, 2014

I fixed sys::fs::FileDesc::fstat() for Windows. I need to get mingw cross compilers...

The same goes for sys::fs::FileDesc::fstat() on Windows.

Signed-off-by: NODA, Kai <[email protected]>
On *BSD systems, we can open(2) a directory and directly read(2) from
it due to an old tradition.  We should avoid doing so by explicitly
calling fstat(2) to check the type of the opened file.

Opening a directory as a module file can't always be avoided.
Even when there's no "path" attribute trick involved, there can always
be a *directory* named "my_module.rs".

Fix rust-lang#12460

Signed-off-by: NODA, Kai <[email protected]>
@nodakai nodakai force-pushed the libsyntax-reject-dirs branch from e7e2d7a to 3980cde Compare December 4, 2014 05:34
@nodakai
Copy link
Contributor Author

nodakai commented Dec 4, 2014

A unit test on OS X failed because it expected an error message modified by my patch. I updated the unit test as well.

@nodakai
Copy link
Contributor Author

nodakai commented Dec 4, 2014

@cmr Please note that I updated my commit after @alexcrichton reviewed it.

Update: oops, it was a classical case of race between two threads

bors added a commit that referenced this pull request Dec 5, 2014
…hton

On *BSD systems, we can `open(2)` a directory and directly `read(2)` from it due to an old tradition.  We should avoid doing so by explicitly calling `fstat(2)` to check the type of the opened file.

Opening a directory as a module file can't always be avoided.  Even when there's no "path" attribute trick involved, there can always be a *directory* named `my_module.rs`.

Incidentally, remove unnecessary mutability of `&self` from `io::fs::File::stat()`.
@bors bors closed this Dec 5, 2014
@nodakai nodakai deleted the libsyntax-reject-dirs branch December 17, 2014 04:59
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 this pull request may close these issues.

3 participants