bug in FacesServlet?

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

bug in FacesServlet?

Romain Manni-Bucau
Hi guys,

org.apache.myfaces.context.servlet.FacesContextImpl#release does the
release but javax.faces.webapp.FacesServlet#service doesn't handle
context push/pop so if a JSF request does a JSF include (and retrigger
the servlet) it will likely reset too early the context.

Here a diagram hoping it helps:

-> request
        -> FacesServlet
                  -> setFacesContext
                         -> FacesServlet
                             -> anything relaunching a JSF "request"
org.apache.myfaces.view.jsp.JspViewDeclarationLanguage#buildView does
a forward for instance)
                                 -> setFacesContext
                                      -> setFacesContext
                                      -> releaseFacesContext
                  -> end of lifecycle // oops faces context is null
                  -> releaseFacesContext

Romain Manni-Bucau
@rmannibucau |  Blog | Github | LinkedIn | Tomitriber
Reply | Threaded
Open this post in threaded view
|

Re: bug in FacesServlet?

Leonardo Uribe
Hi

It could be possible, I do not have all the details, but I have the
impression that happens on jsp, or at least I have seen that before.
MyFaces does what the spec says, but this part could not be well defined.

It is clear the release should happen after the end of the lifecycle, but
since the rendering step is delegated to jsp in this case, after that point
there is no control, so the code just release in that point. The problem
here is we don't know what happens for different jsp containers. In my
understanding not every jsp implementation works the same, but as I said, I
have not entered into details.

regards,

Leonardo Uribe

2016-03-25 13:15 GMT-05:00 Romain Manni-Bucau <[hidden email]>:

> Hi guys,
>
> org.apache.myfaces.context.servlet.FacesContextImpl#release does the
> release but javax.faces.webapp.FacesServlet#service doesn't handle
> context push/pop so if a JSF request does a JSF include (and retrigger
> the servlet) it will likely reset too early the context.
>
> Here a diagram hoping it helps:
>
> -> request
>         -> FacesServlet
>                   -> setFacesContext
>                          -> FacesServlet
>                              -> anything relaunching a JSF "request"
> org.apache.myfaces.view.jsp.JspViewDeclarationLanguage#buildView does
> a forward for instance)
>                                  -> setFacesContext
>                                       -> setFacesContext
>                                       -> releaseFacesContext
>                   -> end of lifecycle // oops faces context is null
>                   -> releaseFacesContext
>
> Romain Manni-Bucau
> @rmannibucau |  Blog | Github | LinkedIn | Tomitriber
>
Reply | Threaded
Open this post in threaded view
|

SV: bug in FacesServlet?

Dan Østerberg
Hi,

I'd second that this is a bug. I don't know about the spec, but it certainly makes sense to support nested contexts and thus a push / pop like create / release. We stumbled upon this issue quite some time ago, when dispatching to an error page from and exception handler. And we solved it with an own FacesContext implementation that resets the previous context if the current context during release is our own.

Cheers,
Dan Østerberg

-----Opprinnelig melding-----
Fra: Leonardo Uribe [mailto:[hidden email]]
Sendt: 26. mars 2016 03:07
Til: MyFaces Discussion <[hidden email]>
Emne: Re: bug in FacesServlet?

Hi

It could be possible, I do not have all the details, but I have the impression that happens on jsp, or at least I have seen that before.
MyFaces does what the spec says, but this part could not be well defined.

It is clear the release should happen after the end of the lifecycle, but since the rendering step is delegated to jsp in this case, after that point there is no control, so the code just release in that point. The problem here is we don't know what happens for different jsp containers. In my understanding not every jsp implementation works the same, but as I said, I have not entered into details.

regards,

Leonardo Uribe

2016-03-25 13:15 GMT-05:00 Romain Manni-Bucau <[hidden email]>:

> Hi guys,
>
> org.apache.myfaces.context.servlet.FacesContextImpl#release does the
> release but javax.faces.webapp.FacesServlet#service doesn't handle
> context push/pop so if a JSF request does a JSF include (and retrigger
> the servlet) it will likely reset too early the context.
>
> Here a diagram hoping it helps:
>
> -> request
>         -> FacesServlet
>                   -> setFacesContext
>                          -> FacesServlet
>                              -> anything relaunching a JSF "request"
> org.apache.myfaces.view.jsp.JspViewDeclarationLanguage#buildView does
> a forward for instance)
>                                  -> setFacesContext
>                                       -> setFacesContext
>                                       -> releaseFacesContext
>                   -> end of lifecycle // oops faces context is null
>                   -> releaseFacesContext
>
> Romain Manni-Bucau
> @rmannibucau |  Blog | Github | LinkedIn | Tomitriber
>
Reply | Threaded
Open this post in threaded view
|

Re: SV: bug in FacesServlet?

Romain Manni-Bucau
FYI solving it is as easy as
https://github.com/apache/tomee/blob/a5088393dd32f167ab0869e955db194aa914dc67/tomee/tomee-myfaces/src/main/java/org/apache/tomee/myfaces/TomEEWorkaroundFacesServlet.java
Le 26 mars 2016 09:13, "Dan Østerberg" <[hidden email]> a écrit :

> Hi,
>
> I'd second that this is a bug. I don't know about the spec, but it
> certainly makes sense to support nested contexts and thus a push / pop like
> create / release. We stumbled upon this issue quite some time ago, when
> dispatching to an error page from and exception handler. And we solved it
> with an own FacesContext implementation that resets the previous context if
> the current context during release is our own.
>
> Cheers,
> Dan Østerberg
>
> -----Opprinnelig melding-----
> Fra: Leonardo Uribe [mailto:[hidden email]]
> Sendt: 26. mars 2016 03:07
> Til: MyFaces Discussion <[hidden email]>
> Emne: Re: bug in FacesServlet?
>
> Hi
>
> It could be possible, I do not have all the details, but I have the
> impression that happens on jsp, or at least I have seen that before.
> MyFaces does what the spec says, but this part could not be well defined.
>
> It is clear the release should happen after the end of the lifecycle, but
> since the rendering step is delegated to jsp in this case, after that point
> there is no control, so the code just release in that point. The problem
> here is we don't know what happens for different jsp containers. In my
> understanding not every jsp implementation works the same, but as I said, I
> have not entered into details.
>
> regards,
>
> Leonardo Uribe
>
> 2016-03-25 13:15 GMT-05:00 Romain Manni-Bucau <[hidden email]>:
>
> > Hi guys,
> >
> > org.apache.myfaces.context.servlet.FacesContextImpl#release does the
> > release but javax.faces.webapp.FacesServlet#service doesn't handle
> > context push/pop so if a JSF request does a JSF include (and retrigger
> > the servlet) it will likely reset too early the context.
> >
> > Here a diagram hoping it helps:
> >
> > -> request
> >         -> FacesServlet
> >                   -> setFacesContext
> >                          -> FacesServlet
> >                              -> anything relaunching a JSF "request"
> > org.apache.myfaces.view.jsp.JspViewDeclarationLanguage#buildView does
> > a forward for instance)
> >                                  -> setFacesContext
> >                                       -> setFacesContext
> >                                       -> releaseFacesContext
> >                   -> end of lifecycle // oops faces context is null
> >                   -> releaseFacesContext
> >
> > Romain Manni-Bucau
> > @rmannibucau |  Blog | Github | LinkedIn | Tomitriber
> >
>