[patch for] Re: Is serialization broken?

[Bryce McKinlay <bryce at albatross dot co dot nz>]

Warren Levy wrote:

> That may help but I think that ObjectInputStream.read() is the real
> problem.

Turns out that there were bugs in both the read() varients of ObjectInputStream.

I was seeing the second readInt() call in HashMap.readObject return 1535 which was
not the value that the second writeInt() call wrote - it should be 1000. When
HashMap's readObject() attempted to read past the end of its data, readObject
threw an "Invalid marker on stream" IOException because it sees a TC_ENDBLOCKDATA,
which is not handled by the marker switch. Further, useful exception information
is not propogated across the reflection invocation of readObject() because
callReadMethod() essentially ignores InvocationTargetException, making the problem
difficult to identify.

This patch fixes the bugs in both read() methods, and modifies
callReadMethod()/callWriteMethod() to rethrow the target's exception if possible.

The test cases now passes with either of the DataInputStream implementations. I'm
checking this in.

regards

  [ bryce ]


2001-01-27  Bryce McKinlay  <bryce@albatross.co.nz>

	* java/io/ObjectInputStream.java (read): AND byte with 0xff to make
	result unsigned.
	(read (byte[], int, int)): Only call readNextBlock() if the block 
	buffer would actually be overrun. Increment blockDataPosition.
	(callReadMethod): Propogate exceptions from invocation target.
	* java/io/ObjectOutputStream.java (callWriteMethod): Propogate 
	exceptions from invocation target.

Index: ObjectInputStream.java
===================================================================
RCS file: /cvs/gcc/egcs/libjava/java/io/ObjectInputStream.java,v
retrieving revision 1.5
diff -u -r1.5 ObjectInputStream.java
--- ObjectInputStream.java	2000/11/26 01:48:04	1.5
+++ ObjectInputStream.java	2001/01/27 05:52:56
@@ -577,21 +577,23 @@
     {
       if (this.blockDataPosition >= this.blockDataBytes)
 	readNextBlock ();
-      return this.blockData[this.blockDataPosition++];
+      return (this.blockData[this.blockDataPosition++] & 0xff);
     }
     else
       return this.realInputStream.read ();
   }
 
-  public int read (byte data[], int offset, int length) throws IOException
+  public int read (byte[] data, int offset, int length) throws IOException
   {
     if (this.readDataFromBlock)
     {
-      if (this.blockDataPosition + length >= this.blockDataBytes)
+      if (this.blockDataPosition + length > this.blockDataBytes)
 	readNextBlock ();
 
       System.arraycopy (this.blockData, this.blockDataPosition,
 			data, offset, length);
+      blockDataPosition += length;	
+
       return length;
     }
     else
@@ -1359,16 +1361,29 @@
   {
     try
       {
-	Class classArgs[] = {Class.forName ("java.io.ObjectInputStream")};
+	Class classArgs[] = {ObjectInputStream.class};
 	Method m = getMethod (klass, "readObject", classArgs);
 	if (m == null)
 	  return;
 	Object args[] = {this};
-	m.invoke (obj, args);	
+	m.invoke (obj, args);
+      }
+    catch (InvocationTargetException x)
+      {
+        /* Rethrow if possible. */
+	Throwable exception = x.getTargetException();
+	if (exception instanceof RuntimeException)
+	  throw (RuntimeException) exception;
+	if (exception instanceof IOException)
+	  throw (IOException) exception;
+
+	throw new IOException ("Exception thrown from readObject() on " +
+			       klass + ": " + exception.getClass().getName());
       }
-    catch (Exception _)
+    catch (Exception x)
       {
-	throw new IOException ();
+	throw new IOException ("Failure invoking readObject() on " +
+			       klass + ": " + x.getClass().getName());
       }
   }
     
Index: ObjectOutputStream.java
===================================================================
RCS file: /cvs/gcc/egcs/libjava/java/io/ObjectOutputStream.java,v
retrieving revision 1.7
diff -u -r1.7 ObjectOutputStream.java
--- ObjectOutputStream.java	2000/10/05 23:57:16	1.7
+++ ObjectOutputStream.java	2001/01/27 05:52:56
@@ -633,7 +633,7 @@
   /**
      @see java.io.DataOutputStream#write (byte[])
   */
-  public void write (byte b[]) throws IOException
+  public void write (byte[] b) throws IOException
   {
     write (b, 0, b.length);
   }
@@ -642,7 +642,7 @@
   /**
      @see java.io.DataOutputStream#write (byte[],int,int)
   */
-  public void write (byte b[], int off, int len) throws IOException
+  public void write (byte[] b, int off, int len) throws IOException
   {
     if (writeDataAsBlocks)
     {
@@ -1175,22 +1175,35 @@
 
   private void callWriteMethod (Object obj) throws IOException
   {
+    Class klass = obj.getClass ();
     try
       {
-	Class classArgs[] = {Class.forName ("java.io.ObjectOutputStream")};
-	Class klass = obj.getClass ();
+	Class classArgs[] = {ObjectOutputStream.class};
 	Method m = getMethod (klass, "writeObject", classArgs);
 	if (m == null)
 	  return;
 	Object args[] = {this};
 	m.invoke (obj, args);	
       }
-    catch (Exception _)
+    catch (InvocationTargetException x)
       {
-	throw new IOException ();
+        /* Rethrow if possible. */
+	Throwable exception = x.getTargetException();
+	if (exception instanceof RuntimeException)
+	  throw (RuntimeException) exception;
+	if (exception instanceof IOException)
+	  throw (IOException) exception;
+
+	throw new IOException ("Exception thrown from writeObject() on " +
+			       klass + ": " + exception.getClass().getName());
       }
+    catch (Exception x)
+      {
+	throw new IOException ("Failure invoking writeObject() on " +
+			       klass + ": " + x.getClass().getName());
+      }
   }
-    
+
   private boolean getBooleanField (Object obj, String field_name) throws IOException
   {
     try
@@ -1331,7 +1344,7 @@
   private static native Field getField (Class klass, String name)
     throws java.lang.NoSuchFieldException;
 
-  private static native Method getMethod (Class klass, String name, Class args[])
+  private static native Method getMethod (Class klass, String name, Class[] args)
     throws java.lang.NoSuchMethodException;
 
   // this value comes from 1.2 spec, but is used in 1.1 as well