XMLEncoder can clobber arrays

[Chris Riesbeck]

My "gotcha" for the day... java.beans.XMLEncoder writes
a bean to a stream in XML form. But be careful if there
are static arrays! Here's what can go wrong.

First, a tiny class that "holds" an array of int's:

public class IntHolder {
private static final int[] initInts = { 0 };
private int[] myInts = initInts;

public int[] getInts() { return myInts; }
public void setInts(int[] ints) { myInts = ints; }
public String toString() {
return "myInts = [" + myInts[0] + "] initInts =[" + initInts[0] + "]";
}
}

Now some test code that creates an IntHolder, writes it
out in XML, and shows the arrays before and after:

import java.beans.*;
import java.io.*;

public class TestIntHolder {
public static void main(String[] args) throws IOException {
IntHolder h1 = new IntHolder();
h1.setInts(new int[]{ 3 });

System.out.println("h1: " + h1);

System.out.println("Writing h1");
XMLEncoder e = new XMLEncoder(new FileOutputStream("C:/temp/foo.xml"));
e.writeObject(h1);
e.close();

System.out.println("h1: " + h1);
}
}

Finally, the output (J2SDK 1.4.2):

h1: myInts = [3] initInts =[0] <-- initInts OK
Writing h1 <-- writing XML
h1: myInts = [3] initInts =[3] <-- initInts changed!

Note: If you make initInts non-static, this doesn't happen.

If you read the brief description of how XMLEncode works at

http://java.sun.com/products/jfc/tsc/articles/persistence4/#intro

you can probably guess at what's happening.

This seems like a bug to me -- encoding and writing should not
alter an object -- but it probably only breaks code that's asking
for trouble.

Anyone seen any discussion on this?

 

[Adam Jenkins]

My "gotcha" for the day... java.beans.XMLEncoder writes
a bean to a stream in XML form. But be careful if there
are static arrays! Here's what can go wrong. ...SNIP..
public class IntHolder {
private static final int[] initInts = { 0 };
private int[] myInts = initInts;

public int[] getInts() { return myInts; }
public void setInts(int[] ints) { myInts = ints; }
public String toString() {
return "myInts = [" + myInts[0] + "] initInts =[" + initInts[0] + "]";
}
}

Now some test code that creates an IntHolder, writes it
out in XML, and shows the arrays before and after:

import java.beans.*;
import java.io.*;

public class TestIntHolder {
public static void main(String[] args) throws IOException {
IntHolder h1 = new IntHolder();
h1.setInts(new int[]{ 3 });

System.out.println("h1: " + h1);

System.out.println("Writing h1");
XMLEncoder e = new XMLEncoder(new FileOutputStream("C:/temp/foo.xml"));
e.writeObject(h1);
e.close();

System.out.println("h1: " + h1);
}
}

Finally, the output (J2SDK 1.4.2):

h1: myInts = [3] initInts =[0] <-- initInts OK
Writing h1 <-- writing XML
h1: myInts = [3] initInts =[3] <-- initInts changed! ...SNIP..

This seems like a bug to me -- encoding and writing should not
alter an object -- but it probably only breaks code that's asking
for trouble.

I'd say this is because XMLEncoder is treating "ints" as an indexed
property. As part of XMLEncoder's algorithm for writing out the bean,
it creates a new IntHolder, and sets its properties to the same values
as the one being written. Since it's treating "ints" as an indexed
property, it does something like
copy.getInts()[0] = bean.getInts()[0];
which does seem reasonable except for the fact that really you're
sharing the same array of ints between both beans.

If it is correct for XMLEncoder to treat this as an indexed property,
then I think the bug is in your bean implementation. However, I'm not
sure if this really should be treated as an indexed property. I thought
to be an indexed property, you needed to implement the pattern:

public int getInts(int index);
public setInts(int index, int value);

and that the array versions were optional *in addition* to the above.

public int[] getInts();
public void setInts(int[] values);

See the Java Bean Spec, section 7.2.

However XMLEncoder seems to think that just the array versions also
indicate and indexed property. Does anyone know which is right, or is
the Bean Spec just fuzzy on this?

Adam

 

[Chris Smith]

If you read the brief description of how XMLEncode works at

http://java.sun.com/products/jfc/tsc/articles/persistence4/#intro

you can probably guess at what's happening.

Please enlighten me. I have read that article, and traced through the
code with a debugger, and still have no clue what's happening. I think
I'm having one of those senior moments, a little too early. What's the
deal here?

--
www.designacourse.com
The Easiest Way to Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[nos]

My "gotcha" for the day... java.beans.XMLEncoder writes
a bean to a stream in XML form. But be careful if there
are static arrays! Here's what can go wrong.

First, a tiny class that "holds" an array of int's:

public class IntHolder {
private static final int[] initInts = { 0 };
private int[] myInts = initInts;

public int[] getInts() { return myInts; }
public void setInts(int[] ints) { myInts = ints; }
public String toString() {
return "myInts = [" + myInts[0] + "] initInts =[" + initInts[0] + "]";
}
}

Now some test code that creates an IntHolder, writes it
out in XML, and shows the arrays before and after:

import java.beans.*;
import java.io.*;

public class TestIntHolder {
public static void main(String[] args) throws IOException {
IntHolder h1 = new IntHolder();
h1.setInts(new int[]{ 3 });

System.out.println("h1: " + h1);

System.out.println("Writing h1");
XMLEncoder e = new XMLEncoder(new FileOutputStream("C:/temp/foo.xml"));
e.writeObject(h1);
e.close();

System.out.println("h1: " + h1);
}
}

Finally, the output (J2SDK 1.4.2):

h1: myInts = [3] initInts =[0] <-- initInts OK
Writing h1 <-- writing XML
h1: myInts = [3] initInts =[3] <-- initInts changed!

Note: If you make initInts non-static, this doesn't happen.

If you read the brief description of how XMLEncode works at

http://java.sun.com/products/jfc/tsc/articles/persistence4/#intro

you can probably guess at what's happening.

This seems like a bug to me -- encoding and writing should not
alter an object -- but it probably only breaks code that's asking
for trouble.

Anyone seen any discussion on this?

I just found out about XMLEncoder yesterday. And I don't
know what beans are. What I did find what that I could write
out objects into XML format in a file. And that there was
nothing in the file that would identify these objects so I have
to be careful to read them in in the same order that was used
when I wrote them out.

BTW: I don't see the word bean in your code.

 

[Andrew Thompson]

....
BTW: I don't see the word bean in your code.

A bean is simply a class that has a no args constructor
for which all the readable/writable attributes have
a corresponding getAttribute/setAttribute method.

More a coding convention.

 

[Chris Riesbeck]

Please enlighten me. I have read that article, and traced through the
code with a debugger, and still have no clue what's happening. I think
I'm having one of those senior moments, a little too early. What's the
deal here?

The API docs for XMLEncoder just say it uses a "redundancy elimination"
algorithm to avoid writing default values for an object X. The obvious
way to do that would be to create a fresh bean B, then go through each
property and only write those properties of X with values different
than B. That wouldn't affect X in any way.

The article at Sun on writing persistence delegates doesn't give
a lot more detail but it does give a crucial hint: "XMLEncoder works
by cloning the object graph and recording the steps that were necessary
to create the clone. This way XMLEncoder has a "working copy" of the
object graph that mimics the steps XMLDecoder would take to decode
the file."

In other words, XMLEncoder starts with a fresh bean B, then
modifies it until it matches X. It uses java.bean.Statement objects
to hold the modification steps.

Furthermore, XMLEncoder handles array values specially. If the
array in B differs from the array in X, it modifies just those
elements in B that differ EVEN THOUGH the bean class may not
define a method that supports that. There's a line in the API
docs for Statement.execute() about the use of Array.get() and set()
that hints at this.

So the reason XMLEncode clobbered X was that it created a fresh
bean B, which had myInts == initInts. It then effectively did
B.myInts[0] = X.myInts[0]. That clobbered B.initInts which, since
it was a static variable, clobbered X.initInts.

If initInts is not static, B's initInts gets clobbered, but that
doesn't bother anybody.

 

[Chris Riesbeck]

My "gotcha" for the day... java.beans.XMLEncoder writes
a bean to a stream in XML form. But be careful if there
are static arrays! Here's what can go wrong. ..SNIP..
public class IntHolder {
private static final int[] initInts = { 0 };
private int[] myInts = initInts;

I'd say this is because XMLEncoder is treating "ints" as an indexed
property. As part of XMLEncoder's algorithm for writing out the bean,
it creates a new IntHolder, and sets its properties to the same values
as the one being written. Since it's treating "ints" as an indexed
property, it does something like
copy.getInts()[0] = bean.getInts()[0];
which does seem reasonable except for the fact that really you're
sharing the same array of ints between both beans.

Your analysis is dead on, as far as I can tell.

The only point of disagreement I have is with the "reasonable"
part. XMLEncoder's contract is to write a bean in XML. Writing
by default would never mean "modify" the bean.

The XMLEncoder algorithm used is an implementation of encoding.
It's not part of the contract. Another algorithm would be
to create a default bean and then only write those values
that differ from the default beans. That algorithm
would not clobber the bean's static array.

If it is correct for XMLEncoder to treat this as an indexed property,
then I think the bug is in your bean implementation.

I'm definitely not defending my IntHolder code. Just the
opposite. It's a terribly fragile design. I don't think
the bean has a bug though, IMO. I think XMLEncoder has the bug.
But it's a bug that only affects badly designed code.

However, I'm not
sure if this really should be treated as an indexed property. I thought
to be an indexed property, you needed to implement the pattern:

public int getInts(int index);
public setInts(int index, int value);

and that the array versions were optional *in addition* to the above.

public int[] getInts();
public void setInts(int[] values);

The Introspector agrees with you. I added

BeanInfo info = Introspector.getBeanInfo(IntHolder.class);
PropertyDescriptor[] props = info.getPropertyDescriptors();
for (int i = 0; i < props.length; ++i) {
System.out.println(props.getName() + " "
+ (props instanceof IndexedPropertyDescriptor));
}

to my test code and it printed

class false
ints false

i.e., Introspector does not think ints is an indexed property.
XMLEncoder is assuming it's safe to modify elements of arrays,
even when indexed properties are not involved.

 

[Adam Jenkins]

The XMLEncoder algorithm used is an implementation of encoding.
It's not part of the contract. Another algorithm would be
to create a default bean and then only write those values
that differ from the default beans. That algorithm
would not clobber the bean's static array.

That wouldn't work though, because it's possible that setting one
property also changes other property values, and that wouldn't be
detected by your algorithm. That's why it needs to actually set each
property of the copy bean to figure out what it needs to do. I think
this is reasonable, since setting a bean property to a new value should
not affect any other beans.

If it is correct for XMLEncoder to treat this as an indexed property,
then I think the bug is in your bean implementation.

However, I'm not
sure if this really should be treated as an indexed property. I thought
to be an indexed property, you needed to implement the pattern:

public int getInts(int index);
public setInts(int index, int value);

and that the array versions were optional *in addition* to the above.

public int[] getInts();
public void setInts(int[] values);

The Introspector agrees with you. I added

BeanInfo info = Introspector.getBeanInfo(IntHolder.class);
PropertyDescriptor[] props = info.getPropertyDescriptors();
for (int i = 0; i < props.length; ++i) {
System.out.println(props.getName() + " "
+ (props instanceof IndexedPropertyDescriptor));
}

to my test code and it printed

class false
ints false

i.e., Introspector does not think ints is an indexed property.
XMLEncoder is assuming it's safe to modify elements of arrays,
even when indexed properties are not involved.

I think this is the real bug; that XMLEncoder is treating this as an
indexed property when it shouldn't. If this were an indexed property,
then I would say your bean implementation was buggy for sharing the
array data between bean instances. But since it's not an indexed
property, then I agree with you that XMLEncoder is going a little too
far in its optimization. I don't think it should assume that it's safe
to modify the objects returned as property values from a bean.

Adam