You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by James Carman <ja...@carmanconsulting.com> on 2011/11/25 17:24:50 UTC
JavaSerializer doesn't call SerializableChecker...
While doing a bit of debugging today, I noticed that I didn't see that
nice serialization issue message that tells me which field is the
problem. So, I started looking into it (with some direction from
martin-g on IRC):
The Problem
In the new JavaSerializer class, it has a CheckerOutputStream which
extends ObjectOutputStream. The intent is to use the
ObjectOutputStream.writeObjectOverride() support. However, the
writeObjectOverride() method is never called unless you use the no-arg
constructor from the ObjectOutputStream class (which sets the
"enableOverride" flag to true). The CheckerOutputStream uses the
ObjectOutputStream(OutputStream) constructor in its constructor.
Worse yet, even if the writeObjectOverride() method were to be called,
it would create a StackOverflowError because it's calling the
super.writeObject() method which is what called it in the first place
(infinite recursion).
My Proposed Solution
I propose we do the following:
1. Change the following JavaSerializer method:
ObjectOutputStream newObjectOutputStream(OutputStream out)
to
ObjectOutput newObjectOutput(OutputStream out)
2. Change CheckerOutputStream to implement ObjectOutput instead of
extending ObjectOutputStream.
3. Have CheckerOutputStream delegate its ObjectOutput methods to an
internal ObjectOutputStream object.
Now, this would break the existing API because you're changing the
signature of the newObjectOutputSream() method, but I think this is
how it should have been written in the first place (to the interface,
not the class).
What do you guys think?
Re: JavaSerializer doesn't call SerializableChecker...
Posted by James Carman <ja...@carmanconsulting.com>.
I don't know about your solution. What if someone tries to use one of
the other methods of the ObjectOutputStream contract? You're not
delegating all of them. That's why I did what I did.
On Sun, Nov 27, 2011 at 6:50 AM, Martin Grigorov <mg...@apache.org> wrote:
> Fixed it without API breaks and new classes.
>
> On Fri, Nov 25, 2011 at 6:19 PM, James Carman
> <ja...@carmanconsulting.com> wrote:
>> JIRA created and patch attached:
>>
>> https://issues.apache.org/jira/browse/WICKET-4264
>>
>> I ended up actually not using ObjectOutput. I created a new interface
>> called ObjectWriter which only contains the writeObject() and close()
>> methods. The ObjectOutput interface had too many methods in it and we
>> were only using those two. Let me know if you guys want any more help
>> from me on this or if you want me to include a test case that shows
>> that SerializableChecker isn't called (don't know how to do that off
>> the top of my head just yet).
>>
>> After installing 1.5-SNAPSHOT locally, I do see the nice messages now
>> in my logs that tell me exactly where the field is that is causing the
>> problem. Life is good again.
>>
>> Thanks,
>>
>> James
>>
>> On Fri, Nov 25, 2011 at 11:28 AM, Martin Grigorov <mg...@apache.org> wrote:
>>> Hi,
>>>
>>> I think it is OK to break it.
>>> The public API is ISerializer while your proposed changes are
>>> internals of JavaSerializer, imo.
>>>
>>> On Fri, Nov 25, 2011 at 6:24 PM, James Carman
>>> <ja...@carmanconsulting.com> wrote:
>>>> While doing a bit of debugging today, I noticed that I didn't see that
>>>> nice serialization issue message that tells me which field is the
>>>> problem. So, I started looking into it (with some direction from
>>>> martin-g on IRC):
>>>>
>>>> The Problem
>>>>
>>>> In the new JavaSerializer class, it has a CheckerOutputStream which
>>>> extends ObjectOutputStream. The intent is to use the
>>>> ObjectOutputStream.writeObjectOverride() support. However, the
>>>> writeObjectOverride() method is never called unless you use the no-arg
>>>> constructor from the ObjectOutputStream class (which sets the
>>>> "enableOverride" flag to true). The CheckerOutputStream uses the
>>>> ObjectOutputStream(OutputStream) constructor in its constructor.
>>>> Worse yet, even if the writeObjectOverride() method were to be called,
>>>> it would create a StackOverflowError because it's calling the
>>>> super.writeObject() method which is what called it in the first place
>>>> (infinite recursion).
>>>>
>>>> My Proposed Solution
>>>>
>>>> I propose we do the following:
>>>>
>>>> 1. Change the following JavaSerializer method:
>>>>
>>>> ObjectOutputStream newObjectOutputStream(OutputStream out)
>>>>
>>>> to
>>>>
>>>> ObjectOutput newObjectOutput(OutputStream out)
>>>>
>>>> 2. Change CheckerOutputStream to implement ObjectOutput instead of
>>>> extending ObjectOutputStream.
>>>>
>>>> 3. Have CheckerOutputStream delegate its ObjectOutput methods to an
>>>> internal ObjectOutputStream object.
>>>>
>>>> Now, this would break the existing API because you're changing the
>>>> signature of the newObjectOutputSream() method, but I think this is
>>>> how it should have been written in the first place (to the interface,
>>>> not the class).
>>>>
>>>> What do you guys think?
>>>>
>>>
>>>
>>>
>>> --
>>> Martin Grigorov
>>> jWeekend
>>> Training, Consulting, Development
>>> http://jWeekend.com
>>>
>>
>
>
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com
>
Re: JavaSerializer doesn't call SerializableChecker...
Posted by Martin Grigorov <mg...@apache.org>.
Fixed it without API breaks and new classes.
On Fri, Nov 25, 2011 at 6:19 PM, James Carman
<ja...@carmanconsulting.com> wrote:
> JIRA created and patch attached:
>
> https://issues.apache.org/jira/browse/WICKET-4264
>
> I ended up actually not using ObjectOutput. I created a new interface
> called ObjectWriter which only contains the writeObject() and close()
> methods. The ObjectOutput interface had too many methods in it and we
> were only using those two. Let me know if you guys want any more help
> from me on this or if you want me to include a test case that shows
> that SerializableChecker isn't called (don't know how to do that off
> the top of my head just yet).
>
> After installing 1.5-SNAPSHOT locally, I do see the nice messages now
> in my logs that tell me exactly where the field is that is causing the
> problem. Life is good again.
>
> Thanks,
>
> James
>
> On Fri, Nov 25, 2011 at 11:28 AM, Martin Grigorov <mg...@apache.org> wrote:
>> Hi,
>>
>> I think it is OK to break it.
>> The public API is ISerializer while your proposed changes are
>> internals of JavaSerializer, imo.
>>
>> On Fri, Nov 25, 2011 at 6:24 PM, James Carman
>> <ja...@carmanconsulting.com> wrote:
>>> While doing a bit of debugging today, I noticed that I didn't see that
>>> nice serialization issue message that tells me which field is the
>>> problem. So, I started looking into it (with some direction from
>>> martin-g on IRC):
>>>
>>> The Problem
>>>
>>> In the new JavaSerializer class, it has a CheckerOutputStream which
>>> extends ObjectOutputStream. The intent is to use the
>>> ObjectOutputStream.writeObjectOverride() support. However, the
>>> writeObjectOverride() method is never called unless you use the no-arg
>>> constructor from the ObjectOutputStream class (which sets the
>>> "enableOverride" flag to true). The CheckerOutputStream uses the
>>> ObjectOutputStream(OutputStream) constructor in its constructor.
>>> Worse yet, even if the writeObjectOverride() method were to be called,
>>> it would create a StackOverflowError because it's calling the
>>> super.writeObject() method which is what called it in the first place
>>> (infinite recursion).
>>>
>>> My Proposed Solution
>>>
>>> I propose we do the following:
>>>
>>> 1. Change the following JavaSerializer method:
>>>
>>> ObjectOutputStream newObjectOutputStream(OutputStream out)
>>>
>>> to
>>>
>>> ObjectOutput newObjectOutput(OutputStream out)
>>>
>>> 2. Change CheckerOutputStream to implement ObjectOutput instead of
>>> extending ObjectOutputStream.
>>>
>>> 3. Have CheckerOutputStream delegate its ObjectOutput methods to an
>>> internal ObjectOutputStream object.
>>>
>>> Now, this would break the existing API because you're changing the
>>> signature of the newObjectOutputSream() method, but I think this is
>>> how it should have been written in the first place (to the interface,
>>> not the class).
>>>
>>> What do you guys think?
>>>
>>
>>
>>
>> --
>> Martin Grigorov
>> jWeekend
>> Training, Consulting, Development
>> http://jWeekend.com
>>
>
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com
Re: JavaSerializer doesn't call SerializableChecker...
Posted by James Carman <ja...@carmanconsulting.com>.
JIRA created and patch attached:
https://issues.apache.org/jira/browse/WICKET-4264
I ended up actually not using ObjectOutput. I created a new interface
called ObjectWriter which only contains the writeObject() and close()
methods. The ObjectOutput interface had too many methods in it and we
were only using those two. Let me know if you guys want any more help
from me on this or if you want me to include a test case that shows
that SerializableChecker isn't called (don't know how to do that off
the top of my head just yet).
After installing 1.5-SNAPSHOT locally, I do see the nice messages now
in my logs that tell me exactly where the field is that is causing the
problem. Life is good again.
Thanks,
James
On Fri, Nov 25, 2011 at 11:28 AM, Martin Grigorov <mg...@apache.org> wrote:
> Hi,
>
> I think it is OK to break it.
> The public API is ISerializer while your proposed changes are
> internals of JavaSerializer, imo.
>
> On Fri, Nov 25, 2011 at 6:24 PM, James Carman
> <ja...@carmanconsulting.com> wrote:
>> While doing a bit of debugging today, I noticed that I didn't see that
>> nice serialization issue message that tells me which field is the
>> problem. So, I started looking into it (with some direction from
>> martin-g on IRC):
>>
>> The Problem
>>
>> In the new JavaSerializer class, it has a CheckerOutputStream which
>> extends ObjectOutputStream. The intent is to use the
>> ObjectOutputStream.writeObjectOverride() support. However, the
>> writeObjectOverride() method is never called unless you use the no-arg
>> constructor from the ObjectOutputStream class (which sets the
>> "enableOverride" flag to true). The CheckerOutputStream uses the
>> ObjectOutputStream(OutputStream) constructor in its constructor.
>> Worse yet, even if the writeObjectOverride() method were to be called,
>> it would create a StackOverflowError because it's calling the
>> super.writeObject() method which is what called it in the first place
>> (infinite recursion).
>>
>> My Proposed Solution
>>
>> I propose we do the following:
>>
>> 1. Change the following JavaSerializer method:
>>
>> ObjectOutputStream newObjectOutputStream(OutputStream out)
>>
>> to
>>
>> ObjectOutput newObjectOutput(OutputStream out)
>>
>> 2. Change CheckerOutputStream to implement ObjectOutput instead of
>> extending ObjectOutputStream.
>>
>> 3. Have CheckerOutputStream delegate its ObjectOutput methods to an
>> internal ObjectOutputStream object.
>>
>> Now, this would break the existing API because you're changing the
>> signature of the newObjectOutputSream() method, but I think this is
>> how it should have been written in the first place (to the interface,
>> not the class).
>>
>> What do you guys think?
>>
>
>
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com
>
Re: JavaSerializer doesn't call SerializableChecker...
Posted by Martin Grigorov <mg...@apache.org>.
Hi,
I think it is OK to break it.
The public API is ISerializer while your proposed changes are
internals of JavaSerializer, imo.
On Fri, Nov 25, 2011 at 6:24 PM, James Carman
<ja...@carmanconsulting.com> wrote:
> While doing a bit of debugging today, I noticed that I didn't see that
> nice serialization issue message that tells me which field is the
> problem. So, I started looking into it (with some direction from
> martin-g on IRC):
>
> The Problem
>
> In the new JavaSerializer class, it has a CheckerOutputStream which
> extends ObjectOutputStream. The intent is to use the
> ObjectOutputStream.writeObjectOverride() support. However, the
> writeObjectOverride() method is never called unless you use the no-arg
> constructor from the ObjectOutputStream class (which sets the
> "enableOverride" flag to true). The CheckerOutputStream uses the
> ObjectOutputStream(OutputStream) constructor in its constructor.
> Worse yet, even if the writeObjectOverride() method were to be called,
> it would create a StackOverflowError because it's calling the
> super.writeObject() method which is what called it in the first place
> (infinite recursion).
>
> My Proposed Solution
>
> I propose we do the following:
>
> 1. Change the following JavaSerializer method:
>
> ObjectOutputStream newObjectOutputStream(OutputStream out)
>
> to
>
> ObjectOutput newObjectOutput(OutputStream out)
>
> 2. Change CheckerOutputStream to implement ObjectOutput instead of
> extending ObjectOutputStream.
>
> 3. Have CheckerOutputStream delegate its ObjectOutput methods to an
> internal ObjectOutputStream object.
>
> Now, this would break the existing API because you're changing the
> signature of the newObjectOutputSream() method, but I think this is
> how it should have been written in the first place (to the interface,
> not the class).
>
> What do you guys think?
>
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com