3

I'm playing with "stateless"/"transient" views in JSF and I noticed that invoking ExternalContext.redirect() causes a new session to be created.

So, I dug into the Mojarra (2.2.15) code:

// -> com.sun.faces.context.ExternalContextImpl:653

public void redirect(String requestURI) throws IOException {

    FacesContext ctx = FacesContext.getCurrentInstance();
    doLastPhaseActions(ctx, true);

    if (ctx.getPartialViewContext().isPartialRequest()) {
        if (getSession(true) instanceof HttpSession &&
            ctx.getResponseComplete()) {
            throw new IllegalStateException();
        }
        PartialResponseWriter pwriter;
        ResponseWriter writer = ctx.getResponseWriter();
        if (writer instanceof PartialResponseWriter) {
            pwriter = (PartialResponseWriter) writer;
        } else {
            pwriter = ctx.getPartialViewContext().getPartialResponseWriter();
        }
        setResponseContentType("text/xml");
        setResponseCharacterEncoding("UTF-8");
        addResponseHeader("Cache-Control", "no-cache");
//        pwriter.writePreamble("<?xml version='1.0' encoding='UTF-8'?>\n");
        pwriter.startDocument();
        pwriter.redirect(requestURI);
        pwriter.endDocument();
    } else {
        ((HttpServletResponse) response).sendRedirect(requestURI);
    }
    ctx.responseComplete();

}

Note that this method is the same also on JSF-2.3 GitHub master, nevertheless the check is not present at all on MyFaces

I wonder why they included getSession(true) instanceof HttpSession, it seems pointless to me.

Can anyone explain the reason why shuch a check is there?

BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
Michele Mariotti
  • 7,372
  • 5
  • 41
  • 73
  • 1
    I'd say it is an omission in removing pieces of code when the 'transient' feature was implemented. But I'm no expert – Kukeltje Oct 24 '19 at 09:15
  • 2
    Maybe you can find or get closer to the answer via [this commit](https://github.com/javaserverfaces/mojarra/commit/3fefa2c9e24ac0bdc480b3d2949cbb929a5302a9) for [issue 2648](https://github.com/javaserverfaces/mojarra/issues/2652). I can't actually say why a session instance is needed to check if this is a Servlet context. There might be other less expansive ways to assert this. – Selaron Oct 24 '19 at 10:12
  • 1
    Yes MyFaces did it better by checking whether the response is an instance of `HttpServletResponse`, but even they didn't throw the intended exception. – BalusC Oct 24 '19 at 10:19

1 Answers1

3

This is indeed not the correct behavior. It should simply have checked if the response is an instance of HttpServletResponse. It should also have done that before invoking its sendRedirect() method which in its current form could have thrown a ClassCastException in a Portlet environment with a poorly implemented bridge.

Technical reason for the explicit instanceof checks on javax.servlet.http.* classes is because JSF is also available for a Portlet environment. There they use javax.portlet.* API instead of javax.servlet.http.* API. A well known example which you probably ever heard about is "Liferay".

BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
  • 1
    So that was just to check if the environment is Servlet or Portlet. I created the issue [Mojarra #4405](https://github.com/javaserverfaces/mojarra/issues/4405). Thank you :) – Michele Mariotti Oct 24 '19 at 12:06
  • @MicheleMariotti while I pointed you to the old mojarra github repo in order to find the commit, I don't know if this is still monitored. You should post the isse on the new repo at https://github.com/eclipse-ee4j/mojarra – Selaron Oct 24 '19 at 12:16
  • @Selaron thanks, I added the same issue to [eclipse-ee4j/mojarra #4649](https://github.com/eclipse-ee4j/mojarra/issues/4649) – Michele Mariotti Oct 24 '19 at 13:12