This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Patch] Fix PR libgcj/20389


As stated in the PR:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20389

In 4.0.0 (and the trunk) BufferedInputStream has (at least) two regressions where a carefully crafted sequence of mark and read will cause either an erroneous EOF or ArrayIndexOutOfBoundsException.

Somewhere after 3.4.x a field called marktarget was added with a bunch of associated logic. Other than crashing my program, it was unclear to me what the intended purpose of marktarget and its associated code was.

The attached patch removes marktarget and simplifies the refill logic.

I added the PR testcase to mauve and did make check in libjava and mauve with no regressions on the 4.0.0 branch (i686-pc-linux).


2005-03-08 David Daney <ddaney@avtrex.com>


	PR libgcj/20389
	* java/io/BufferedInputStream.java (marktarget):  Field removed.
	(mark):  Rewrote.
	(read()): Replaced marktarget with marklimit.
	(read(byte[], int, int)): Ditto.
	(skip): Ditto.
	(refill): Rewrote mark handling case.


OK to commit to both the branch and the trunk?


David Daney.
Index: java/io/BufferedInputStream.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/io/BufferedInputStream.java,v
retrieving revision 1.11
diff -c -p -r1.11 BufferedInputStream.java
*** java/io/BufferedInputStream.java	11 Jan 2005 20:04:33 -0000	1.11
--- java/io/BufferedInputStream.java	8 Mar 2005 23:45:25 -0000
*************** public class BufferedInputStream extends
*** 103,117 ****
    protected int marklimit;
  
    /**
!    * This is the maximum size we have to allocate for the mark buffer.
!    * This number may be huge (Integer.MAX_VALUE). The class will continue
!    * to allocate new chunks (specified by <code>CHUNKSIZE</code>) until the
!    * the size specified by this field is achieved.
!    */
!   private int marktarget = 0;
! 
!   /**
!    * This is the number of bytes to allocate to reach marktarget.
     */
    static final private int CHUNKSIZE = 1024;
  
--- 103,109 ----
    protected int marklimit;
  
    /**
!    * This is the number of bytes to allocate to reach marklimit.
     */
    static final private int CHUNKSIZE = 1024;
  
*************** public class BufferedInputStream extends
*** 196,204 ****
     */
    public synchronized void mark(int readlimit)
    {
!     marktarget = marklimit = readlimit;
!     if (marklimit > CHUNKSIZE)
!       marklimit = CHUNKSIZE;
      markpos = pos;
    }
  
--- 188,214 ----
     */
    public synchronized void mark(int readlimit)
    {
!     // readlimit can be at most Integer.MAX_VALUE - 1 as we make the
!     // buffer one larger than readlimit.  For negative readlimits
!     // choose a value that will not cause us to resize the buffer and
!     // will not be negative.  This should prevent various types of
!     // array access and creation exceptions elsewhere in the code.
!     if (readlimit > Integer.MAX_VALUE - 1)
!       readlimit = Integer.MAX_VALUE - 1;
!     else if (readlimit <= 0)
!       {
!         if (buf.length > 1)
!           readlimit = buf.length - 1;
!         else
!           {
!             // mark not possible for negitive readlimit and buffer of
!             // length 1.  Silently ignore the mark request.
!             readlimit = 0;
!             markpos = -1;
!             return;
!           }
!       }
!     marklimit = readlimit;
      markpos = pos;
    }
  
*************** public class BufferedInputStream extends
*** 231,237 ****
      if (pos >= count && !refill())
        return -1;	// EOF
  
!     if (markpos >= 0 && pos - markpos > marktarget)
        markpos = -1;
  
      return buf[pos++] & 0xFF;
--- 241,247 ----
      if (pos >= count && !refill())
        return -1;	// EOF
  
!     if (markpos >= 0 && pos - markpos > marklimit)
        markpos = -1;
  
      return buf[pos++] & 0xFF;
*************** public class BufferedInputStream extends
*** 278,284 ****
      off += totalBytesRead;
      len -= totalBytesRead;
  
!     if (markpos >= 0 && pos - markpos > marktarget)
        markpos = -1;
  
      while (len > 0 && super.available() > 0 && refill())
--- 288,294 ----
      off += totalBytesRead;
      len -= totalBytesRead;
  
!     if (markpos >= 0 && pos - markpos > marklimit)
        markpos = -1;
  
      while (len > 0 && super.available() > 0 && refill())
*************** public class BufferedInputStream extends
*** 290,296 ****
  	len -= remain;
  	totalBytesRead += remain;
  
! 	if (markpos >= 0 && pos - markpos > marktarget)
  	  markpos = -1;
        }
  
--- 300,306 ----
  	len -= remain;
  	totalBytesRead += remain;
  
! 	if (markpos >= 0 && pos - markpos > marklimit)
  	  markpos = -1;
        }
  
*************** public class BufferedInputStream extends
*** 348,354 ****
  	pos += numread;
  	n -= numread;
  
!         if (markpos >= 0 && pos - markpos > marktarget)
            markpos = -1;
        }
  
--- 358,364 ----
  	pos += numread;
  	n -= numread;
  
!         if (markpos >= 0 && pos - markpos > marklimit)
            markpos = -1;
        }
  
*************** public class BufferedInputStream extends
*** 378,394 ****
  	pos -= markpos;
  	markpos = 0;
        }
!     else if (count >= buf.length && count < marktarget)	// BTW, markpos == 0
        {
! 	// Need to grow the buffer now to have room for marklimit bytes.
! 	// Note that the new buffer is one greater than marklimit.
! 	// This is so that there will be one byte past marklimit to be read
! 	// before having to call refill again, thus allowing marklimit to be
! 	// invalidated.  That way refill doesn't have to check marklimit.
! 	marklimit += CHUNKSIZE;
! 	if (marklimit >= marktarget)
! 	  marklimit = marktarget;
! 	byte[] newbuf = new byte[marklimit + 1];
  	System.arraycopy(buf, 0, newbuf, 0, count);
  	buf = newbuf;
        }
--- 388,409 ----
  	pos -= markpos;
  	markpos = 0;
        }
!     else if (count >= buf.length && count <= marklimit)	// BTW, markpos == 0
        {
! 	// Need to grow the buffer now to have room for marklimit
! 	// bytes.  Note that the new buffer is one greater than
! 	// marklimit.  This is so that there will be one byte past
! 	// marklimit to be read before having to call refill again,
! 	// thus allowing marklimit to be invalidated.  That way refill
! 	// doesn't have to check marklimit.  If count == marklimit and
! 	// we were sure the underlying read would succeed, we could
! 	// invalidate the mark and use the existing buffer.  But we
! 	// don't know if it will succeed thus we grow the buffer so
! 	// reset will work after an EOF.
!         int newsize = Math.min(buf.length + CHUNKSIZE, marklimit + 1);
!           
! 	byte[] newbuf = new byte[newsize];
!         
  	System.arraycopy(buf, 0, newbuf, 0, count);
  	buf = newbuf;
        }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]