[kaffe] Bug in FileInputStream

Patrick Tullmann tullmann@cs.utah.edu
Tue, 18 Jun 2002 23:15:52 -0600


Jim wrote:
> Okay, I just tried the patch, but now it fails some regression tests...

It turns out I was invoking "File.getName()" where I should have
invoked "File.getPath()".  Duh.  I swear I ran FullTest.sh before
submitting this patch.  I also included FileChecks.java, which tests
the File-based constructors with various sorts of bad File objects.
Before I get to the hopefully improved patch, a couple notes.

First, the java.util.TimeZone implementation goes into infinite
recursion if it cannot load the "GMT" time zone.  A patch to abort if
this is encountered is included, but I wonder if such fatal errors at
startup should be redirected to a common place in kaffe.lang or
something?  Feel free to leave this out, of course.

Second, occasionally when I run FullTest.sh, it doesn't seem to
recompile the class libraries totally.  Anyone else seen this?
(Perhaps I'm just making up excuses, though... :)

Here's the improved patch.  Here's a ChangeLog entry, too.  I'm quite
sure I ran the FullTest.sh script this time.  

Tue Jun 18 22:36:21 MDT 2002 Pat Tullmann <pat@tullmann.org>
	* libraries/javalib/java/io/FileInputStream.java,
	libraries/javalib/java/io/FileOutputStream.java: Change
	constructors to detect and fail with FileNotFoundException if
	given file/filename is a directory.
	* test/regression/FileChecks.java: Test the FileInputStream
	constructors.
	* java/util/TimeZone.java: Add sanity check to prevent failure
	to lookup GMT from causing infinite recursion.

-Pat

----- ----- ---- ---  ---  --   -    -      -         -               -
Pat Tullmann                                       tullmann@cs.utah.edu
		 Does the name `Pavlov' ring a bell?


 Sun Jun 16 22:40:33 PDT 2002  Jim Pick  <jim@kaffe.org>
 
         * developers/update-class-list: Fixed (was broken by
diff -u -r -N --exclude=CVS --exclude=.* --exclude=DEAD --exclude=Makefile.in --exclude=configure pure/libraries/javalib/java/io/FileInputStream.java fstreamfix/libraries/javalib/java/io/FileInputStream.java
--- pure/libraries/javalib/java/io/FileInputStream.java	Thu Jan  3 22:12:22 2002
+++ fstreamfix/libraries/javalib/java/io/FileInputStream.java	Tue Jun 18 17:12:36 2002
@@ -20,31 +20,38 @@
         System.loadLibrary("io");
 }
 
-public FileInputStream(File file) throws FileNotFoundException {
-	this(file.getPath());
-}
-
 public FileInputStream(FileDescriptor fdObj) {
 	SecurityManager sm = System.getSecurityManager();
 	if (sm != null)
 		sm.checkRead(fdObj);
+	// XXX check if fd is invalid?  Or points to a non-file object?
 	fd=fdObj;
 }
 
 public FileInputStream(String name) throws FileNotFoundException {
-	final SecurityManager sm = System.getSecurityManager();
-	if (sm != null)
-		sm.checkRead(name);
+	this(new File(name));
+}
+
+public FileInputStream(File file) throws FileNotFoundException {
+	final String fname = file.getPath();
+
+	// Note File.isDirectory() will do the required SecurityManager 
+	// canRead() check for us.
+	if (file.isDirectory())
+		throw new FileNotFoundException(fname+ ": Is a directory");
+
 	try {
-		open(name);
+		open(fname);
 	} catch (IOException e) {
 		/* Note that open may throw an IOException, but the spec says
 		 * that this constructor throws only FileNotFoundExceptions,
 		 * hence we must map them.
 		 */
-		throw new FileNotFoundException(name + ": " + e.getMessage());
+		throw new FileNotFoundException(fname + ": " + e.getMessage());
 	}
 }
+
+
 
 native public int available() throws IOException;
 
diff -u -r -N --exclude=CVS --exclude=.* --exclude=DEAD --exclude=Makefile.in --exclude=configure pure/libraries/javalib/java/io/FileOutputStream.java fstreamfix/libraries/javalib/java/io/FileOutputStream.java
--- pure/libraries/javalib/java/io/FileOutputStream.java	Thu Jan  3 22:12:22 2002
+++ fstreamfix/libraries/javalib/java/io/FileOutputStream.java	Tue Jun 18 17:12:28 2002
@@ -20,13 +20,8 @@
 
 private FileDescriptor fd = new FileDescriptor();
 
-public FileOutputStream(File file) throws IOException
-{
-	this(file.getPath());
-}
-
 public FileOutputStream(FileDescriptor fdObj)
-	{
+{
 	final SecurityManager sm = System.getSecurityManager();
 	if (sm != null)
 		sm.checkWrite(fdObj);
@@ -38,16 +33,39 @@
 	this(name, false);
 }
 
+public FileOutputStream(File file) throws FileNotFoundException
+{
+	this(file, false);
+}
+
 public FileOutputStream(String name, boolean append) throws FileNotFoundException
 {
+	this(new File(name), append);
+}
+
+public FileOutputStream(File file, boolean append) throws FileNotFoundException
+{
+	final String fname = file.getPath();
+
+	// Note, don't need an explicit File.isDirectory() check 
+	// as the open() call in write mode will check that.
+	
 	final SecurityManager sm = System.getSecurityManager();
 	if (sm != null)
-		sm.checkWrite(name);
-	if (!append) {
-		open(name);
+		sm.checkWrite(fname);
+
+	try
+	{
+		if (!append) {
+			open(fname);
+		}
+		else {
+			openAppend(fname);
+		}
 	}
-	else {
-		openAppend(name);
+	catch (IOException ioe)
+	{
+		throw new FileNotFoundException(fname+ ": " +ioe.getMessage());
 	}
 }
 
@@ -69,9 +87,9 @@
 	return (fd);
 }
 
-native private void open(String name);
+native private void open(String name) throws IOException;
 
-native private void openAppend(String name);
+native private void openAppend(String name) throws IOException;
 
 public void write(byte b[]) throws IOException
 {
diff -u -r -N --exclude=CVS --exclude=.* --exclude=DEAD --exclude=Makefile.in --exclude=configure pure/libraries/javalib/java/util/TimeZone.java fstreamfix/libraries/javalib/java/util/TimeZone.java
--- pure/libraries/javalib/java/util/TimeZone.java	Thu Mar 28 04:24:13 2002
+++ fstreamfix/libraries/javalib/java/util/TimeZone.java	Tue Jun 18 17:01:37 2002
@@ -154,6 +154,13 @@
 		/* If file cannot parsed properly, simply fall back to GMT
 		 * timezone.  This is what the getTimeZone() API doc demands.
 		 */
+		if (ID.equals("GMT"))
+		{
+			System.err.println("Fatal error loading (required) GMT TimeZone.  Aborting.");
+			System.err.println("Exception: " +e);
+			e.printStackTrace(System.err);
+			System.exit(1);
+		}
 		tz = getTimeZone("GMT");
 		zones.put(ID, tz);
 	}
diff -u -r -N --exclude=CVS --exclude=.* --exclude=DEAD --exclude=Makefile.in --exclude=configure pure/test/regression/FileChecks.java fstreamfix/test/regression/FileChecks.java
--- pure/test/regression/FileChecks.java	Wed Dec 31 17:00:00 1969
+++ fstreamfix/test/regression/FileChecks.java	Tue Jun 18 20:41:12 2002
@@ -0,0 +1,76 @@
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.FileNotFoundException;
+
+public class FileChecks 
+{
+	public static void main(String[] argv) 
+	{
+		try
+		{
+			FileInputStream     fis  = new FileInputStream(".");
+			System.out.println("1 FAIL! Should throw java.io.FileNotFoundException!");
+			System.exit(1);
+		}
+		catch (FileNotFoundException e)
+		{
+			System.out.println("1 Success got: " +e);
+		}
+
+		try
+		{
+			File                file = new File(".");
+			FileInputStream     fis  = new FileInputStream(file);
+			System.out.println("2 FAIL! Should throw java.io.FileNotFoundException!");
+			System.exit(1);
+		}
+		catch (FileNotFoundException e)
+		{
+			System.out.println("2 Success got: " +e);
+		}
+
+		try
+		{
+			FileOutputStream     fis  = new FileOutputStream(".");
+			System.out.println("3 FAIL! Should throw java.io.FileNotFoundException!");
+			System.exit(1);
+		}
+		catch (FileNotFoundException e)
+		{
+			System.out.println("3 Success got: " +e);
+		}
+
+		try
+		{
+			File                file = new File(".");
+			FileOutputStream     fis  = new FileOutputStream(file);
+			System.out.println("4 FAIL! Should throw java.io.FileNotFoundException!");
+			System.exit(1);
+		}
+		catch (FileNotFoundException e)
+		{
+			System.out.println("4 Success got: " +e);
+		}
+
+		try
+		{
+			File                file = new File("FileChecks.class"); // should exist
+			FileInputStream     fis  = new FileInputStream(file);
+			System.out.println("5 Success.");
+		}
+		catch (FileNotFoundException e)
+		{
+			System.out.println("5 FAIL! Should not get: " +e);
+		}
+
+	}
+}
+
+/* Expected Output:
+1 Success got: java.io.FileNotFoundException: .: Is a directory
+2 Success got: java.io.FileNotFoundException: .: Is a directory
+3 Success got: java.io.FileNotFoundException: .: Is a directory
+4 Success got: java.io.FileNotFoundException: .: Is a directory
+5 Success.
+*/
diff -u -r -N --exclude=CVS --exclude=.* --exclude=DEAD --exclude=Makefile.in --exclude=configure pure/test/regression/Makefile.am fstreamfix/test/regression/Makefile.am
--- pure/test/regression/Makefile.am	Wed May 29 16:58:44 2002
+++ fstreamfix/test/regression/Makefile.am	Tue Jun 18 20:38:04 2002
@@ -68,6 +68,7 @@
 	SoTimeout.java \
 	wc.java \
 	FileTest.java \
+	FileChecks.java \
 	finalexc.java \
 	finaltest.java \
 	finaltest2.java \