Bug 48692 - Provide option to parse application/x-www-form-urlencoded PUT requests
Summary: Provide option to parse application/x-www-form-urlencoded PUT requests
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: trunk
Hardware: PC Windows Server 2003
: P2 enhancement (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
: 49964 53409 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-02-07 02:41 UTC by Mark Thomas
Modified: 2012-10-04 14:56 UTC (History)
2 users (show)



Attachments
Patch to provide request-entity parsing for arbitrary HTTP methods (10.46 KB, application/octet-stream)
2010-11-03 20:22 UTC, Christopher Schultz
Details
Patch to docs describing the previous patch (2.19 KB, patch)
2010-11-05 13:17 UTC, Christopher Schultz
Details | Diff
Patch for Tomcat 6.0 (7.16 KB, patch)
2012-08-14 00:54 UTC, Christopher Schultz
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Thomas 2010-02-07 02:41:33 UTC
Provide an option to treat application/x-www-form-urlencoded encoded parameters in a PUT request in a similar manner to application/x-www-form-urlencoded parameters in a POST request
Comment 1 Mark Thomas 2010-09-20 16:31:54 UTC
*** Bug 49964 has been marked as a duplicate of this bug. ***
Comment 2 Christopher Schultz 2010-09-20 16:40:52 UTC
I'll copy my comments from the duplicate bug #49964 just so they're more likely to be read:

My proposal is this (and I'm prepared to write the patches myself):

1. A new configuration attribute on the <Connector> element:
"parseBodyRequestParametersMethods" (suggestions for a better name are
welcome). The default value for this setting is "POST" which retains strict
servlet spec compliance.

2. Change catalina.connector.Request.parseParameters method to allow any method
mentioned in the above configuration attribute to undergo request body
conversion into request parameters. All other existing requirements remain in
place: the request body must be application/x-www-form-urlencoded, must fit
into maxPostSize, etc.

One question I have about this implementation: how should it behave under
org.apache.catalina.STRICT_SERVLET_COMPLIANCE=true? I believe that
org.apache.catalina.STRICT_SERVLET_COMPLIANCE=true should take precedence over
this setting, disable it's use, and issue a warning/error during startup when
they conflict.
Comment 3 Mark Thomas 2010-09-20 16:45:51 UTC
How about parsePutBodyParams (since POST will always be parsed) with values false (default) or true?

The logic for STRICT_SERVLET_COMPLIANCE in 7.0.x has been made more consistent. It now only changes the defaults for other options and explicitly setting an option always overrides STRICT_SERVLET_COMPLIANCE.

Therefore, assuming PUT parsing is off by default, it should be independent of STRICT_SERVLET_COMPLIANCE.
Comment 4 Christopher Schultz 2010-09-20 16:50:00 UTC
In one of the discussions on the tomcat-user list, the OP had complained that he couldn't pass "parameters" using methods such as DELETE, etc. I figured I'd let the configuration specify any HTTP method(s) that should be parsed, and not just PUT.

That's why I chose that stupidly-long name.

Good news about the STRICT_SERVLET_COMPLIANCE: since this is a new configuration parameter, it will simply default to adhere strictly to the servlet spec (that is, only POST gets it's body parsed) and any user-defined configuration will override it. Thanks.
Comment 5 Mark Thomas 2010-09-20 16:54:15 UTC
OK. A comma separated list of method names would make sense in that case. Although methods that are not allowed to have bodies should not be permitted.
Comment 6 Christopher Schultz 2010-09-20 17:02:15 UTC
Hmm. I was trying to avoid an exhaustive or partial list of HTTP methods specifically in the code: I wanted to let this code grow to work with any crazy non-standard methods a user might want to use, too (like "MYPUT"). Such non-standard HTTP methods might actually then allow, say, a REST-based service work without breaking other webapps that depend on spec-compliant handling of PUT bodies :)

I'll read-through the HTTP spec to look for methods for which bodies are actually forbidden, and we can reject those with a warning.
Comment 7 Christopher Schultz 2010-09-20 18:15:54 UTC
Okay, I've got my patch working in TC 7, and I've renamed the configuration setting to the less unwieldy "parseBodyMethods".
Comment 8 Christopher Schultz 2010-09-21 10:56:59 UTC
Mark, I've just glanced-over RFC 2616 and I can't find any HTTP methods (defined in section 9) that specifically prohibit a request body.

On the other hand, section 4.3 says:

"
The presence of a message-body in a request is signaled by the inclusion of a Content-Length or Transfer-Encoding header field in the request's message-headers. A message-body MUST NOT be included in a request if the specification of the request method (section 5.1.1) does not allow sending an entity-body in requests.
"

There's a short discussion that can be found here:
http://lists.w3.org/Archives/Public/ietf-http-wg/2006AprJun/0103.html

I tend to agree with one of the contributor's assertion that the principle of robustness applies, here: we shouldn't actively prohibit a request body from being sent, here.
Comment 9 Mark Thomas 2010-09-21 15:36:42 UTC
(In reply to comment #8)
> Mark, I've just glanced-over RFC 2616 and I can't find any HTTP methods
> (defined in section 9) that specifically prohibit a request body.

TRACE does. No others I could see.
Comment 10 Christopher Schultz 2010-11-03 20:22:40 UTC
Created attachment 26252 [details]
Patch to provide request-entity parsing for arbitrary HTTP methods

This patch adds a new configuration setting "parseBodyMethods" to all connectors. Simply specify the list of HTTP methods that should be parsed as a comma-separated list (e.g. "POST, PUT") to enable this parsing on those methods.

Requesting the TRACE method will result in an IllegalArgumentException and no changes to the Connector's configuration, to avoid an HTTP spec violation.
Comment 11 Christopher Schultz 2010-11-03 20:24:25 UTC
I ought to provide a documentation patch as well. One will be forthcoming.

Also: should I bother to log a WARN (or other level) message when this setting is used to alter the servlet-spec-compliant setting of simply "POST"?
Comment 12 Mark Thomas 2010-11-03 20:41:15 UTC
No need for a log message. We don't do it elsewhere in similar circumstances.
Comment 13 Christopher Schultz 2010-11-05 13:17:46 UTC
Created attachment 26259 [details]
Patch to docs describing the previous patch

...and the doc patch.
Comment 14 Christopher Schultz 2010-12-03 11:11:26 UTC
Fixed in trunk. Will be included in Tomcat 7.0.6.
Comment 15 Christopher Schultz 2012-08-09 16:44:24 UTC
*** Bug 53409 has been marked as a duplicate of this bug. ***
Comment 16 Christopher Schultz 2012-08-09 16:48:23 UTC
Proposed for 6.0.x.
Comment 17 Christopher Schultz 2012-08-14 00:54:10 UTC
Created attachment 29222 [details]
Patch for Tomcat 6.0

This patch is an amalgamation of the following patches:
http://svn.apache.org/viewvc?view=revision&revision=1041892 (initial patch)
http://svn.apache.org/viewvc?view=revision&revision=1043983 (fixes formatting; improved documentation)
http://svn.apache.org/viewvc?view=revision&revision=1049264 (improved javadoc)

Plus a modification of the original patch because Tomcat 6.0.x's Connector does not have an "initInternal" method.
Comment 18 Mark Thomas 2012-10-04 14:56:44 UTC
The patch has been applied to 6.0.x and will be included in 6.0.36 onwards.