Clarify on closing response before returning from dispatch forward()

[pmd1nh]

Servlet Spec. section 9.4. The Forward Method states:

Before the forward method of the RequestDispatcher interface returns without exception, the response content must be sent and committed, and closed by the servlet container, unless the request was put into the asynchronous mode.

When a wrapped response is used in the dispatch forward, which response object should be closed:

1. The wrapped response
2. The underlying/base response

[joakime]

My expectations ...

The wrapped response interface is used during the dispatch.
The underlying/base response is used once dispatch is done to ensure commit + close before returning to caller of RequestDispatch.forward().

IMO, the wrapped response should not be allowed to subvert or generally muck around with this rule. (such as preventing flush, or doing in-wrapper buffering, etc)

[gregw]

Current Jetty implementation is:

                // If we are not async and not closed already, then close via the possibly wrapped response.
                if (!baseRequest.getHttpChannelState().isAsync() && !baseResponse.getHttpOutput().isClosed())
                {
                    try
                    {
                        response.getOutputStream().close();
                    }
                    catch (IllegalStateException e)
                    {
                        response.getWriter().close();
                    }
                }

So the close is done via the wrapper. Note this does open the possibility that @joakime alludes to that a wrapper may subvert the intention to commit. But note also that the dispatching servlet may itself already be a target of a wrappers that does such subversion. I guess you could look at this as the include method is really just the "official way" to do such subversion (with a whole bunch of other confusing stuff thrown in).

I also think that being put into async mode during a forward is rather suspect... but not as much as being put into async mode during and include!

Either way, I think it may be worthwhile clarifying the language... assuming other containers agree with our interpretation.

[volosied]

Hello,

Phu brought up this question because of a change we made which caused JSF TCK failures.

MyFaces (and Mojarra, I believe) wrap the response when handing a jsp page before dispatching. However, at the end of the forward, our change closed the underlying response. This caused any further text not to be written out, therefore the TCK failed due to missing elements on the page.

https://github.com/apache/myfaces/blob/2.3.x/impl/src/main/java/org/apache/myfaces/view/jsp/JspViewDeclarationLanguage.java#L110-L115

Basically, per the TCK, it appears to indication option 1. However, the language is not exactly clear.

Peeking at the Tomcat code, it appears to close the wrapped response, too:

https://github.com/apache/tomcat/blob/main/java/org/apache/catalina/core/ApplicationDispatcher.java#L383-L395

I'm still a bit confused by the previous comments. Is the consensus so far for option 1 or option 2?

Thank you

EDIT: Here's the Faces 3.0 Spec regarding the RequestDispatcher.forward() for JSPs:

2.2.6. Render Response:
Jakarta Faces implementations must provide a default ViewHandler implementation that is capable of handling views written in Jakarta Server Pages as well as views written in the Faces View Declaration Language (VDL). In the case of Jakarta Server Pages, the ViewHandler must perform a RequestDispatcher.forward() call to a web application resource whose context-relative path is equal to the view identifier of the component tree.

[pmd1nh]

Additional information from spec and doc:

Chapter 9. Dispatching Requests
When building a web application, it is often useful to **forward processing of a request to another servlet**, or **to include the output of another servlet in the response.** The RequestDispatcher interface provides a mechanism to accomplish this.

https://javadoc.io/static/jakarta.servlet/jakarta.servlet-api/5.0.0/jakarta/servlet/RequestDispatcher.html#forward-jakarta.servlet.ServletRequest-jakarta.servlet.ServletResponse-

Forwards a request from a servlet to another resource (servlet, JSP file, or HTML file) on the server. This method allows one servlet to do preliminary processing of a request and **another resource to generate the response.**

All the above data point to the base response object in this context. The response content can not be generated from two (or more) resources in the forward case.

[pmd1nh]

Can we clarify this in
https://github.com/eclipse-ee4j/servlet-api/blob/master/spec/src/main/asciidoc/servlet-spec-body.adoc
?

@stuartwdouglas @markt-asf @arjantijms

[pmd1nh]

Check back on this clarification - what is response object (base vs wrapped) that should be closed in the forward case ?

[pmd1nh]

@markt-asf Can you please give this one last review and see if it can make into Servlet-6.1 or close it out. Thanks!