You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Sean Schofield <se...@gmail.com> on 2005/01/16 18:30:15 UTC
MYFACES-81 concerning serialization and violation of JSF spec
Here is a copy of my original bug report for:
http://issues.apache.org/jira/browse/MYFACES-81
The JIRA entry contains a patch to fix the problem.
========================================
StateManager.SerializedView should not implement Serializable
This inner class is not serializable in the 1.1 specification and the
fact that it is in the MyFaces implementation is a violation of the
specification.
It appears that this class was made serializable in order to implement
server-side state saving. Not only is this a violation of the spec,
but it does not even work properly. If the servlet container attempts
to serialize the session it throws an exception. This is because the
SerializedView class is not serializable even though it claims to
implement this interface. Because it is an inner class, it contains an
implicit reference to the parent class (in this case, this reference
ends up being JspStateManagerImpl.) Serialization fails because this
reference cannot be serialized (nor should it be.)
I have found another mechanism for handling server side state saving
that does not violate the spec. I've included a patch that will fix
this problem.
The solution is to include a private inner class called
_SerializedView that can implement serializable without violating the
spec. It has the same properties as the SerializedView class and
according to the spec, these properties will be serializable (b/c they
are ultimately generated from classes that implement StateHolder.) I
store this class in the session instead and everything works fine.
I believe the spec should change so that the SerializedView class is
its own standalone class that does implement serializable. That way,
implementations can use this class for both the client-side and
server-side state saving. That was what was being attempted in the
MyFaces implementation but this approach can't work b/c of the missing
serializable interface on SerializedView (and the implicit reference
to the parent class.) If the spec were changed, we could get rid of
this extra _SerializedView class that we are introducing and use the
proposed new class instead.
Re: MYFACES-81 concerning serialization and violation of JSF spec
Posted by Sean Schofield <se...@gmail.com>.
OK. I'm just waiting on someone to commit the patch (hint) so I can
use it with the latest source. Also once committed, the bug should be
marked RESOLVED/FIXED.
sean
Re: MYFACES-81 concerning serialization and violation of JSF spec
Posted by Sean Schofield <se...@gmail.com>.
Good point. I had forgotten that Object array was serializable. I
submitted a new patch per your suggestion. (Please remove old patch
from JIRA).
sean
On Sun, 16 Jan 2005 21:09:47 +0100, Manfred Geiler <ma...@apache.org> wrote:
> Sean,
> What I see from the patch, the _SerializedView is used twice and has
> only one purpose: enclose the structure and the state Object.
> So, why not take these two Objects and put them in a Object[2] array
> instead. Object array is serializable as well and we would not need the
> extra class.
> Just my 2 cents...
>
> Thanks,
> Manfred
Re: MYFACES-81 concerning serialization and violation of JSF spec
Posted by Manfred Geiler <ma...@apache.org>.
Sean,
What I see from the patch, the _SerializedView is used twice and has
only one purpose: enclose the structure and the state Object.
So, why not take these two Objects and put them in a Object[2] array
instead. Object array is serializable as well and we would not need the
extra class.
Just my 2 cents...
Thanks,
Manfred
Sean Schofield schrieb:
> Here is a copy of my original bug report for:
> http://issues.apache.org/jira/browse/MYFACES-81
>
> The JIRA entry contains a patch to fix the problem.
>
> ========================================
>
> StateManager.SerializedView should not implement Serializable
>
> This inner class is not serializable in the 1.1 specification and the
> fact that it is in the MyFaces implementation is a violation of the
> specification.
>
> It appears that this class was made serializable in order to implement
> server-side state saving. Not only is this a violation of the spec,
> but it does not even work properly. If the servlet container attempts
> to serialize the session it throws an exception. This is because the
> SerializedView class is not serializable even though it claims to
> implement this interface. Because it is an inner class, it contains an
> implicit reference to the parent class (in this case, this reference
> ends up being JspStateManagerImpl.) Serialization fails because this
> reference cannot be serialized (nor should it be.)
>
> I have found another mechanism for handling server side state saving
> that does not violate the spec. I've included a patch that will fix
> this problem.
>
> The solution is to include a private inner class called
> _SerializedView that can implement serializable without violating the
> spec. It has the same properties as the SerializedView class and
> according to the spec, these properties will be serializable (b/c they
> are ultimately generated from classes that implement StateHolder.) I
> store this class in the session instead and everything works fine.
>
> I believe the spec should change so that the SerializedView class is
> its own standalone class that does implement serializable. That way,
> implementations can use this class for both the client-side and
> server-side state saving. That was what was being attempted in the
> MyFaces implementation but this approach can't work b/c of the missing
> serializable interface on SerializedView (and the implicit reference
> to the parent class.) If the spec were changed, we could get rid of
> this extra _SerializedView class that we are introducing and use the
> proposed new class instead.
>
>
Re: MYFACES-81 concerning serialization and violation of JSF spec
Posted by Manfred Geiler <ma...@apache.org>.
+1 on sean schofields patch
I never felt happy with SerializedView beeing serializable.
SerializedView was serializable long ago, then we changed it to be
according to the spec and removed it again. Now, I remember once again,
why I removed the Serializable interface at that time - thanks sean. ;-)
Well, at the beginning of this new discussion I had a weird feeling and
a deja vu, but I could not remember what was the true reason I removed
the marker at that time. So, I added Serializable again, but - of course
- we should rather fix the cause and not the symptom. ;-)
Manfred
Matthias Wessendorf schrieb:
> Since I yesterday tested Kalle's suggestion.
> I just tested Sean's patch.
>
> Well, I removed the changes, suggested in Kalle's
> email and included Sean's patch.
>
> Also that works too.
>
> I am now +1 on commiting this.
>
> Any thoughts?
>
>
>>-----Original Message-----
>>From: Sean Schofield [mailto:sean.schofield@gmail.com]
>>Sent: Sunday, January 16, 2005 6:30 PM
>>To: MyFaces Development
>>Subject: MYFACES-81 concerning serialization and violation of JSF spec
>>
>>
>>Here is a copy of my original bug report for:
>>http://issues.apache.org/jira/browse/MYFACES-81
>>
>>The JIRA entry contains a patch to fix the problem.
>>
>>========================================
>>
>>StateManager.SerializedView should not implement Serializable
>>
>>This inner class is not serializable in the 1.1 specification
>>and the fact that it is in the MyFaces implementation is a
>>violation of the specification.
>>
>>It appears that this class was made serializable in order to
>>implement server-side state saving. Not only is this a
>>violation of the spec, but it does not even work properly. If
>>the servlet container attempts to serialize the session it
>>throws an exception. This is because the SerializedView class
>>is not serializable even though it claims to implement this
>>interface. Because it is an inner class, it contains an
>>implicit reference to the parent class (in this case, this
>>reference ends up being JspStateManagerImpl.) Serialization
>>fails because this reference cannot be serialized (nor should it be.)
>>
>>I have found another mechanism for handling server side state
>>saving that does not violate the spec. I've included a patch
>>that will fix this problem.
>>
>>The solution is to include a private inner class called
>>_SerializedView that can implement serializable without
>>violating the spec. It has the same properties as the
>>SerializedView class and according to the spec, these
>>properties will be serializable (b/c they are ultimately
>>generated from classes that implement StateHolder.) I store
>>this class in the session instead and everything works fine.
>>
>>I believe the spec should change so that the SerializedView
>>class is its own standalone class that does implement
>>serializable. That way, implementations can use this class
>>for both the client-side and server-side state saving. That
>>was what was being attempted in the MyFaces implementation
>>but this approach can't work b/c of the missing serializable
>>interface on SerializedView (and the implicit reference to
>>the parent class.) If the spec were changed, we could get rid
>>of this extra _SerializedView class that we are introducing
>>and use the proposed new class instead.
>>
>
>
>
>
RE: MYFACES-81 concerning serialization and violation of JSF spec
Posted by Matthias Wessendorf <ma...@matthias-wessendorf.de>.
Since I yesterday tested Kalle's suggestion.
I just tested Sean's patch.
Well, I removed the changes, suggested in Kalle's
email and included Sean's patch.
Also that works too.
I am now +1 on commiting this.
Any thoughts?
> -----Original Message-----
> From: Sean Schofield [mailto:sean.schofield@gmail.com]
> Sent: Sunday, January 16, 2005 6:30 PM
> To: MyFaces Development
> Subject: MYFACES-81 concerning serialization and violation of JSF spec
>
>
> Here is a copy of my original bug report for:
> http://issues.apache.org/jira/browse/MYFACES-81
>
> The JIRA entry contains a patch to fix the problem.
>
> ========================================
>
> StateManager.SerializedView should not implement Serializable
>
> This inner class is not serializable in the 1.1 specification
> and the fact that it is in the MyFaces implementation is a
> violation of the specification.
>
> It appears that this class was made serializable in order to
> implement server-side state saving. Not only is this a
> violation of the spec, but it does not even work properly. If
> the servlet container attempts to serialize the session it
> throws an exception. This is because the SerializedView class
> is not serializable even though it claims to implement this
> interface. Because it is an inner class, it contains an
> implicit reference to the parent class (in this case, this
> reference ends up being JspStateManagerImpl.) Serialization
> fails because this reference cannot be serialized (nor should it be.)
>
> I have found another mechanism for handling server side state
> saving that does not violate the spec. I've included a patch
> that will fix this problem.
>
> The solution is to include a private inner class called
> _SerializedView that can implement serializable without
> violating the spec. It has the same properties as the
> SerializedView class and according to the spec, these
> properties will be serializable (b/c they are ultimately
> generated from classes that implement StateHolder.) I store
> this class in the session instead and everything works fine.
>
> I believe the spec should change so that the SerializedView
> class is its own standalone class that does implement
> serializable. That way, implementations can use this class
> for both the client-side and server-side state saving. That
> was what was being attempted in the MyFaces implementation
> but this approach can't work b/c of the missing serializable
> interface on SerializedView (and the implicit reference to
> the parent class.) If the spec were changed, we could get rid
> of this extra _SerializedView class that we are introducing
> and use the proposed new class instead.
>