[equinox-dev] Reliable Testing For a Writable Folder

[Ed Merks <ed.merks@xxxxxxxxx>]

While investigating https://bugs.eclipse.org/bugs/show_bug.cgi?id=558462 I tracked it down to the fact that testing a folder's writability, modifies the folder's timestamp if the folder is writable.

There is very similar (copied) logic in all the following places:

  org.eclipse.equinox.launcher.Main.canWrite(File)
  org.eclipse.osgi.storage.StorageUtil.canWrite(File)
  org.eclipse.equinox.internal.p2.director.app.DirectorApplication.canWrite(File)
  org.eclipse.equinox.internal.p2.artifact.repository.Activator.isReadOnly(File)

Each takes the approach of creating a file in the folder and then deleting it, changing the folder's timestamp, if the temp file could be created.

The Activator.isReadOnly(File) logic specifically is what causes side-effects in the file system when loading an artifact repository via a file: URI when the running JVM has write access to that folder.  This is  what caused the mirrors for 2019-03 through 2019-12 to be invalidated daily, but anything loading artifact metadata via p2's APIs can result in this same problem.

I considered that perhaps calling File.lastModified and File.setLastModified is a reasonable workaround, e.g., like this:

    private boolean isReadOnly(File file) {
        if (file == null)
            return true; // If we've reached the root, then return true
        if (file.exists()) {
            try {
                // on Vista/Windows 7 you are not allowed to write executable files on virtual directories like "Program Files"
                long lastModified = file.lastModified();
                File tmpTestFile = File.createTempFile(".artifactlocktest", ".dll", file); //$NON-NLS-1$ //$NON-NLS-2$
                tmpTestFile.delete();
                file.setLastModified(lastModified);
                return false;
            } catch (IOException e) {
                return true; // permission issue to create new file, so it's readonly
            }
        }

        return isReadOnly(file.getParentFile());
    }

This does restore the lastModified value, except for the very short time interval while creating the temp file and deleting it...

Perhaps there's a better way to test for writability if we must continue to assume that java.io.File.canWrite() is unreliable.

It seems all this is there only for Vista, though for Activator.isReadOnly, the comment mentions Windows 7:

  // on Vista/Windows 7 you are not allowed to write executable files on virtual directories like "Program Files"

So I tested on my Windows 7 machine and indeed when I create the folder C:/Program Files/Test, it's not possible to create a temp file in that folder (regardless of extension) even though File.canWrite returns true.

Does anyone have any thoughts on how we might improve this behavior?  Note that I'm not sure whether or not the other 3 implementations might also cause similar unfortunate side-effects...

Perhaps they should all be guarded to apply only on Windows?

Regards,
Ed