Bug 39417

Summary: ApplicationDispatcher.unwrapRequest() should not assume request is an extension of ServletRequestWrapper
Product: Tomcat 5 Reporter: Sean <seanrforeman>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED INVALID    
Severity: normal CC: mailmur
Priority: P2    
Version: 5.5.17   
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   

Description Sean 2006-04-26 18:43:00 UTC
If someone writes a wrapper for a request and does not extend
ServletRequestWrapper, tomcat blows up in unwrapRequest()

Example:

java.lang.ClassCastException: com.mpaygateway.web.XSSEncodedHttpServletRequest
	at
org.apache.catalina.core.ApplicationDispatcher.unwrapRequest(ApplicationDispatcher.java:814)
	at
org.apache.catalina.core.ApplicationDispatcher.doForward(ApplicationDispatcher.java:401)
	at
org.apache.catalina.core.ApplicationDispatcher.forward(ApplicationDispatcher.java:301)

Ye offending code snippet in method:

current = ((ServletRequestWrapper) current).getRequest();
Comment 1 william.barker 2006-04-26 20:00:37 UTC
<spec-quote version="2.4" section="8.2">
The parameters to these methods can be either the request and response 
arguments that were passed in via the service method of the javax.servlet 
interface, or instances of subclasses of the request and response wrapper 
classes that were introduced for version 2.3 of the
specification. In the latter case, the wrapper instances must wrap the request 
or response objects that the container passed into the service method.
</spec-quote>
Comment 2 Eric Jain 2007-10-09 09:02:43 UTC
Since the spec states that either the original request and response objects *or*
wrapped versions of these can be passed in, shouldn't you handle both cases
rather than throwing a ClassCastException in one case? Just checked with Jetty,
seems to work fine there...
Comment 3 Mailmur 2007-10-21 13:46:47 UTC
*** Bug 43669 has been marked as a duplicate of this bug. ***
Comment 4 Mailmur 2007-10-21 16:34:06 UTC
I made a quick test, and this works fine. My "requestDispatcher.forward(offReq,
offRes)" call went to the target webapp and reply written to the logfile. No
more ClassCastException.

[ org.apache.catalina.core.ApplicationDispatcher.java ]
  private void unwrapRequest(State state) {
    ...
      // FIX: avoid ClassCast exception if current one is ServletRequest
      // but not ServletRequestWrapper.
      if (!(current instanceof ServletRequestWrapper))
        break;

      // Advance to the next request in the chain
      previous = current;
      current = ((ServletRequestWrapper) current).getRequest();
    }
  }

  private void unwrapResponse(State state) {
    ...
      // FIX: avoid ClassCast exception if current one is ServletResponse
      // but not ServletResponseWrapper.
      if (!(current instanceof ServletResponseWrapper))
               break;

      // Advance to the next response in the chain
      previous = current;
      current = ((ServletResponseWrapper) current).getResponse();
    }
  }
Comment 5 Mark Thomas 2009-02-09 11:43:46 UTC
Looking at the OPs stack trace, the request/response objects being passed in are newly created objects rather than the originals or wrapped versions of the originals. This is not allowed by the spec.

The proposed patch would also breach the spec.
Comment 6 Mailmur 2009-02-15 10:21:15 UTC
Jetty=works fine same issues
Tomcat5=works fine same issues

Problem with the original tomcat source is that it creates a ClassCastException if you give it ServletRequest inteface. All it wants is ServletRequestWrapper and thats the problem.
Comment 7 Mailmur 2009-02-15 10:22:05 UTC
(In reply to comment #6)
> Jetty=works fine same issues
> Tomcat5=works fine same issues
> 
> Problem with the original tomcat source is that it creates a ClassCastException
> if you give it ServletRequest inteface. All it wants is ServletRequestWrapper
> and thats the problem.
> 
Opps, I mean Tomcat4=worked fine

Tomcat5=no good anymore.
Tomcat6=don't know, I havent tried my apps on it.