FYI: java.io.File security check fixes

[Mark Wielaard]

Hi,

This fixes a couple of File.security failures in Mauve.
It makes sure that we do the needed security check first before trying
to do any actual file operations. Unneeded checks have been eliminated.

2004-04-18  Mark Wielaard  <address@hidden>

       * java/io/File.java (canWrite): Only do checkWrite() security
       check, use internal methods for actual actions.
       (createTempFile): Don't do security checks for temp files that
       won't be created.
       (setReadOnly): Do checkWrite() security check before trying to
       do anything else.
       (renameTo): Add checkWrite() security check for destination file.

Only one related mauve failure remains with this:

java.lang.SecurityException: Unexpected check: (java.io.FilePermission 
/tmp/mauve-testdir/test-dir-write1082297238707.tmp write)
FAIL: gnu.testlet.java.io.File.security: dir.canWrite - unexpected exception 
(number 1)

This is because we actually use createTempFile() to check that a
directory is writable and that generates some extra security checks. It
would be nice to just check whether or not a directory is writable
directly without trying to do a couple of explicit file creation calls.
But currently our native layer doesn't offer such an option.

Cheers,

Mark

Index: java/io/File.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/io/File.java,v
retrieving revision 1.40
diff -u -r1.40 File.java
--- java/io/File.java   17 Apr 2004 17:29:18 -0000      1.40
+++ java/io/File.java   18 Apr 2004 14:26:54 -0000
@@ -173,22 +173,21 @@
    */
   public boolean canWrite()
   {
-    // We still need to do a SecurityCheck since exists() only checks
-    // for read access
+    // First do a SecurityCheck before doing anything else.
     checkWrite();
      
     // Test for existence.  This is required by the spec
-    if (!exists())
+    if (!existsInternal(path))
       return false;
 
-    if (!isDirectory())
+    if (!isDirectoryInternal(path))
       return canWriteInternal(path);
     else
       try
         {
           /* If the separator is '\' a DOS-style-filesystem is assumed
              and a short name is used, otherwise use a long name.
-             WARNGIN: some implementation of DOS-style-filesystems also
+             WARNING: some implementation of DOS-style-filesystems also
              accept '/' as separator. In that case the following code
              will fail.
           */
@@ -961,10 +960,10 @@
           throw new IOException("Cannot determine system temporary 
directory"); 
        
         directory = new File(dirname);
-        if (!directory.exists())
+        if (!directory.existsInternal(directory.path))
           throw new IOException("System temporary directory "
                                 + directory.getName() + " does not exist.");
-        if (!directory.isDirectory())
+        if (!directory.isDirectoryInternal(directory.path))
           throw new IOException("System temporary directory "
                                 + directory.getName()
                                 + " is not really a directory.");
@@ -994,7 +993,7 @@
             String filename = prefix + System.currentTimeMillis() + suffix;
             file = new File(directory, filename);
           }
-        while (file.exists());
+        while (file.existsInternal(file.path));
       }
     else
       {
@@ -1011,7 +1010,7 @@
             String filename = prefix + java.lang.Integer.toHexString(n) + 
suffix;
             file = new File(directory, filename);
           }
-        while (file.exists());
+        while (file.existsInternal(file.path));
       }
 
     // Verify that we are allowed to create this file
@@ -1020,6 +1019,7 @@
       sm.checkWrite(file.getAbsolutePath());
 
     // Now create the file and return our file object
+    // XXX - FIXME race condition.
     createInternal(file.getAbsolutePath()); 
     return file;
   }
@@ -1045,13 +1045,13 @@
    */
   public boolean setReadOnly()
   {
+    // Do a security check before trying to do anything else.
+    checkWrite();
+
     // Test for existence.
-    if (!exists())
+    if (!existsInternal(path))
       return false;
 
-    // We still need to do a SecurityCheck since exists() only checks
-    // for read access
-    checkWrite();
     return setReadOnlyInternal(path);
   }
 
@@ -1192,6 +1192,7 @@
   public synchronized boolean renameTo(File dest)
   {
     checkWrite();
+    dest.checkWrite();
     // Call our native rename method
     return renameToInternal(path, dest.path);
   }

signature.asc
Description: This is a digitally signed message part