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

add a createDirectoryUnder #106

Open
joeyh opened this issue Mar 6, 2020 · 3 comments
Open

add a createDirectoryUnder #106

joeyh opened this issue Mar 6, 2020 · 3 comments
Labels
type: a-feature-request This is a request for a new feature.

Comments

@joeyh
Copy link

joeyh commented Mar 6, 2020

createDirectoryIfMissing True is a larger hammer than called for under many circumstances, but by being the only one in this module, it is in my experience very easy to reach for it without much thought. I suggest adding an alternative such as:

createDirectoryUnder
   :: FilePath --^ a top directory, which must already exist
   -> FilePath  --^ create this directory, and any missing intermediate directories before the top
   -> IO ()

Here are some use cases for createDirectoryUnder:

  • On Linux, it's common for removable drives to be mounted at eg, /media/user/foo. If a program that uses createDirectoryIfMissing True has been passed a directory under there, and is operating on it, when the removable drive gets disconnected, it will be unmounted, and the mount point removed. But, /media/user often remains, and is owned by the non-root user. So, the program may recreate /media/user/foo, and proceed with IO that was supposed to go to the removable drive. Data loss can result, things written there can prevent a later re-mount of the drive, etc.

  • A program that has generated a temporary directory in a secure manner (eg mode 700) is writing to paths under /tmp/foo.pid/. If the user or some automated process decides to clean up /tmp while the program is still running, a later call to createDirectoryIfMissing True "/tmp/foo.pid/bar/baz" will re-create /tmp/foo.pid/, but in doing so it has opened a security hole; the directory's mode is no longer secure and an attacker can read files that the program writes there. (Other security holes are also possible.)

I have an implementation of createDirectoryUnder in git-annex, but relies on some other code from it, and probably gets some of the race conditions wrong that createDirectoryIfMissing deals with, so would be at best a hint at an implementation suitable for directory.

While implementing it, I found it made sense for it to throw an exception if the top directory does not exist, but not to create the top directory. It also makes sense for it to fail if the directory to be created is not actually located under the top directory. And, either or both directories can be relative (to the current directory, not one-another!), which complicates the implementation.

@Fuuzetsu
Copy link
Member

Fuuzetsu commented Mar 9, 2020

Is this not just:

createDirectoryUnder top = withCurrentDirectory top . createDirectoryIfMissing True

I'm unsure what kind of new guarantees/races could createDirectoryUnder provide that the simple user-land code above doesn't.

@shym
Copy link

shym commented Mar 15, 2020

@Fuuzetsu withCurrentDirectory modifies the current directory of the whole process. As stated in setCurrentDirectory documentation, “in a multithreaded program, the current working directory is a global state shared among all threads of the process.”

@Fuuzetsu
Copy link
Member

@shym Ah, right, completely forgot.

I don't think what you're asking for can be reasonably atomic, just like mkdir -p is not. However, we can still implement what OP asked for in a reasonable way that doesn't suffer from the issues mentioned.

I'm on mobile but simply: splitDirectories, createDirectoryIfMissing False for each subsequent level of directory, with the very first call being in top </> firstLevel, second call being in top </> firstLevel </> secondLevel and so on. We let any exception go back to top level so that user can catch it and decide what to do.

This will never create top and if it doesn't throw an exception and terminates, we have the whole chain of directories at last call. If it throws an exception, some directory higher up may be gone such as in mounting example given in OP: and that's perfectly fine. It'd simply look something like this:

createDirectoryUnder top = void . foldM (\t d -> let p = t </> d in createDirectoryIfMissing False p >> pure p) top . splitDirectories

Is there a problem with this approach?

@Rufflewind Rufflewind added the type: a-feature-request This is a request for a new feature. label Mar 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: a-feature-request This is a request for a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants