I upgraded one of our in-house applications as part of regular maintenance. As part of this I upgraded the DWR version from 3.0.0-RC2-final to 3.0.2-RELEASE, and after doing this I found the application’s DWR calls were no longer working. It turns out that the forwardToString() method in the default web context changed from using HttpServletRequest FORWARD to using INCLUDE instead. As a result the Struts2 filter wasn’t getting triggered, because the mapping dispatchers in web.xml were REQUEST and FORWARD but not INCLUDE.
Adding INCLUDE to the filter dispatchers got the application working again but I’m concerned that if forwardToString() is actually including rather than forwarding it might affect other parts of the application in unpredictable ways. (And also it means the name of the method is now not descriptive of what’s really going on, and the documentation at http://directwebremoting.org/dwr/documentation/server/generic.html is no longer true.)
Was this change intentional?
Yes, this was part of an intentional bugfix, https://directwebremoting.atlassian.net/browse/DWR-659.
You could argue that DWR’s forwardToString method has probably always been named incorrectly, but historically I think it depends on if you choose a “logical” or technical viewpoint. From a logical viewpoint the developer will see this as a forward to a URL as the contents of this resource (nothing else added) will be returned.
But to perform this DWR will need to package the returned contents inside its own added envelope, so technically this will be an inclusion. DWR still initially used RequestDispatcher.forward() for this method which worked well at the time, but after some spec-compliant changes to Tomcat 8 we realized that we had to adhere harder to the spec and switched the implementation to RequestDispatcher.include().
Thanks for the heads-up regarding the incorrect info in the docs!
I have created https://github.com/directwebremoting/dwr/issues/13
Ok - so I added INCLUDE to the Struts2 filter mapping and really that should be all that’s needed. We’ll run the full set of tests to be certain it doesn’t affect anything it shouldn’t but all of our forwardToString() calls are simple and I don’t foresee any problems. Thanks for the update.