directory icon indicating copy to clipboard operation
directory copied to clipboard

add a createDirectoryUnder

Open joeyh opened this issue 4 years ago • 3 comments

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.

joeyh avatar Mar 06 '20 17:03 joeyh

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.

Fuuzetsu avatar Mar 09 '20 00:03 Fuuzetsu

@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.”

shym avatar Mar 15 '20 18:03 shym

@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?

Fuuzetsu avatar Mar 16 '20 13:03 Fuuzetsu