search

eclipse-ee4j / mojarra

Mojarra, a Jakarta Faces implementation
Other
158 stars 107 forks source link

NPE in ExternalContextImpl.encodeRedirectURL when no ViewRoot available #5383

Closed shadogray closed 5 months ago

shadogray commented 5 months ago

ExternalContextImpl.encodeRedirectURL tries to access ViewRoot.attributes w/o checking, if ViewRoot is set. In cases, when no ViewRoot has been identified (e.g. Window-ID redirect), this causes a NullPointerException!

StackTrace: ERROR [io.undertow.request] UT005023: Exception handling request to /pfad/NewFile.xhtml: java.lang.NullPointerException: Cannot invoke "jakarta.faces.component.UIViewRoot.getAttributes()" because the return value of "jakarta.faces.context.FacesContext.getViewRoot()" is null at jakarta.faces.impl@4.0.4//com.sun.faces.context.ExternalContextImpl.encodeRedirectURL(ExternalContextImpl.java:996) at jakarta.faces.impl@4.0.4//jakarta.faces.context.ExternalContextWrapper.encodeRedirectURL(ExternalContextWrapper.java:1064) at jakarta.faces.impl@4.0.4//jakarta.faces.context.ExternalContextWrapper.encodeRedirectURL(ExternalContextWrapper.java:1064)

BalusC commented 5 months ago

(e.g. Window-ID redirect)

Can you elaborate a bit more about the actual use case triggering this NPE?

shadogray commented 5 months ago

DeltaSpikeLifecycleWrapper tries to attach DS-WindowID and redirect 

        at deployment.pfad.war//org.apache.deltaspike.jsf.impl.util.ClientWindowHelper.constructRequestUrl(ClientWindowHelper.java:64)
        at deployment.pfad.war//org.apache.deltaspike.jsf.impl.util.ClientWindowHelper.handleInitialRedirect(ClientWindowHelper.java:83)
        at deployment.pfad.war//org.apache.deltaspike.jsf.impl.clientwindow.LazyClientWindow.decode(LazyClientWindow.java:57)
        at deployment.pfad.war//org.apache.deltaspike.jsf.impl.listener.request.DeltaSpikeLifecycleWrapper.attachWindow(DeltaSpikeLifecycleWrapper.java:154)
        at jakarta.faces.impl@4.0.4//jakarta.faces.webapp.FacesServlet.executeLifecyle(FacesServlet.java:690)
        at jakarta.faces.impl@4.0.4//jakarta.faces.webapp.FacesServlet.service(FacesServlet.java:449)
        at io.undertow.servlet@2.3.10.Final//io.undertow.servlet.handlers.ServletHandler.handleRequest(ServletHandler.java:74)
        at io.undertow.servlet@2.3.10.Final//io.undertow.servlet.handlers.FilterHandler.handleRequest(FilterHandler.java:81)
        at io.undertow.servlet@2.3.10.Final//io.undertow.servlet.handlers.security.ServletSecurityRoleHandler.handleRequest(ServletSecurityRoleHandler.java:62)
        at io.undertow.servlet@2.3.10.Final//io.undertow.servlet.handlers.ServletChain$1.handleRequest(ServletChain.java:68)
        at io.undertow.servlet@2.3.10.Final//io.undertow.servlet.handlers.ServletDispatchingHandler.handleRequest(ServletDispatchingHandler.java:36)
        at org.wildfly.security.elytron-web.undertow-server@4.0.0.Final//org.wildfly.elytron.web.undertow.server.ElytronRunAsHandler.lambda$handleRequest$1(ElytronRunAsHandler.java:68)
BalusC commented 5 months ago

Okay.

But apart from this I think the check for response encoding can be further improved. The existing impl makes little sense by basically storing and checking it in 5 different places (HttpServletRequest, FacesContext, UIViewRoot, HttpSession, HttpServletResponse). I want to take a closer look at it.

BalusC commented 5 months ago

I reviewed all the involved logic and did some cleanup https://github.com/eclipse-ee4j/mojarra/pull/5385