[kaffe] Bug in FileInputStream

Jim Pick jim@kaffe.org
19 Jun 2002 18:55:23 -0700


Okay, I put the patches in (on the head and on the branch).

Cheers,

 - Jim

On Tue, 2002-06-18 at 22:15, Patrick Tullmann wrote:
> 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 \
> 
> _______________________________________________
> kaffe mailing list
> kaffe@kaffe.org
> http://kaffe.org/cgi-bin/mailman/listinfo/kaffe