Bug 34956 - Tomcat should enforce the requirements from servlet 2.4 specification SRV.8.2
Tomcat should enforce the requirements from servlet 2.4 specification SRV.8.2
Status: RESOLVED FIXED
Product: Tomcat 5
Classification: Unclassified
Component: Servlet & JSP API
5.5.9
All other
: P2 normal (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2005-05-18 14:47 UTC by christian.buchegger
Modified: 2006-10-01 15:50 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description christian.buchegger 2005-05-18 14:47:47 UTC
Tomcat should enforce the requirements from servlet 2.4 specification SRV.8.2

SRV.8.2 Using a Request Dispatcher

"To use a request dispatcher, a servlet calls either the include method or
forward method of the RequestDispatcher interface. The parameters to these
methods can be either the request and response arguments that were passed in via
the service method of the Servlet interface, or instances of subclasses of the
request and response wrapper classes that have been 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.
The Container Provider must ensure that the dispatch of the request to a target
servlet occurs in the same thread of the same VM as the original request"

Justification:
==============
The absense of this enforcement leads to software beeing developed not following
the specification. The software cannot be deployed later on a container which
conforms to the above paragraph and hence must be changed before deployment.
This somehow contradicts the idea of having a standards based infrastructure.
Comment 1 Yoav Shapira 2005-06-01 04:38:53 UTC
How does Tomcat not enforce this?  Please provide a test case or steps to
reproduce the bug.
Comment 2 Carlin Rogers 2005-06-08 08:32:32 UTC
I've also noticed this issue specifically with the implementation of
the RequestDispatcher forward and handling a ServletRequestWrapper.
Seems to modify the value of the javax.servlet.forward.request_uri
request attribute incorrectly, according to servlet spec SRV.8.4.2.

Here are some specifics, at least with version 5.0.28.

Stepping through the Tomcat source in the debugger, it
appears that the request's requestURI field gets stomped
on in the forward method of the RequestDispatcher
(ApplicationDispatcher). I'm not sure Tomcat is handling
a ServletRequestWrapper correctly.

I gather the servlet spec says that users may wrap the
request/response objects with their own implementation.

Tomcat's ApplicationDispatcher gets the request from
the outer request (the ServletRequestWrapper), trying to
keep track of the previous wrapper and current wrapper
(or request) as it loops through to get the real request.
With a single wrapper, the value of "previous" is the
same as the original outer request. Then Tomcat calls...

((ServletRequestWrapper) previous).setRequest(wrapper);

which is the same as calling setRequest(wrapper) on the
incoming request. Tomcat does not get and save the value
of the original request URI. It calls setRequestURI(path)
on the wrapper, effectively changing the request URI of the
original incoming request to the path of the forward.

Then Tomcat sets the javax.servlet.forward.request_uri
attribute by calling getRequestURI() from the original
request... but that just got modified. Implying the
javax.servlet.forward.request_uri attribute is going to
get the value of the path for the forward.

You can test this on Tomcat by using a couple JSP to. Use
a HttpServletRequestWrapper in one JSP for the forward.

First create "result.jsp" to display the desired request attribute...

<%@ page language="java" contentType="text/html;charset=UTF-8"%>
<html>
    <head>
        <title>RequestDispatcher Test</title>
    </head>
    <body>
        <h1>Forward Request URI</h1>
        javax.servlet.forward.request_uri =
        <%= request.getAttribute("javax.servlet.forward.request_uri") %>
    </body>
</html>

Create a JSP to forward without using a wrapper, "forward.jsp"...

<%
    javax.servlet.RequestDispatcher rd =
        request.getRequestDispatcher("result.jsp");
    rd.forward(request, response);
%>

and a second forward using HttpServletRequestWrapper, "wrapperforward.jsp"...

<%
    HttpServletRequestWrapper wrapper = new HttpServletRequestWrapper(request);
    javax.servlet.RequestDispatcher rd =
        wrapper.getRequestDispatcher("result.jsp");
    rd.forward(wrapper, response);
%>

When you hit forward.jsp, the result page displays "/some-context/forward.jsp"
for the javax.servlet.forward.request_uri request attribute.
However, hit wrapperforward.jsp and the result page displays
"/some-context/result.jsp". From looking at the spec, I'd expect this 
should be "/some-context/wrapperforward.jsp".

I could probably create a patch for this specific case if you'd like.
Let me know.
Comment 3 Remy Maucherat 2005-06-08 11:06:36 UTC
Try to write less headache inducing comments in the future, as I didn't make it
to the end.

What I see is that: we wrap, we call wrequest.setRequestURI(requestURI) on *our*
ApplicationHttpRequest, and then after processing we discard that wrapper. What
you wrote does not make sense to me.
Comment 4 Brian DeHamer 2005-06-08 15:03:48 UTC
I'm experiencing the same problem that Carlin describes on Tomcat 5.0.28 as 
well.  The javax.servlet.forward.request_uri attribute is definitely NOT being 
set correctly when forwarding with a wrapped request.  Carlin's test case 
demonstrates the problem pretty clearly.
Comment 5 christian.buchegger 2005-06-08 15:53:18 UTC
I would say what Carlin describes is a different bug and is not what the
original request is about. I propose to open a new bug for this. 

Here a clarification what this bug is about:

Tomcat allows to do sth like that:

a)  create a own implementaion of javax.servlet.http.HttpServletRequest
public class MyRequestImpl implements javax.servlet.http.HttpServletRequest {
....
}

b) use that request objectt in a forward call
    javax.servlet.RequestDispatcher rd =
        wrapper.getRequestDispatcher("somepage.jsp");
    
    MyRequestImpl myRequest=new MyRequestImpl();
    rd.forward(myRequest, response);

This is against the spec, which clearly says either the original request or
HttpServletRequestWrapper has to be used.

You can find a real live example for this in the cocoon code. Sepcifically in
org.apache.cocoon.components.jsp.JSPEngineImplNamedDispatcherInclude from cocoon
 2.0.4.

Thanks
Comment 6 Carlin Rogers 2005-06-08 17:38:59 UTC
Christian, thanks for the clarification. My apologies for causing any
confusion here. I've opened a new Tomcat bug, 35270, for my specific
issue with regards to servlet spec SRV.8.4.2. Please disregard my
earlier comment in this bug.
Comment 7 Tim Funk 2005-07-27 14:52:46 UTC
Still confused. Marking as NEEDINFO. Requesting a war file to demonstrate the
bug if there still is one after wading through the converstation. (since a new
bug seemed to be open)
Comment 8 Mark Thomas 2006-09-29 18:38:23 UTC
The quote from the spec is:
<quote>
The parameters to these methods can be either...
</quote>

Can is not the same as must. The language as currently written does not mean
that the only permitted parameters are the original request/response or the
wrapped request/response. The example given in comment 5 is perfectly valid for
the current spec language.
Comment 9 william.barker 2006-09-29 19:41:17 UTC
(In reply to comment #8)
> The quote from the spec is:
> <quote>
> The parameters to these methods can be either...
> </quote>
> Can is not the same as must. The language as currently written does not mean
> that the only permitted parameters are the original request/response or the
> wrapped request/response. The example given in comment 5 is perfectly valid 
for
> the current spec language.

You are so wrong here:
<spec-quote version="2.4" section="14.2.5.1">
The request and response parameters must be either the same objects as were
passed to the calling servlet’s service method or be subclasses of the
ServletRequestWrapper or ServletResponseWrapper classes that wrap
them.
</spec-quote>

That means you can use the Request/Response that Tomcat passes in, or a 
HttpServletRequest/ResponseWrapper that wraps the Request/Response that Tomcat 
passes in. And that is it.  The example in #5 isn't valid wrt the spec.
Comment 10 Mark Thomas 2006-09-29 19:55:44 UTC
The spec is inconsistent. I was reading section 8.4.2 which uses "can", rather
than 14.2.5.1 which uses "must". I'll send an e-mail to the spec team.

In the meantime which do we apply? My guess is that 14.2.5.1 was the real
intention. Thoughts?
Comment 11 Mark Thomas 2006-09-30 14:42:33 UTC
(In reply to comment #10)
> The spec is inconsistent. I was reading section 8.4.2 which uses "can", rather
> than 14.2.5.1 which uses "must". I'll send an e-mail to the spec team.
> 
> In the meantime which do we apply? My guess is that 14.2.5.1 was the real
> intention. Thoughts?

Sorry, that should have be 8.2 not 8.4.2
Comment 12 Remy Maucherat 2006-10-01 03:12:38 UTC
If you add support for this "feature", don't forget to use
Globals.STRICT_SERVLET_COMPLIANCE, because besides annoying some people, this
won't have any real value.
Comment 13 Mark Thomas 2006-10-01 15:45:04 UTC
This has been fixed in SVN for 5.5.x and will be included in 5.5.21 onwards.
When ported to 6.0.x this code will be wrapped in a check for
Globals.STRICT_SERVLET_COMPLIANCE==true
Comment 14 Remy Maucherat 2006-10-01 15:50:56 UTC
It's likely more risky to put it in enabled by default in 5.5, so I think it
would be a good idea to add a flag there too, right ?