RequestAttributes in FrameworkPortlet [SPR-11295]

[spring-projects-issues]

Szczepan Kuzniarz opened SPR-11295 and commented

In Spring 2.5.x the type of RequestAttributes stored in RequestContextHolder while handling portlet request was always PortletRequestAttributes. It was guaranteed by the code in DispatcherPortlet.doRenderService/doActionService methods:

...
// Expose current RequestAttributes to current thread.
RequestAttributes previousRequestAttributes = RequestContextHolder.getRequestAttributes();
PortletRequestAttributes requestAttributes = new PortletRequestAttributes(request);
RequestContextHolder.setRequestAttributes(requestAttributes, this.threadContextInheritable);
...

Since Spring 3.2.x the type is indeterminate - sometimes it is PortletRequestAttributes and sometimes ServletRequestAttributes (if there is RequestContextListener present). This is caused by the following code in FrameworkPortlet.processRequest method:

...
// Expose current RequestAttributes to current thread.
RequestAttributes previousRequestAttributes = RequestContextHolder.getRequestAttributes();
PortletRequestAttributes requestAttributes = null;
if (previousRequestAttributes == null || previousRequestAttributes.getClass().equals(PortletRequestAttributes.class)) {
	requestAttributes = new PortletRequestAttributes(request);
	RequestContextHolder.setRequestAttributes(requestAttributes, this.threadContextInheritable);
}
...

Is the condition in if statement correct? It seems to me it should be:

if (previousRequestAttributes == null || !previousRequestAttributes.getClass().equals(PortletRequestAttributes.class)) {

---

Affects: 3.2.6, 4.0 GA

Referenced from: commits 1be3a6c, 7d94b5e, 2d892da

Backported to: 3.2.10

[spring-projects-issues]

Juergen Hoeller commented

This is actually meant to be that way: We are only overriding with a fresh PortletRequestAttributes instance if the previously bound instance was also of exact type PortletRequestAttributes, and in that sense compatible with our replacement. If any kind of custom RequestAttributes have been bound before, we're preserving it. The Grails framework relies on that behavior, and it generally works fine with our core arrangements as well.

Why specifically do have a RequestContextListener set up next to your DispatcherPortlet? Which code relies on the RequestContextListener there? It's generally not recommendable to rely on a specific RequestAttributes implementation type in your code... The whole point of the RequestAttributes abstraction is that it shouldn't matter which API those values come from, as long as they are referring to the currently processed HTTP request..

Juergen

[spring-projects-issues]

Torsten Hartwig commented

We are currently migrating our portlet application running on WebSphere Portal from Spring 2.5.6 to 4.0.5 and have problems with the code moved from DispatcherPortlet to FrameworkPortlet metioned by Szczepan. Why must a portlet check that previous RequestAttributes were bound to ThreadLocal by another portlet and not by a servlet? In WebSphere Portal there is a servlet application called Themes and Skins, which "represents" the entire portal web site and handles requests to all portlets, and if this servlet application is configured with Spring, no portlet will ever bind its request attributes to thread local, because there are servletrequestattributes already in existence.

Do you have any suggestions to solve our problem?

[spring-projects-issues]

Juergen Hoeller commented

Fixed through exposing local PortletRequestAttributes in case of pre-bound ServletRequestAttributes as well. Note that custom subclasses of either will be preserved if pre-bound (such as in the case of Grails); we're only going to replace standard RequestAttributes implementations that way.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

Guys, if you could verify that this works for you against 4.0.6.BUILD-SNAPSHOT or 3.2.10.BUILD-SNAPSHOT now, that'd be great. We intend to release 4.0.6 late tomorrow.

Juergen

[spring-projects-issues]

Torsten Hartwig commented

Yes, fix works.