6

I have Java code doing the following:

  1. Create a temporary empty file with ZIP extension using File.createTempFile()
  2. Delete it with File.delete() (we only really wanted it to generate a temp file name)
  3. Copy a "template" ZIP file to the same path with com.google.commons.io.ByteStreams.copy() using a new OutputSupplier given the same filename
  4. Modify the ZIP archive (remove a directory) using TrueZIP 7.4.3

On a specific system, step 4 fails consistently with FsReadOnlyArchiveFileSystemException - "This is a read-only archive file system!" (see http://java.net/projects/truezip/lists/users/archive/2011-05/message/9)

Debugging the TrueZIP code, I noticed the following:

  • There is no open file handle on this file between any of the steps above, and specifically not before step 4
  • Checking the same file with File.canWrite() rather than NIO returns at the exact same timing (using a debugger), it shows that it is writable

Here is what you see in the debugger expressions list:

fn => "C:/myworkdir/temp/myfile4088293380313057223tmp.zip"
java.nio.file.Files.isWritable(java.nio.file.Paths.get(fn)) => false
new java.io.File(fn).canWrite() => true

Using JDK 1.7.04

Any ideas?

kenshinji
  • 2,021
  • 4
  • 25
  • 39
user1414274
  • 211
  • 3
  • 6

3 Answers3

6

There is a bug in java.nio.file.Files.isWritable under windows: it won't take implicit permissions into consideration. java bug #7190897

xophos
  • 366
  • 4
  • 19
5

I would avoid using both APIs and instead rely on the exceptions thrown by e.g. new FileOutputStream(). They at least are real, and of real concern. Using the APIs you mention is entirely pointless, and it introduces timing windows and repeated code. You have to catch the IOException anyway: why write all that code twice?

user207421
  • 305,947
  • 44
  • 307
  • 483
  • That might be true, but your answer doesn't explain why the two give a different result in this case. (see title) – Puce Oct 02 '12 at 10:50
  • The test is part of TrueZIP code actually and can't be avoided because depending on the result, you may be allowed to write to the virtual file system or not. – Christian Schlichtherle Oct 02 '12 at 11:54
  • 1
    @Puce Sometimes the answer to a question is to do it another way. – user207421 Oct 02 '12 at 22:53
  • @EJP True, I didn't down-vote. But I'm curious myself why the result is different for the 2 APIs. :-) Is there a good reson or is there a bug? – Puce Oct 03 '12 at 09:15
5

The end result isn't too surprising:

java.nio.file.Files.isWritable(java.nio.file.Paths.get(fn)) => false
new java.io.File(fn).canWrite() => true

File.canWrite doesn't pay attention to ACLs at all and only checks the MS-DOS read-only attribute.

Files.isWriteable pays attention to ACLs but for whatever reason (to keep broken programs broken?) they left File.canWrite un-fixed. This turns out to be lucky, because in some situations it seems like it can return false even when you can open the file with no problems.

Really, I would summarise the methods like this:

  • File.canWrite sometimes returns true when you can't actually write to the file.
  • Files.isWriteable sometimes returns false when you can actually write to the file.

I'm not sure what the point of either method is right now. Since everyone who uses these ultimately has to write a non-broken equivalent which actually tries to open the file, one wonders why they didn't just open the file to do the check themselves.

Hakanai
  • 12,010
  • 10
  • 62
  • 132
  • Since https://bugs.openjdk.java.net/browse/JDK-7190897 Files.isWritable works as expected. – Joonas Pulakka Nov 07 '16 at 05:54
  • I think it's still advisable to simply perform the operation, rather than checking and then performing it, because the result of Files.isWritable is immediately out of date, even if it _was_ correct. Same goes for isReadable, exists, notExists, isRegularFile, isDirectory, isSymbolicLink, and probably more. – Hakanai Nov 07 '16 at 06:32