tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Luehe <Jan.Lu...@Sun.COM>
Subject Re: [Fwd: Re: Response not flushed before RD.forward() returns]
Date Sat, 21 Jan 2006 00:25:12 GMT


Remy Maucherat wrote On 01/20/06 03:45,:
> I tried to post that to dev, but there's apparently some issue since my 
> message hasn't been posted yet.
> 
> Rémy
> 
> -------- Original Message --------
> Subject: Re: Response not flushed before RD.forward() returns
> Date: Fri, 20 Jan 2006 12:08:30 +0100
> From: Remy Maucherat <remm@apache.org>
> To: Tomcat Developers List <dev@tomcat.apache.org>
> References: <200601200211.k0K2BA7j003539@sneezy.wilshire.com> 
> <43D069D8.6050707@sun.com>
> 
> Jan Luehe wrote:
> 
>>Bill Barker wrote On 01/19/06 18:11,:
>>
>>> 
>>>
>>>
>>>
>>>>-----Original Message-----
>>>>From: Jan Luehe [mailto:Jan.Luehe@Sun.COM] 
>>>>Sent: Thursday, January 19, 2006 5:57 PM
>>>>To: tomcat-dev@jakarta.apache.org
>>>>Subject: Response not flushed before RD.forward() returns
>>>>
>>>>Consider the following code snippet of a servlet's service() method:
>>>>
>>>> public class DispatcherServlet extends HttpServlet {
>>>>
>>>>   public void service(HttpServletRequest req, 
>>>>HttpServletResponse res)
>>>>       throws IOException, ServletException {
>>>>
>>>>     
>>>>request.getRequestDispatcher("/target").forward(request, response);
>>>>
>>>>     try {
>>>>         Thread.currentThread().sleep(60000);
>>>>     } catch (Exception ex) { }
>>>>   }
>>>>
>>>>where "target" prints some output to the response.
>>>>
>>>>The code currently returns the output printed by "target" only after
>>>>DispatcherServlet's service() method has finished, instead of right
>>>>before RD.forward() returns.
>>>>
>>>>This seems to be in violation of the Servlet Spec, which has this:
>>>>
>>>>SRV.8.4 ("The Forward Method"):
>>>>
>>>> Before the forward() method of the RequestDispatcher interface
>>>> returns, the response content must be sent and committed, and closed
>>>> by the servlet container.
>>>>
>>>>The code at the end of o.a.c.core.ApplicationDispatcher.doForward()
>>>>looks like this:
>>>>
>>>>       // This is not a real close in order to support error 
>>>>processing
>>>>       if ( log.isDebugEnabled() )
>>>>           log.debug(" Disabling the response for futher output");
>>>>
>>>>       if  (response instanceof ResponseFacade) {
>>>>           ((ResponseFacade) response).finish();
>>>>       } else {
>>>>           // Servlet SRV.6.2.2. The Resquest/Response may have been
>>>>wrapped
>>>>           // and may no longer be instance of RequestFacade
>>>>           if (log.isDebugEnabled()){
>>>>               log.debug( " The Response is vehiculed using 
>>>>a wrapper: "
>>>>                          + response.getClass().getName() );
>>>>           }
>>>>
>>>>           // Close anyway
>>>>           try {
>>>>               PrintWriter writer = response.getWriter();
>>>>               writer.close();
>>>>           } catch (IllegalStateException e) {
>>>>               try {
>>>>                   ServletOutputStream stream = 
>>>>response.getOutputStream();
>>>>                   stream.close();
>>>>               } catch (IllegalStateException f) {
>>>>                   ;
>>>>               } catch (IOException f) {
>>>>                   ;
>>>>               }
>>>>           } catch (IOException e) {
>>>>               ;
>>>>           }
>>>>       }
>>>>
>>>>In the above code sample, response will be an instance of
>>>>ResponseFacade, meaning it will be suspended instead of being flushed
>>>>and closed right away.
>>>>
>>>>Does anyone remember why the "response instanceof ResponseFacade"
>>>>check is there? I would have expected the "else" case to always be
>>>>executed.
>>>>
>>>
>>>Without ever actually having looked at ResponseFacade, I'd always assumed
>>>that ResponseFacade.finish called Response.finishResponse.  And I would have
>>>been wrong ;-).  This would have done the commit/send/close properly.
>>>
>>>I don't have time right now to dig through the SVN logs to see why it's this
>>>way, but suspended doesn't really look good enough to satisfy the spec.
>>
>>Yes. I'm afraid it's been like this forever.
> 
> 
> And it has to stay like this forever too: this needs to get some post
> processing done (like error pages, etc). If this person wants to flush,
> let him call flush.

Right, but flush does nothing once the response has been suspended.

In addition, consider the case where a request has been
forward-dispatched into a foreign context. If the target
servlet in the foreign context has set a status code on the response,
and the response is being suspended before returning from the
RD.forward(), the status code will be mapped to an error page
using the mappings of the *origin* context as the response
travels through the origin context's pipeline (and error valve)
on the way out.

The origin and target contexts may map the same status code to
different error pages, or the origin context may not even contain any
mapping for the given status code. In any case, I would find it
confusing for the origin context's mappings to be used, since
a RD.forward() is supposed to transfer control to the target.

I think it is cleaner for the response to be committed before
returning from RD.forward(), at the cost of not being mapped
to any error page. This is better than mapping the response to
a wrong error page. Or we could suspend the response when the
origin and target servlets share the same context, and commit it
when they reside in different contexts? Clearly, this would require
a spec clarification.


Jan


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message