Hello all!
I would like to submit this fix.
The problem is that to detect if a file is writable we actually open it
for writing... Even worse, in the case of a directory, we write to it a
temp file!
There are a number of issues that I see wrong here, but what worries me
most is that these operations can kill filesystem that reside on compact
flash memories and other devices that have a limited number of write.
The current fix does two things.
1. introduce a new method, native, canWriteDirectory(String path)
This method is meant to be implemented by whoever implements the VM
interface, so that it can do nice things with their target environment.
The default implementation works on Linux and should work on any *nix
operating systems. I've left canWriteDirectory(File file) for backward
compatibility, but I guess no one uses this anyway (it defer to the
native method).
2. Change the implementation of Java_java_io_VMFile_canWrite and
Java_java_io_VMFile_canRead so that they use cpio_checkAccess now,
instead of doing the old "The lazy man's way out" (which is also more
code, btw...)
I'm going to submit this today if no one complains, any ideas?
Ah, before I forget, there are a number of unrelated fixes like
indentation and all the function signature now reflect the fact that
these methods are static (jclass instead of jobject). It would be handy
here to use mercurial (hg record) to separate patchsets.
Mario
2008-04-09 Mario Torre <[EMAIL PROTECTED]>
* java/io/File.java (canWrite): use canWriteDirectory(String).
* vm/reference/java/io/VMFile.java (canWriteDirectory): new native
method.
* native/jni/java-io/java_io_VMFile.c: correct indentation, sync
function
names with header file definition.
(Java_java_io_VMFile_canRead): use cpio_checkAccess to get access
permission. Removed unused variable.
(Java_java_io_VMFile_canWrite): likewise.
(Java_java_io_VMFile_canWriteDirectory): new function.
--
Mario Torre, Software Developer, http://www.jroller.com/neugens/
aicas Allerton Interworks Computer Automated Systems GmbH
Haid-und-Neu-Straße 18 * D-76131 Karlsruhe * Germany
http://www.aicas.com * Tel: +49-721-663 968-53
pgp key: http://subkeys.pgp.net/ PGP Key ID: 80F240CF
Fingerprint: BA39 9666 94EC 8B73 27FA FC7C 4086 63E3 80F2 40CF
USt-Id: DE216375633, Handelsregister HRB 109481, AG Mannheim
Geschäftsführer: Dr. James J. Hunt
Please, support open standards:
http://opendocumentfellowship.org/petition/
http://www.nosoftwarepatents.com/
### Eclipse Workspace Patch 1.0
#P classpath
Index: java/io/File.java
===================================================================
RCS file: /sources/classpath/classpath/java/io/File.java,v
retrieving revision 1.72
diff -u -r1.72 File.java
--- java/io/File.java 6 Nov 2007 13:38:39 -0000 1.72
+++ java/io/File.java 9 Apr 2008 18:28:36 -0000
@@ -158,7 +158,7 @@
return false;
if (VMFile.isDirectory(path))
- return VMFile.canWriteDirectory(this);
+ return VMFile.canWriteDirectory(path);
else
return VMFile.canWrite(path);
}
Index: native/jni/java-io/java_io_VMFile.c
===================================================================
RCS file: /sources/classpath/classpath/native/jni/java-io/java_io_VMFile.c,v
retrieving revision 1.16
diff -u -r1.16 java_io_VMFile.c
--- native/jni/java-io/java_io_VMFile.c 6 Nov 2007 13:38:39 -0000 1.16
+++ native/jni/java-io/java_io_VMFile.c 9 Apr 2008 18:28:36 -0000
@@ -97,7 +97,7 @@
if (result != CPNATIVE_OK)
{
if (result != EEXIST)
- JCL_ThrowException (env,
+ JCL_ThrowException (env,
"java/io/IOException",
cpnative_getErrorString (result));
JCL_free_cstring (env, name, filename);
@@ -124,12 +124,11 @@
JNIEXPORT jboolean JNICALL
Java_java_io_VMFile_canRead (JNIEnv * env,
- jobject obj __attribute__ ((__unused__)),
- jstring name)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name)
{
#ifndef WITHOUT_FILESYSTEM
const char *filename;
- int fd;
int result;
/* Don't use the JCL convert function because it throws an exception
@@ -137,20 +136,18 @@
filename = (*env)->GetStringUTFChars (env, name, 0);
if (filename == NULL)
{
- return 0;
+ return JNI_FALSE;
}
- /* The lazy man's way out. We actually do open the file for reading
- briefly to verify it can be done */
- result = cpio_openFile (filename, &fd, CPFILE_FLAG_READ, 0);
+ result = cpio_checkAccess (filename, CPFILE_FLAG_READ);
+
(*env)->ReleaseStringUTFChars (env, name, filename);
if (result != CPNATIVE_OK)
- return 0;
- cpio_closeFile (fd);
+ return JNI_FALSE;
- return 1;
+ return JNI_TRUE;
#else /* not WITHOUT_FILESYSTEM */
- return 0;
+ return JNI_FALSE;
#endif /* not WITHOUT_FILESYSTEM */
}
@@ -166,12 +163,11 @@
JNIEXPORT jboolean JNICALL
Java_java_io_VMFile_canWrite (JNIEnv * env,
- jobject obj __attribute__ ((__unused__)),
- jstring name)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name)
{
#ifndef WITHOUT_FILESYSTEM
const char *filename;
- int fd;
int result;
/* Don't use the JCL convert function because it throws an exception
@@ -179,27 +175,34 @@
filename = (*env)->GetStringUTFChars (env, name, 0);
if (filename == NULL)
{
- return 0;
+ return JNI_FALSE;
}
- /* The lazy man's way out. We actually do open the file for writing
- briefly to verify it can be done */
- result = cpio_openFile (filename, &fd, CPFILE_FLAG_READWRITE, 0);
+ result = cpio_checkAccess (filename, CPFILE_FLAG_WRITE);
+
(*env)->ReleaseStringUTFChars (env, name, filename);
if (result != CPNATIVE_OK)
{
- return 0;
+ return JNI_FALSE;
}
- cpio_closeFile (fd);
- return 1;
+ return JNI_TRUE;
#else /* not WITHOUT_FILESYSTEM */
- return 0;
+ return JNI_FALSE;
#endif /* not WITHOUT_FILESYSTEM */
}
/*************************************************************************/
+JNIEXPORT jboolean JNICALL
+Java_java_io_VMFile_canWriteDirectory (JNIEnv *env, jclass clazz, jstring path)
+{
+ /* this is only valid on *nix systems */
+ return Java_java_io_VMFile_canWrite(env, clazz, path);
+}
+
+/*************************************************************************/
+
/*
* This method checks to see if we have execute permission on a file.
*
@@ -250,8 +253,8 @@
JNIEXPORT jboolean JNICALL
Java_java_io_VMFile_setReadOnly (JNIEnv * env,
- jobject obj __attribute__ ((__unused__)),
- jstring name)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name)
{
#ifndef WITHOUT_FILESYSTEM
const char *filename;
@@ -326,10 +329,10 @@
*/
JNIEXPORT jboolean JNICALL
Java_java_io_VMFile_setExecutable (JNIEnv *env,
- jclass clazz __attribute__ ((__unused__)),
- jstring name,
- jboolean executable,
- jboolean ownerOnly)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name,
+ jboolean executable,
+ jboolean ownerOnly)
{
return set_file_permissions (env, name, executable, ownerOnly,
CPFILE_FLAG_EXEC);
@@ -434,8 +437,8 @@
JNIEXPORT jboolean JNICALL
Java_java_io_VMFile_exists (JNIEnv * env,
- jobject obj __attribute__ ((__unused__)),
- jstring name)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name)
{
#ifndef WITHOUT_FILESYSTEM
const char *filename;
@@ -471,8 +474,8 @@
JNIEXPORT jboolean JNICALL
Java_java_io_VMFile_isFile (JNIEnv * env,
- jobject obj __attribute__ ((__unused__)),
- jstring name)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name)
{
#ifndef WITHOUT_FILESYSTEM
const char *filename;
@@ -508,8 +511,8 @@
JNIEXPORT jboolean JNICALL
Java_java_io_VMFile_isDirectory (JNIEnv * env,
- jobject obj __attribute__ ((__unused__)),
- jstring name)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name)
{
#ifndef WITHOUT_FILESYSTEM
const char *filename;
@@ -545,8 +548,8 @@
JNIEXPORT jlong JNICALL
Java_java_io_VMFile_length (JNIEnv * env,
- jobject obj __attribute__ ((__unused__)),
- jstring name)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name)
{
#ifndef WITHOUT_FILESYSTEM
const char *filename;
@@ -593,8 +596,8 @@
JNIEXPORT jlong JNICALL
Java_java_io_VMFile_lastModified (JNIEnv * env,
- jobject obj __attribute__ ((__unused__)),
- jstring name)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name)
{
#ifndef WITHOUT_FILESYSTEM
const char *filename;
@@ -630,8 +633,8 @@
JNIEXPORT jboolean JNICALL
Java_java_io_VMFile_setLastModified (JNIEnv * env,
- jobject obj __attribute__ ((__unused__)),
- jstring name, jlong newtime)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name, jlong newtime)
{
#ifndef WITHOUT_FILESYSTEM
const char *filename;
@@ -667,8 +670,8 @@
JNIEXPORT jboolean JNICALL
Java_java_io_VMFile_delete (JNIEnv * env,
- jobject obj __attribute__ ((__unused__)),
- jstring name)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name)
{
#ifndef WITHOUT_FILESYSTEM
const char *filename;
@@ -703,8 +706,8 @@
JNIEXPORT jboolean JNICALL
Java_java_io_VMFile_mkdir (JNIEnv * env,
- jobject obj __attribute__ ((__unused__)),
- jstring name)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name)
{
#ifndef WITHOUT_FILESYSTEM
const char *pathname;
@@ -739,8 +742,8 @@
JNIEXPORT jboolean JNICALL
Java_java_io_VMFile_renameTo (JNIEnv * env,
- jobject obj __attribute__ ((__unused__)),
- jstring t, jstring d)
+ jclass clazz __attribute__ ((__unused__)),
+ jstring t, jstring d)
{
#ifndef WITHOUT_FILESYSTEM
const char *old_filename, *new_filename;
@@ -783,8 +786,9 @@
*/
JNIEXPORT jobjectArray JNICALL
-Java_java_io_VMFile_list (JNIEnv * env, jobject obj
- __attribute__ ((__unused__)), jstring name)
+Java_java_io_VMFile_list (JNIEnv * env,
+ jclass clazz __attribute__ ((__unused__)),
+ jstring name)
{
#ifndef WITHOUT_FILESYSTEM
const int REALLOC_SIZE = 10;
@@ -991,8 +995,8 @@
JNIEXPORT jstring JNICALL
Java_java_io_VMFile_toCanonicalForm (JNIEnv *env,
- jclass class __attribute__ ((__unused__)),
- jstring jpath)
+ jclass class __attribute__ ((__unused__)),
+ jstring jpath)
{
#ifndef WITHOUT_FILESYSTEM
const char *path;
Index: vm/reference/java/io/VMFile.java
===================================================================
RCS file: /sources/classpath/classpath/vm/reference/java/io/VMFile.java,v
retrieving revision 1.12
diff -u -r1.12 VMFile.java
--- vm/reference/java/io/VMFile.java 22 Feb 2008 03:06:05 -0000 1.12
+++ vm/reference/java/io/VMFile.java 9 Apr 2008 18:28:36 -0000
@@ -58,10 +58,10 @@
{
if (Configuration.INIT_LOAD_LIBRARY)
{
- System.loadLibrary("javaio");
+ System.loadLibrary("javaio");
}
}
-
+
/*
* This native method does the actual work of getting the last file
* modification time. It also does the existence check to avoid the
@@ -167,19 +167,7 @@
/**
* This methods checks if a directory can be written to.
*/
- static boolean canWriteDirectory(File dir)
- {
- try
- {
- String filename = IS_DOS_8_3 ? "tst" : "test-dir-write";
- File test = File.createTempFile(filename, null, dir);
- return (test != null && test.delete());
- }
- catch (IOException ioe)
- {
- return false;
- }
- }
+ static native boolean canWriteDirectory(String path);
/**
* This native method checks file permissions for reading
@@ -199,6 +187,14 @@
static native boolean isDirectory(String dirpath);
/**
+ * This methods checks if a directory can be written to.
+ */
+ static boolean canWriteDirectory(File path)
+ {
+ return canWriteDirectory(path.getAbsolutePath());
+ }
+
+ /**
* This method returns an array of filesystem roots. Some operating systems
* have volume oriented filesystem. This method provides a mechanism for
* determining which volumes exist. GNU systems use a single hierarchical
