11

Currently I use this function, based on JCL code, which works fine:

function IsDirectoryWriteable(const AName: string): Boolean;
var
  FileName: PWideChar;
  H: THandle;
begin
  FileName := PWideChar(IncludeTrailingPathDelimiter(AName) + 'chk.tmp');

  H := CreateFile(FileName, GENERIC_READ or GENERIC_WRITE, 0, nil,
    CREATE_NEW, FILE_ATTRIBUTE_TEMPORARY or FILE_FLAG_DELETE_ON_CLOSE, 0);

  Result := H <> INVALID_HANDLE_VALUE;

  DeleteFile(FileName);
end;

Is there anything I could improve with the flags? Can the test be done without actually creating a file? Or is this functionality even already available in one of the RTL or Jedi libraries?

mjn
  • 36,362
  • 28
  • 176
  • 378
  • 4
    Doesn't the code work for you? Is there something about this approach you do not like? It is indeed a very simple (the simplest?) way of testing for directory write access. Although I have never worked much with Windows security, I guess an alternative approach is to use the `GetFileSecurity` function. – Andreas Rejbrand Aug 30 '10 at 10:01
  • @Andreas see my edit - if I could replace this function by a call of an existing library function (maybe even with multiplatform support), this would definitely be an improvement. – mjn Aug 30 '10 at 11:41
  • I can't actually see anything there that's not in the RTL - which is the JCL call? – Mark Robinson Aug 30 '10 at 13:01
  • @Mark code is inspired by FileCreateTemp in the JclFileUstils unit, but only the MSWINDOWS part. – mjn Aug 30 '10 at 13:12
  • 2
    I've never been a fan of functions like this. What are you going to do based on whether or not the directory is writable? Why don't you just try to do it, and if it fails then handle it gracefully. – Luke Aug 30 '10 at 15:17
  • @Luke not every user has administration privileges, but the app needs to write to that directory - now guess what this application will do if the function returns false :) – mjn Aug 30 '10 at 15:57
  • What do administrative privileges have to do with anything, @Mjustin? This function doesn't account for them at all. Do you want it to? What difference would it make? Note that just because you can create new files doesn't mean you can edit (or delete) existing files. Or, you might be allowed to edit files, but not create new ones. Make sure you're testing for the actions you really want to be able to do. – Rob Kennedy Aug 30 '10 at 16:52
  • 1
    My point is that there is not really any need for generic functions like this. Instead you should just try to write whatever files you need to, and if something fails then do whatever you would do if this function returned false. For instance, what if the files you need to write to are locked by another process? This function won't catch things like that. – Luke Aug 30 '10 at 17:13
  • 1
    @Luke: when a createfile fails, it can be useful to log extra information about the environment. Part of that could be whether the destination folder was found and writeable. Another part could be (something we do regularly in our logs) to list all processes having handles to the file we cannot access... So this function does have its merits in and of it self. Also, we often use it after write failure to detect a read-only folder and attempt to change it to a writeable one so we can retry the write. Yes, we could blindly try to make it writeable, but our logs are now much more informative. – Marjan Venema Aug 30 '10 at 19:50
  • @Marjan you have hit the spot, for example it is useful for checks of correct environment configuration in a multi-user / terminalserver / fileserver setups. – mjn Aug 30 '10 at 20:45
  • 2
    @Luke: here's a possible scenario. An application that can be used as "portable", i.e. run from a removable drive. The first thing I'd want to do is check if I can write changed data and configuration. If for whatever reason I'm running from a CD-ROM, say, or a write-protected pendrive, then I would set a flag to prevent any attempts at write operations, because in this case it is not an error or cause for concern. Otherwise the user could be subjected to a whole lot of "access denied" error messages and be justifiably annoyed. – Marek Jedliński Aug 30 '10 at 22:34
  • Those are two different things; just because a directory is not writable (as determined by the code given in the question) does not mean it resides on read-only media. Using code like this is probably good enough for practical use, but if your program is widely used you will run into edge cases. – Luke Aug 31 '10 at 00:01

4 Answers4

21

Actually writing to the directory is the simpliest way to determine if the directory is writable. There are too many security options available to check individually, and even then you might miss something.

You also need to close the opened handle before calling DeleteFile(). Which you do not need to call anyway since you are using the FILE_FLAG_DELETE_ON_CLOSE flag.

BTW, there is a small bug in your code. You are creating a temporary String and assigning it to a PWideChar, but the String goes out of scope, freeing the memory, before the PWideChar is actually used. Your FileName variable should be a String instead of a PWideChar. Do the type-cast when calling CreateFile(), not before.

Try this:

function IsDirectoryWriteable(const AName: string): Boolean; 
var 
  FileName: String; 
  H: THandle; 
begin 
  FileName := IncludeTrailingPathDelimiter(AName) + 'chk.tmp'; 
  H := CreateFile(PChar(FileName), GENERIC_READ or GENERIC_WRITE, 0, nil, 
    CREATE_NEW, FILE_ATTRIBUTE_TEMPORARY or FILE_FLAG_DELETE_ON_CLOSE, 0); 
  Result := H <> INVALID_HANDLE_VALUE; 
  if Result then CloseHandle(H);
end;
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • +1 (Personally I would use `HFILE` rather than `THandle`, but of course this is only a matter of taste.) – Andreas Rejbrand Sep 01 '10 at 00:38
  • 2
    The temporary string does not go out of scope. The temporary's scope is the same as everything else in the function. It's only destroyed when the function exits, or when the temporary needs to be re-used to hold another temporary string. – Rob Kennedy Sep 02 '10 at 22:00
  • 2
    But you must probably use "random" file name, because function return FALSE, if file chk.tmp already exists in checked directory. – Peter Aug 22 '11 at 10:50
  • There's no need to use a "random" filename. If you actually want to write to a directory, check if the filename exists there. If so, try to open the file, if not, create it with the given filename. If you just want to check you can use any filename. – stackmik Apr 19 '19 at 20:40
  • @stackmik `CREATE_NEW` checks for existence. If the file already exists, `CreateFile()` fails with `ERROR_FILE_EXISTS`. – Remy Lebeau Apr 20 '19 at 14:11
  • @RemyLebeau Sorry, Remy, for the delay. What I meant is, just pick any filename and check for its existence. If it does exists, use OPEN_EXISTING, if not, CREATE_NEW. – stackmik May 25 '19 at 22:06
  • @stackmik that is not necessary, either. You can use `OPEN_ALWAYS` instead, which does that check for you – Remy Lebeau May 25 '19 at 23:16
  • @RemyLebeau You are right, Remy. I was fooled by the statement "...the last-error code is set to ERROR_ALREADY_EXISTS" and overlooked "...the function succeeds..." But still, there's not need to put effort into finding an unique filename if you use OPEN_ALWAYS. – stackmik May 26 '19 at 11:59
5

Here is my version using GetTempFileName which will attempt to create a unique temp file in the target directory:

function IsDirecoryWriteable(const AName: string): Boolean;
var
  TempFileName: array[0..MAX_PATH] of Char;
begin
  { attempt to create a temp file in the directory }
  Result := GetTempFileName(PChar(AName), '$', 0, TempFileName) <> 0;
  if Result then
    { clean up }
    Result := DeleteFile(TempFileName);
end;
kobik
  • 21,001
  • 4
  • 61
  • 121
  • 1
    I like this. But do note that being able to create a file and being able to delete it afterwards are two different things. It is possible, albeit rare, for `GetTempFileName()` to succeed and `DeleteFile()` to fail – Remy Lebeau Apr 20 '19 at 14:15
2

Andreas...

Using the security APIs to get the effective rights for a file/directory is a PIA mess and just not reliable. (I dumped all of my code for doing so in favor of just checking to see if I could write a file in the dir.)

C.f., http://www.ureader.com/msg/16591730.aspx

(I have other refs., but I'm a new user and can post only one link. Just follow along with the URLS given in the link above.)

Erik Knowles
  • 999
  • 7
  • 15
1

Surely all you need to do is verify your Access Rights to the Directory. What is wrong with this:

function IsDirectoryWriteable(aName : String);
var
  FileObject : TJwSecureFileObject;
  DesiredAccess: ACCESS_MASK;
begin
  DesiredAccess := FILE_GENERIC_WRITE;
  FileObject := TJwSecureFileObject.Create(aName);
  try
    result := FileObject.AccessCheck(DesiredAccess);
  finally
    FileObject.Free;
  end;
end;