Caching of portal parameters o.a.j.engine.servlet.ServletRequestImpl.getParameterMap() prevents subsequent RequestDispatcher invokation query string parameter merging

• Key: JS2-9
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Blocker
• Assignee: Ate Douma
• Reporter: Ate Douma
• Affects Version/s: 2.0-dev/cvs
• Fix Version/s: 2.0-dev/cvs, 2.0-M2, 2.0-FINAL

Description

Sorry for the long summary but I couldn't think of something shorter.

The o.a.j.engine.servlet.ServletRequestImpl.getParameterMap() only retrieves the portal parameters on first access and stores them in a Map which is returned on subsequent requests.

This seems smart from performance but actually is killing when a portlet wants to include a servlet or jsp with a RequestDispatcher and needs to specify additional query parameters (on the path).

On invokation of the RequestDispatcher Tomcat (at least) unwraps the ServletRequest chain of ServerRequestWrappers until it finds one of its own or one which is not an instance of ServletRequestWrapper (in org.apache.catalina.core.ApplicationDispatcher.wrapRequest()). Then it plugs a new HttpServletRequestWrapper into that part of the chain so it can merge any query string parameters on *top* of whatever parameters were already defined.

This all means that the set of request parameters will have been changed BELOW the o.a.j.engine.servlet.ServletRequestImpl (funny method name: wrapRequest() if you think of it ;-)
Because of the parameter caching these will however NOT become available to the included servlet or jsp.

Personnally I don't understand why Tomcat is not wrapping the request instead of plugging itself lower into the chain. Probably this should be reported as a bug to Tomcat.

But for the moment the current problem should be fixed I think by no longer caching the parameters in o.a.j.engine.servlet.ServletRequestImpl because its blocking my StrutsPortlet framework because its a common practice in struts applications to use query string parameters on actionforwards.

Ate

Comments

[Ate Douma]

This one is even trickier than I thought.

I traced back the org.apache.catalina.core.ApplicationDispatcher.wrapRequest() implementation causing all this to 2001-07-16 and based on Bugzilla bug #1902.

It turns out that this not wrapping of the passed on request but instead pluggin in somewhat below it in the chain is required per the Servlet 2.3 spec SRV.6.2.2 Wrapping Requests and Responses (for Filtering): The passed on request and response objects MUST be the same objects which were used to invoke!

But that means that even if parameter caching is turned off in o.a.j.engine.servlet.ServletRequestImpl it still won't be possible to dynamicaly inject additional parameters in a request (which is about the only purpose of the o.a.j.engine.servlet.ServletRequestImpl and needed to comply to the portlet specs) without the possibility of violating the specs for Servlet RequestDispatcher / Portlet PortletDispatcher handling of query string parameters.

According to those specs query parameters defined on the dispatcher path must take precedence over already defined parameters in the request used for invokation for the duration of the invokation.
This means that if the request has parameter X=2 and the query string defins X=3 the called servlet must see as values for parameter X: [3,2](3 taking precendence over 2).

But, because of the SRV.6.2.2 requirement Tomcat is forced to use the solution that have taken by injecting the parameters in a lower part of the request handler chain.

Thus if parameter X=2 is injected as portal parameter by o.a.j.engine.servlet.ServletRequestImpl it has to give it precendence over any already defined parameter in the request. Thus in this case if parameter X=3 was defined on the query string of an included servlet from the portlet it will see as values for parameter X: [2,3] which is a violation of the portlet spec PLT.16.1.1.

The only way out of this seems to me to handle query parameters on
request dispatcher paths within the portlet container and NOT rely on the servlet container as that clearly won't work.
So, when a PortletRequestDispatcher is invoked with query parameters on the path it should wrap the request and parse the query parameters itself and inject them in aditional overriden parameter getters.
This will NOT be in violation of SRV.6.2.2 because that only is defined for filtering which is not the case here.

For my Struts Portlet framework in which I already trap any RequestDispatcher invokations I will implement this myself. This isssue, although serious and should be handled, thus won't be blocking anymore rightnow.
I will try to create an generic solution so that if needed it can also be used by Jetspeed.

I just hope I'm not interpreting this all completely wrong and end up making a lot of changes for nothing ;-).

Anyway, as I read the specs now there seems to be a clear conflict between the requirements of SRV.6.2.2, SRV.8.1.1 (which is where PLT.16.1.1. is based upon) and the whole purpose of the ServletRequestWrapper which is meant to be used to extend/override behavior like parameter handling.

Ate

[Ate Douma]

One little addition.

In the portlet spec there is no mention of a restriction like SRV.6.2.2. It seems that for the current spec it is allowed for the portlet container to pass on wrapped request objects to a included servlet. But, once filtering is going to be supported in a future version of the spec it wouldn't supprise me to see this requirement also to be applied.
So, maybe this issue should be passed on to the spec leaders to look into? (I have no idea who's on that)

 Finally, closure to this problem.
While I was reinvestigating this problem because I had to document the workaround I implemented for the
Struts Bridge I determined a way to solve it generally for the Portal.
Side effect of this is that I now can remove the workaround from the Struts Bridge again (which is nice).

I fixed this by flushing the parameterMap when the wrapped request object was changed as Tomcat does
when it reuses the HttpServletRequestWrapper for an RequestDispatcher.include.