[cp-patches] RFC: Patch for duplicate entries in serialPersistentFields
Hi,
Here is a patch which (partially) fix a regression in kaffe. If you
define a class like that:
class A
{
private static final ObjectStreamField[] serialPersistentFields =
{
new ObjectStreamField("i", int.class),
new ObjectStreamField("i", int.class)
};
/* ... */
};
then the JDK throws an InvalidClassException in ObjectStreamClass.lookup.
This goes against the API. However it is true that we must take into
account the presence of duplicate fields. So I propose we throw an
InvalidClassException on writing the object if this sort of situation
happens.
I have written the attached patch for this.
Regards,
Guilhem.
ChangeLog entry:
2005-12-10 Guilhem Lavaux <[EMAIL PROTECTED]>
* java/io/ObjectOutputStream.java
(writeClassDescription): Throw an InvalidClassException if
fields is INVALID_FIELDS.
* java/io/ObjectStreamClass.java
(setFields): Make fields as INVALID if we detect duplicate
entries in serialPersistentFields.
Index: java/io/ObjectOutputStream.java
===
RCS file: /cvsroot/classpath/classpath/java/io/ObjectOutputStream.java,v
retrieving revision 1.63
diff -u -r1.63 ObjectOutputStream.java
--- java/io/ObjectOutputStream.java 1 Nov 2005 14:54:23 - 1.63
+++ java/io/ObjectOutputStream.java 10 Dec 2005 18:45:02 -
@@ -442,6 +442,10 @@
realOutput.writeByte(flags);
ObjectStreamField[] fields = osc.fields;
+
+ if (fields == ObjectStreamClass.INVALID_FIELDS)
+ throw new InvalidClassException("serialPersistentFields in class " +
osc.getName() + " is invalid");
+
realOutput.writeShort(fields.length);
ObjectStreamField field;
Index: java/io/ObjectStreamClass.java
===
RCS file: /cvsroot/classpath/classpath/java/io/ObjectStreamClass.java,v
retrieving revision 1.43
diff -u -r1.43 ObjectStreamClass.java
--- java/io/ObjectStreamClass.java 16 Sep 2005 17:24:04 - 1.43
+++ java/io/ObjectStreamClass.java 10 Dec 2005 18:45:02 -
@@ -63,6 +63,8 @@
public class ObjectStreamClass implements Serializable
{
+ static final ObjectStreamField[] INVALID_FIELDS = new ObjectStreamField[0];
+
/**
* Returns the ObjectStreamClass for cl.
* If cl is null, or is not Serializable,
@@ -608,6 +610,28 @@
fields = getSerialPersistentFields(cl);
if (fields != null)
{
+ ObjectStreamField[] fieldsName = new
ObjectStreamField[fields.length];
+ System.arraycopy(fields, 0, fieldsName, 0, fields.length);
+
+ Arrays.sort (fieldsName, new Comparator() {
+ public int compare(Object o1, Object o2)
+ {
+ ObjectStreamField f1 = (ObjectStreamField)o1;
+ ObjectStreamField f2 = (ObjectStreamField)o2;
+
+ return f1.getName().compareTo(f2.getName());
+ }
+ });
+
+ for (int i=1; i < fields.length; i++)
+ {
+ if
(fieldsName[i-1].getName().equals(fieldsName[i].getName()))
+ {
+ fields = INVALID_FIELDS;
+ return;
+ }
+ }
+
Arrays.sort (fields);
// Retrieve field reference.
for (int i=0; i < fields.length; i++)
___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] RFC: Patch for duplicate entries in serialPersistentFields
On Sat, 2005-12-10 at 20:16 +0100, Guilhem Lavaux wrote:
> Hi,
>
> Here is a patch which (partially) fix a regression in kaffe. If you
> define a class like that:
>
> class A
> {
> private static final ObjectStreamField[] serialPersistentFields =
> {
> new ObjectStreamField("i", int.class),
> new ObjectStreamField("i", int.class)
> };
>
> /* ... */
> };
>
> then the JDK throws an InvalidClassException in ObjectStreamClass.lookup.
> This goes against the API. However it is true that we must take into
> account the presence of duplicate fields. So I propose we throw an
> InvalidClassException on writing the object if this sort of situation
> happens.
Why not do similar and throw the InvalidClassException from
ObjectStreamClass.lookup()?
(But then document that clearly in our version of course.)
If you do like in your patch then at least document this behavior. It is
slightly confusing that ObjectStreamClass.lookup() returns a non-null
result, but getFields() suddenly returns an empty ObjectStreamField
array as if there are no Serializable fields. Now user code has no real
way to check whether that is because the ObjectStreamClass is invalid
(because of bogus serialPersistentFields) or because it just has no
serializable fields.
Cheers,
Mark
signature.asc
Description: This is a digitally signed message part
___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] RFC: Patch for duplicate entries in serialPersistentFields
Mark Wielaard wrote:
On Sat, 2005-12-10 at 20:16 +0100, Guilhem Lavaux wrote:
Hi,
Here is a patch which (partially) fix a regression in kaffe. If you
define a class like that:
class A
{
private static final ObjectStreamField[] serialPersistentFields =
{
new ObjectStreamField("i", int.class),
new ObjectStreamField("i", int.class)
};
/* ... */
};
then the JDK throws an InvalidClassException in ObjectStreamClass.lookup.
This goes against the API. However it is true that we must take into
account the presence of duplicate fields. So I propose we throw an
InvalidClassException on writing the object if this sort of situation
happens.
Why not do similar and throw the InvalidClassException from
ObjectStreamClass.lookup()?
(But then document that clearly in our version of course.)
Because we would not have the same API. ObjectStreamClass.lookup may not
throw any exception (except the default ones like Error, NPE, ...).
If you do like in your patch then at least document this behavior. It is
slightly confusing that ObjectStreamClass.lookup() returns a non-null
result, but getFields() suddenly returns an empty ObjectStreamField
array as if there are no Serializable fields. Now user code has no real
way to check whether that is because the ObjectStreamClass is invalid
(because of bogus serialPersistentFields) or because it just has no
serializable fields.
Ok. I may add some documentation on that.
Cheers,
Guilhem.
___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] RFC: Patch for duplicate entries in serialPersistentFields
Hi Guilhem, On Sun, 2005-12-11 at 16:41 +0100, Guilhem Lavaux wrote: > > Why not do similar and throw the InvalidClassException from > > ObjectStreamClass.lookup()? > > (But then document that clearly in our version of course.) > > Because we would not have the same API. ObjectStreamClass.lookup may not > throw any exception (except the default ones like Error, NPE, ...). O man, it is a checked exception... How ugly. I see why you don't want to do that. > > If you do like in your patch then at least document this behavior. It is > > slightly confusing that ObjectStreamClass.lookup() returns a non-null > > result, but getFields() suddenly returns an empty ObjectStreamField > > array as if there are no Serializable fields. Now user code has no real > > way to check whether that is because the ObjectStreamClass is invalid > > (because of bogus serialPersistentFields) or because it just has no > > serializable fields. > > Ok. I may add some documentation on that. One other suggestion. Might it make sense to do the caching, comparing and sorting in getSerialPersistentFields()? As it is done now we seem to copy and sort this array a lot of times. Cheers, Mark signature.asc Description: This is a digitally signed message part ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] RFC: Patch for duplicate entries in serialPersistentFields
Hi Mark, Mark Wielaard wrote: Hi Guilhem, On Sun, 2005-12-11 at 16:41 +0100, Guilhem Lavaux wrote: Why not do similar and throw the InvalidClassException from ObjectStreamClass.lookup()? (But then document that clearly in our version of course.) Because we would not have the same API. ObjectStreamClass.lookup may not throw any exception (except the default ones like Error, NPE, ...). O man, it is a checked exception... How ugly. I see why you don't want to do that. If you do like in your patch then at least document this behavior. It is slightly confusing that ObjectStreamClass.lookup() returns a non-null result, but getFields() suddenly returns an empty ObjectStreamField array as if there are no Serializable fields. Now user code has no real way to check whether that is because the ObjectStreamClass is invalid (because of bogus serialPersistentFields) or because it just has no serializable fields. Ok. I may add some documentation on that. One other suggestion. Might it make sense to do the caching, comparing and sorting in getSerialPersistentFields()? As it is done now we seem to copy and sort this array a lot of times. Yes we can factorize some code. However the double sort in setFields is necessary: first we compare the names and then we want the full sort relatively also to types. Cheers, Guilhem. ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
