On Feb 17, 2015, at 8:23 AM, Jeremy Boynes <jboy...@apache.org> wrote:
> 
> On Feb 17, 2015, at 4:34 AM, Konstantin Kolinko <knst.koli...@gmail.com> 
> wrote:
>> 
>> 2015-02-14 20:04 GMT+03:00 Stephan van Loendersloot (LIST)
>> <step...@republika.nl>:
>>> Hi everyone,
>>> 
>>> I have a question about this issue:
>>> https://bz.apache.org/bugzilla/show_bug.cgi?id=37466
>>> 
>>> I tried to switch to the latest Tomcat TagLibs implementation, but due to
>>> this fixed bug, it seems that posted form parameters aren't visible anymore
>>> in relative pages imported by <c:import> tags.
>>> 
>>> The JSTl specicication 1.2, Section 7.2 (page 57) states that relative URLs
>>> should be processed in the exact same way as the include action of the JSP
>>> specification (<jsp:include>) when used in the same application context.
>>> 
>>> The older, Jakarta TagLibs don't have this problem, but the Tomcat TagLibs
>>> do...
>>> 
>>> Can anyone tell me if the fix breaks the specification guidelines, or did I
>>> misread the whole thing?
>>> 
>> 
>> 
>> BZ 37466 fix (r495005) was committed 8 years ago.
>> 
>> You need to provide an example that reproduces your issue.  Wrapping a
>> request with overwritten getMethod() should not break parsing of POST
>> parameters.  The parameters parsing is done by tomcat internals and
>> operates on the original request, not on the wrapper.
> 
> I think there may be an issue here but would like an example to confirm. This 
> change was introduced between 1.1 and 1.2 and after GlassFish forked away.
> 
> The original thread that led to BZ 37466 related to HEAD requests where the 
> original HttpServletRequest was being passed to a RequestDispatcher. What I 
> took from the thread is that this would result an empty response from the 
> imported resource which the application was not expecting - it tried to parse 
> empty content and failed. The r495005 patch attempts to fix this by forcing 
> the method to GET, mirroring the semantic of an external absolute URL where 
> the JSTL spec mandates a GET request is made when using HTTP.
> 
> I’m not convinced this change was correct. For relative URLs it specifies the 
> “whole environment is available … as well as request parameters” and by 
> forcing the method to GET we are indicating that there is no request body to 
> parse and so parameters in the original POST body should be skipped. As 
> Konstantin said, Tomcat does not take account of wrappers so will still 
> extract parameters from the request body but that may not be true for other 
> containers or if the application code is parsing the request content.
> 
> Perhaps the issue here is actually with Tomcat’s implementation of 
> HttpServlet and/or DefaultServlet where it appears doHead() never returns 
> content even for dispatched includes. If these only suppressed content for 
> the original response but did return content for included responses then BZ 
> 37466 would not be an issue. This would also seem to be necessary to return 
> the same headers for HEAD as GET e.g. ensuring that the Content-Length 
> returned included bytes from the included resource.
> 
> Stephan, please can you submit an example showing what’s failing for you and 
> include info on which container you are using and what version of taglibs you 
> are upgrading from.
> 

In the original issue, a JSP page was using <c:import> to read a XML resource 
for parsing:
  <c:import url="/org/apache/taglibs/standard/tag/el/core/Test37466.xml" 
varReader="xmlSource”>

This is dispatched and handled by DefaultServlet:
  service() (from HttpServlet) uses getMethod() to determine that doHead() 
should be called
  doHead() calls serveResource() with content = false
  if content is false, serveResource() does not write any data to the response
  the value captured by the HttpResponseWrapper will be empty

If instead, a JSP is used to serve the included resource its _jspService method 
does not check the HTTP method and always emits content.

This can be verified with a simple JSP that does a <jsp:include> of a .txt or 
.jsp resource where the Content-Length header is different depending on whether 
a GET or HEAD request is made.

Similarly, HttpServlet’s doHead() method is implemented by wrapping the 
response in a NoBodyResponse and then calling doGet().

The patch for 37466 attempts to fix this by overriding getMethod() to always 
return GET. This avoids the issue with DefaultServlet but breaks other servlets 
that actually use the method to dispatch e.g. anything that expects to handle a 
POST request in doPost().

I think instead the fix should be in Tomcat’s HttpServlet and DefaultServlet 
which should not suppress the output if they being called as an include. I’ll 
open an issue against Tomcat and propose a patch for this.

—
Jeremy

Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail

Reply via email to