Re: Object serialization patch

[Guilhem Lavaux]

Hi,

Here is a new patch I propose. I've taken the suggestion 
into account and fix another small error reporting problem.

New changelog entry:

2004-02-23  Guilhem Lavaux <address@hidden>
        
        * libraries/javalib/java/io/ObjectInputStream.java
        (readClassDescriptor): Fixed field sorting. Keep
        elements of the mapping non null.
        (readFields): Fixed main loop and base logic. Small
        reindentation.
        (checkTypeConsistency): New method.

        * libraries/javalib/java/io/ObjectStreamField.java
        (lookupField): New method to update the field
        reference.
        (setBooleanField, setByteField, setCharField,
        setShortField, setIntField, setLongField,
        setFloatField, setDoubleField, setObjectField):
        Improved exception reporting.

        * libraries/javalib/java/io/ObjectStreamClass.java
        (setClass, setFields): Call lookupField when
        building the field database.

Index: java/io/ObjectInputStream.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/io/ObjectInputStream.java,v
retrieving revision 1.38
diff -b -B -u -r1.38 ObjectInputStream.java
--- java/io/ObjectInputStream.java      10 Feb 2004 07:28:09 -0000      1.38
+++ java/io/ObjectInputStream.java      23 Feb 2004 15:30:11 -0000
@@ -412,6 +412,65 @@
   }
 
   /**
+   * This method makes a partial check of types for the fields
+   * contained given in arguments. It checks primitive types of
+   * fields1 against non primitive types of fields2. This method 
+   * assumes the two lists has already been sorted according to 
+   * the Java specification.
+   *
+   * @param name Name of the class owning the given fields.
+   * @param fields1 First list to check.
+   * @param fields2 Second list to check.
+   * @throws InvalidClassException if a field in fields1, which has a 
primitive type, is a present
+   * in the non primitive part in fields2.
+   */
+  private void checkTypeConsistency(String name, ObjectStreamField[] fields1, 
ObjectStreamField[] fields2)
+    throws InvalidClassException
+  {
+    int nonPrimitive = 0;
+    
+    for (nonPrimitive = 0; 
+        nonPrimitive < fields1.length
+          && fields1[nonPrimitive].isPrimitive(); nonPrimitive++)
+      {
+      }
+
+    if (nonPrimitive == fields1.length)
+      return;
+    
+    int i = 0;
+    ObjectStreamField f1;
+    ObjectStreamField f2;
+    
+    while (i < fields2.length
+          && nonPrimitive < fields1.length)
+      {
+       f1 = fields1[nonPrimitive];
+       f2 = fields2[i];
+       
+       if (!f2.isPrimitive())
+         break;
+
+       int compVal = f1.getName().compareTo (f2.getName());
+
+       if (compVal < 0)
+         {
+           nonPrimitive++;
+         }
+       else if (compVal > 0)
+         {
+           i++;
+         }
+       else
+         {
+           throw new InvalidClassException
+             ("invalid field type for " + f2.getName() +
+              " in class " + name);
+         }
+      }
+  }
+
+  /**
    * This method reads a class descriptor from the real input stream
    * and use these data to create a new instance of ObjectStreamClass.
    * Fields are sorted and ordered for the real read which occurs for
@@ -496,6 +555,15 @@
     int real_idx = 0;
     int map_idx = 0;
 
+    /*
+     * Check that there is no type inconsistencies between the lists.
+     * A special checking must be done for the two groups: primitive types and
+     * not primitive types. 
+     */
+    checkTypeConsistency(name, real_fields, stream_fields);
+    checkTypeConsistency(name, stream_fields, real_fields);
+
+    
     while (stream_idx < stream_fields.length
           || real_idx < real_fields.length)
       {
@@ -527,21 +595,13 @@
              {
                stream_field = stream_fields[stream_idx++];
                real_field = real_fields[real_idx++];
-               if(stream_field.getType() != real_field.getType())
+               if (stream_field.getType() != real_field.getType())
                    throw new InvalidClassException
                        ("invalid field type for " + real_field.getName() +
                        " in class " + name);
              }
          }
-       if (stream_field != null)
-         {
-           if (stream_field.getOffset() < 0)
-               stream_field = null;
-           else if (!stream_field.isToSet())
-               real_field = null;
-         }
-       if (real_field != null && !real_field.isToSet())
-           real_field = null;
+
        /* If some of stream_fields does not correspond to any of real_fields,
         * or the opposite, then fieldmapping will go short.
         */
@@ -1576,12 +1636,11 @@
       {
        ObjectStreamField stream_field = fields[i];
        ObjectStreamField real_field = fields[i + 1];
-       if(stream_field != null || real_field != null)
-         {
-           boolean read_value = stream_field != null;
-           boolean set_value = real_field != null;
+       boolean read_value = (stream_field != null && stream_field.getOffset() 
>= 0 && stream_field.isToSet());
+       boolean set_value = (real_field != null && real_field.isToSet());
            String field_name;
            char type;
+
            if (stream_field != null)
              {
                field_name = stream_field.getName();
@@ -1680,8 +1739,8 @@
                  {
                    Object value =
                        read_value ? readObject() : null;
-                   if (set_value)
-                       real_field.setObjectField(obj, value);
+             if (set_value && stream_field != null)
+               real_field.setObjectField(obj, stream_field.getTypeString(), 
value);
                    break;
                  }
                default:
@@ -1689,7 +1748,6 @@
              }
          }
       }
-  }
 
   // Toggles writing primitive data to block-data buffer.
   private boolean setBlockDataMode (boolean on)
Index: java/io/ObjectStreamClass.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/io/ObjectStreamClass.java,v
retrieving revision 1.30
diff -b -B -u -r1.30 ObjectStreamClass.java
--- java/io/ObjectStreamClass.java      2 Feb 2004 10:26:51 -0000       1.30
+++ java/io/ObjectStreamClass.java      23 Feb 2004 15:30:11 -0000
@@ -344,6 +344,13 @@
                newFieldList[k] = exportedFields[j];
                newFieldList[k].setPersistent(true);
                newFieldList[k].setToSet(false);
+               try
+                 {
+                   newFieldList[k].lookupField(clazz);
+                 }
+               catch (NoSuchFieldException _)
+                 {
+                 }
                j++;
              }
            else
@@ -554,6 +561,19 @@
            if (fields != null)
              {
                Arrays.sort (fields);
+               // Retrieve field reference.
+               for (int i=0; i < fields.length; i++)
+                 {
+                   try
+                     {
+                       fields[i].lookupField(cl);
+                     }
+                   catch (NoSuchFieldException _)
+                     {
+                       fields[i].setToSet(false);
+                     }
+                 }
+               
                calculateOffsets();
                return;
              }
Index: java/io/ObjectStreamField.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/io/ObjectStreamField.java,v
retrieving revision 1.13
diff -b -B -u -r1.13 ObjectStreamField.java
--- java/io/ObjectStreamField.java      2 Feb 2004 10:26:51 -0000       1.13
+++ java/io/ObjectStreamField.java      23 Feb 2004 15:30:11 -0000
@@ -41,6 +41,8 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import gnu.java.lang.reflect.TypeSignature;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 
 /**
  * This class intends to describe the field of a class for the serialization
@@ -99,7 +101,7 @@
  
   /**
    * There are many cases you can not get java.lang.Class from typename 
-   * if your context class loader cann not load it, then use typename to
+   * if your context class loader cannot load it, then use typename to
    * construct the field.
    *
    * @param name Name of the field to export.
@@ -292,7 +294,7 @@
   }
 
   /**
-   * This methods returns true if the field is marked as to be
+   * This method returns true if the field is marked as to be
    * set.
    *
    * @return True if it is to be set, false in the other cases.
@@ -303,112 +305,191 @@
     return toset;
   }
 
+  /**
+   * This method searches for its field reference in the specified class
+   * object. It requests privileges. If an error occurs the internal field
+   * reference is not modified.
+   *
+   * @throws NoSuchFieldException if the field name does not exist in this 
class.
+   * @throws SecurityException if there was an error requesting the privileges.
+   */
+  void lookupField(Class clazz) throws NoSuchFieldException, SecurityException
+  {
+    final Field f = clazz.getDeclaredField(name);
+    
+    AccessController.doPrivileged(new PrivilegedAction()
+      {
+       public Object run()
+       {
+         f.setAccessible(true);
+         return null;
+       }
+      });
+    
+    this.field = f;
+  }
+
   public String toString ()
   {
     return "ObjectStreamField< " + type + " " + name + " >";
   }
 
-  final void setBooleanField(Object obj, boolean val)
+  /*
+   * These methods set the required field in the class instance obj.
+   * They may throw NullPointerException if the field does not really exist.
+   */
+
+  final void setBooleanField(Object obj, boolean val) throws IOException
   {
       try
       {
          field.setBoolean(obj, val);
       }
+    catch (IllegalArgumentException _)
+      {
+       throw new InvalidClassException("incompatible field type for " +
+                                       obj.getClass().getName() + "." + name);
+      }
       catch(IllegalAccessException x)
       {
          throw new InternalError(x.getMessage());
       }
   }
 
-  final void setByteField(Object obj, byte val)
+  final void setByteField(Object obj, byte val) throws IOException
   {
       try
       {
          field.setByte(obj, val);
       }
+    catch (IllegalArgumentException _)
+      {
+       throw new InvalidClassException("incompatible field type for " +
+                                       obj.getClass().getName() + "." + name);
+      }
       catch(IllegalAccessException x)
       {
          throw new InternalError(x.getMessage());
       }
   }
 
-  final void setCharField(Object obj, char val)
+  final void setCharField(Object obj, char val) throws IOException
   {
       try
       {
          field.setChar(obj, val);
       }
+    catch (IllegalArgumentException _)
+      {
+       throw new InvalidClassException("incompatible field type for " +
+                                       obj.getClass().getName() + "." + name);
+      }
       catch(IllegalAccessException x)
       {
          throw new InternalError(x.getMessage());
       }
   }
 
-  final void setShortField(Object obj, short val)
+  final void setShortField(Object obj, short val) throws IOException
   {
       try
       {
          field.setShort(obj, val);
       }
+    catch (IllegalArgumentException _)
+      {
+       throw new InvalidClassException("incompatible field type for " +
+                                       obj.getClass().getName() + "." + name);
+      }
       catch(IllegalAccessException x)
       {
          throw new InternalError(x.getMessage());
       }
   }
 
-  final void setIntField(Object obj, int val)
+  final void setIntField(Object obj, int val) throws IOException
   {
       try
       {
          field.setInt(obj, val);
       }
+    catch (IllegalArgumentException _)
+      {
+       throw new InvalidClassException("incompatible field type for " +
+                                       obj.getClass().getName() + "." + name);
+      }
       catch(IllegalAccessException x)
       {
          throw new InternalError(x.getMessage());
       }
   }
 
-  final void setLongField(Object obj, long val)
+  final void setLongField(Object obj, long val) throws IOException
   {
       try
       {
          field.setLong(obj, val);
       }
+    catch (IllegalArgumentException _)
+      {
+       throw new InvalidClassException("incompatible field type for " +
+                                       obj.getClass().getName() + "." + name);
+      }
       catch(IllegalAccessException x)
       {
          throw new InternalError(x.getMessage());
       }
   }
 
-  final void setFloatField(Object obj, float val)
+  final void setFloatField(Object obj, float val) throws IOException
   {
       try
       {
          field.setFloat(obj, val);
       }
+    catch (IllegalArgumentException _)
+      {
+       throw new InvalidClassException("incompatible field type for " +
+                                       obj.getClass().getName() + "." + name);
+      }
       catch(IllegalAccessException x)
       {
          throw new InternalError(x.getMessage());
       }
   }
 
-  final void setDoubleField(Object obj, double val)
+  final void setDoubleField(Object obj, double val) throws IOException
   {
       try
       {
          field.setDouble(obj, val);
       }
+    catch (IllegalArgumentException _)
+      {
+       throw new InvalidClassException("incompatible field type for " +
+                                       obj.getClass().getName() + "." + name);
+      }
       catch(IllegalAccessException x)
       {
          throw new InternalError(x.getMessage());
       }
   }
 
-  final void setObjectField(Object obj, Object val)
+  final void setObjectField(Object obj, String valtype, Object val) throws 
IOException
   {
+    if (valtype == null ||
+       !typename.equals(valtype))
+      throw new InvalidClassException("incompatible field type for " + 
+                                     obj.getClass().getName() + "." + name);
+ 
     try
       {
        field.set(obj, val);
+      }
+    catch (IllegalArgumentException _)
+      {
+       throw new InvalidClassException("incompatible field type for " +
+                                       obj.getClass().getName() + "." + name); 
       }
     catch(IllegalAccessException x)
       {