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 <[email protected]> wrote: > Fixed it without API breaks and new classes. > > On Fri, Nov 25, 2011 at 6:19 PM, James Carman > <[email protected]> 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 <[email protected]> >> 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 >>> <[email protected]> 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 >
