Bug 43668 - ApplicationDispatcher.doForward for non-HTTP request is always NULL
ApplicationDispatcher.doForward for non-HTTP request is always NULL
Product: Tomcat 6
Classification: Unclassified
Component: Catalina
Other other
: P2 critical (vote)
: default
Assigned To: Tomcat Developers Mailing List
Depends on:
  Show dependency tree
Reported: 2007-10-21 13:16 UTC by Mailmur
Modified: 2007-10-25 20:10 UTC (History)
0 users

Patch against 6.0 trunk to fix this (4.58 KB, patch)
2007-10-21 19:30 UTC, william.barker
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mailmur 2007-10-21 13:16:07 UTC
This applies to both 6.x and 5.5x version !!



There is a severe always-NULL case in both T6.x and T5.5x sources if request is
non-HTTP forward. processRequest(req, res, state) method should use the original
"request" and "response" objects, but it gives "hrequest" and "hresponse" objects.

private void doForward(ServletRequest request, ServletResponse response) {
  // Identify the HTTP-specific request and response objects (if any)
  HttpServletRequest hrequest = null;
  if (request instanceof HttpServletRequest)
     hrequest = (HttpServletRequest) request;
  HttpServletResponse hresponse = null;
  if (response instanceof HttpServletResponse)
     hresponse = (HttpServletResponse) response;

   // Handle a non-HTTP forward by passing the existing request/response
   if ((hrequest == null) || (hresponse == null)) {

Calling program gets a NPE exception when forwarding a request.
java.lang.NullPointerException at 
Comment 1 william.barker 2007-10-21 19:30:53 UTC
Created attachment 21017 [details]
Patch against 6.0 trunk to fix this

This handles the case where the outer most wrapper is a ServletRequestWrapper,
but not a HttpServletRequestWrapper.  This further breaks 39417, but that one
is invalid anyway (according to the spec).
Comment 2 Mailmur 2007-10-22 06:36:09 UTC
39417: ApplicationDispatcher.unwrapRequest() should not assume servletRequestWrapper

What do you mean, is it that a bug 39417 is even further away to be fixed?
I made a quick test and posted a solution I used, a simple is-instanceof check
and bail out. Fix is posted at the end of 39417 report.

Comment 3 william.barker 2007-10-25 20:10:11 UTC
Fixed in the next 6.0 release with the attached patch.