classpath
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Object serialization patch


From: Guilhem Lavaux
Subject: Re: Object serialization patch
Date: Tue, 24 Feb 2004 18:42:00 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5) Gecko/20031007

Jeroen Frijters wrote:
Guilhem Lavaux wrote:

Yes, I'm also not completely satisfied with this method. But there exists cases where it is needed. For example, if serialPersistentFields declares class types wrongly. The types are not checked previously in that case and this results in an IllegalArgumentException (which is wrong). The solution would be to check the types of all fields when we are building the ObjectStreamClass from serialPersistentFields.


Yes, that would definitely be better. Looking at it again, I think that
the check in setObjectField might be wrong. I'm not sure, but there
might be class loader issues (two classes with the same name, but loaded
by a different class loader) and I think it is legal to change the type
of a field to a super class.


In that case I'm proposing this patch. I have replaced the type checks in setXXXField by a type check in ObjectStreamClass.setClass by invoking a new method ObjectStreamField.checkFieldType. Here I'm only checking that the field is really assignable with a value which would have the type given in the serialPersistentFields list.

BTW, I've found that there was another error in fields sorting in setClass() this is now replaced by the good one.

Regards,
Guilhem.

Index: java/io/ObjectInputStream.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/io/ObjectInputStream.java,v
retrieving revision 1.38
diff -u -b -B -r1.38 ObjectInputStream.java
--- java/io/ObjectInputStream.java      10 Feb 2004 07:28:09 -0000      1.38
+++ java/io/ObjectInputStream.java      24 Feb 2004 17:15:57 -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();
@@ -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 -u -b -B -r1.30 ObjectStreamClass.java
--- java/io/ObjectStreamClass.java      2 Feb 2004 10:26:51 -0000       1.30
+++ java/io/ObjectStreamClass.java      24 Feb 2004 17:15:57 -0000
@@ -327,7 +327,7 @@
        i = 0; j = 0; k = 0;
        while (i < fields.length && j < exportedFields.length)
          {
-           int comp = 
fields[i].getName().compareTo(exportedFields[j].getName());
+           int comp = fields[i].compareTo(exportedFields[j]);
 
            if (comp < 0)
              {
@@ -344,10 +344,27 @@
                newFieldList[k] = exportedFields[j];
                newFieldList[k].setPersistent(true);
                newFieldList[k].setToSet(false);
+               try
+                 {
+                   newFieldList[k].lookupField(clazz);
+                   newFieldList[k].checkFieldType();
+                 }
+               catch (NoSuchFieldException _)
+                 {
+                 }
                j++;
              }
            else
              {
+               try
+                 {
+                   exportedFields[j].lookupField(clazz);
+                   exportedFields[j].checkFieldType();
+                 }
+               catch (NoSuchFieldException _)
+                 {
+                 }
+
                if (!fields[i].getType().equals(exportedFields[j].getType()))
                  throw new InvalidClassException
                    ("serialPersistentFields must be compatible with" +
@@ -554,6 +571,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 -u -b -B -r1.13 ObjectStreamField.java
--- java/io/ObjectStreamField.java      2 Feb 2004 10:26:51 -0000       1.13
+++ java/io/ObjectStreamField.java      24 Feb 2004 17:15:57 -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,6 +305,49 @@
     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;
+  }
+
+  /**
+   * This method check whether the field described by this
+   * instance of ObjectStreamField is compatible with the
+   * actual implementation of this field.
+   *
+   * @throws NullPointerException if this field does not exist
+   * in the real class.
+   * @throws InvalidClassException if the types are incompatible.
+   */
+  void checkFieldType() throws InvalidClassException
+  {
+    Class ftype = field.getType();
+
+    if (!ftype.isAssignableFrom(type))
+      throw new InvalidClassException
+       ("invalid field type for " + name +
+        " in class " + field.getDeclaringClass());
+  }
+
   public String toString ()
   {
     return "ObjectStreamField< " + type + " " + name + " >";

reply via email to

[Prev in Thread] Current Thread [Next in Thread]