You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2015/06/13 09:42:06 UTC

[Bug 58031] New: Posting data exceeding maxPostSize should result in HTTP 413.

https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

            Bug ID: 58031
           Summary: Posting data exceeding maxPostSize should result in
                    HTTP 413.
           Product: Tomcat 9
           Version: unspecified
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Catalina
          Assignee: dev@tomcat.apache.org
          Reporter: christopherleesimons@gmail.com

Created attachment 32821
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=32821&action=edit
Initial draft of patch to set status as 413 when maxPostSize is exceeded.

When data exceeding the value of the "maxPostSize" configuration parameter is
posted within a deployed application, the application sees an empty request
parameter map and cannot access the posted data, yet Tomcat returns a 200
status, indicating to the client that everything was processed successfully
(which likely will mislead the client).  The correct behavior is to return a
status of 413 ("Request Entity Too Large") to indicate that the request could
not be fully processed due to the size being too large.

Attached is a patch that will set the response status to 413 when Tomcat
discovers that the posted data exceeds maxPostSize.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

--- Comment #7 from Konstantin Kolinko <kn...@gmail.com> ---
(In reply to Mark Thomas from comment #5)
> I was thinking a using the existing attribute to trigger a 400 response and
> a new attribute to trigger a 413 response. That should cover all the current
> possibilities for parameter/part issues.

-1.

By my '-1' vote I mean that when "using a new attribute to trigger a 413
response" I am against changing the meaning of the existing attribute. If
something failed, the existing attribute should continue to indicate failure.

It can be
1) Globals.PARAMETER_PARSE_FAILED_ATTR to signal presence of an error
2) additional attribute to signal the kind of an error.

Originally when introducing PARAMETER_PARSE_FAILED_ATTR I have not defined what
its value is. It was documented as "absent = success", "any not-null value =
failure" with an intent to introduce different not-null values for different
use cases. Nowadays javadoc for Globals.PARAMETER_PARSE_FAILED_ATTR explicitly
mentions Boolean.TRUE as the value. Thus for compatibility it is better to go
with a separate attribute to communicate the reason of the failure.

Technically, PARAMETER_PARSE_FAILED_ATTR is a facade that exposes the value of
internal coyoteRequest.getParameters().isParseFailed() flag. For reference:
r1200218


Historically this attribute and FailedRequestFilter were introduced as a review
of CVE-2012-0022 fix that introduced "maxParameterCount" limit. The intent was
to be able to perform a simple test that none parameters were lost.

Also I think that exceeding the "maxParameterCount" limit also would better
result in response status 413.


I wish there were a Servlet API way to communicate parameter processing errors.

HttpServletRequest.getParts() and getPart() methods implement some way to
indicate errors by throwing an exception, but an IllegalStateException thrown
there is used to indicate both "size limit exceeded" and "missing
multipart-config" conditions. [1]

There exist "javax.servlet.error.exception" and other standard Request
attributes as defined in ch.10.9.1 of Servlet 3.1 specification, but IIRC they
are used only when performing an error processing dispatch.

[1]
http://docs.oracle.com/javaee/7/api/javax/servlet/http/HttpServletRequest.html#getParts%28%29

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

Christopher L. Simons <ch...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32821|Initial draft of patch to   |Initial draft of patch,
        description|set status as 413 when      |created against trunk.
                   |maxPostSize is exceeded.    |

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

--- Comment #2 from Mark Thomas <ma...@apache.org> ---
Agreed. The right response is definitely going to be application dependent.
Most of the problem stems from getParameterXXX() methods not declaring any
exceptions. We also run the risk of breaking lots of stuff if we start sending
413 responses where we currently send a 200 response.

On balance I think setting some a custom request attribute that an application
can check if they want to (and if not the current behaviour continues) along
the lines of Globals.PARAMETER_PARSE_FAILED_ATTR is going to be the way to go.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

--- Comment #6 from Konstantin Kolinko <kn...@gmail.com> ---
(In reply to Christopher L. Simons from comment #4)
> 
> Unconditionally sending a 400 from FailedRequestFilter upon failure seems
> incorrect as the HTTP spec states that 400 is to be used when the request
> failed "due to malformed syntax,"[1] 
> 
> [1] HTTP/1.1: Status Code Definitions
> http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.1

In my opinion, it would be nice to use 413, but it is not wrong to use 400.
"400" is just the generic error code in 4xx series. It says "it is client's
fault". Nothing more.

Likewise "500" is the generic error code in 5xx series.


Note that RFC2616 is obsolete.
Current specification for HTTP/1.1 status codes is RFC 7231:
http://tools.ietf.org/html/rfc7231#section-6.5.1

HTTP RFCs are listed at
https://wiki.apache.org/tomcat/Specifications#HTTP

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

Christopher L. Simons <ch...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |christopherleesimons@gmail.
                   |                            |com

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

--- Comment #5 from Mark Thomas <ma...@apache.org> ---
I was thinking a using the existing attribute to trigger a 400 response and a
new attribute to trigger a 413 response. That should cover all the current
possibilities for parameter/part issues.

Note RFC 2616 is obsolete.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by Violeta Georgieva <mi...@gmail.com>.
2015-06-24 18:37 GMT+03:00 Christopher Schultz <chris@christopherschultz.net
>:
>
> Mark,
>
> On 6/24/15 9:57 AM, Mark Thomas wrote:
> > On 24/06/2015 14:43, Violeta Georgieva wrote:
> >> Hi Mark,
> >>
> >> 2015-06-24 16:27 GMT+03:00 <bu...@apache.org>:
> >>>
> >>> https://bz.apache.org/bugzilla/show_bug.cgi?id=58031
> >>>
> >>> --- Comment #10 from Mark Thomas <ma...@apache.org> ---
> >>> I've patched trunk (9.0.x) to make the reason for the failure
available
> >> as a
> >>> request attribute that the FailedRequestFilter can then use to
provide a
> >> better
> >>> status code to the client.
> >>>
> >>> I'll give it a few days for folks to review and comment on the patch
> >> before
> >>> back-porting it.
> >>
> >> Should I wait for this in order to include it in 7.0.63?
> >
> > Up to you.
> >
> > Personally, I view this as more of an enhancement than a bug fix but
YMMV.
>
> +1
>
> I think it can wait for 7.0.64.

OK

Thanks,
Violeta

>
> -chris
>

Re: [Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,

On 6/24/15 9:57 AM, Mark Thomas wrote:
> On 24/06/2015 14:43, Violeta Georgieva wrote:
>> Hi Mark,
>>
>> 2015-06-24 16:27 GMT+03:00 <bu...@apache.org>:
>>>
>>> https://bz.apache.org/bugzilla/show_bug.cgi?id=58031
>>>
>>> --- Comment #10 from Mark Thomas <ma...@apache.org> ---
>>> I've patched trunk (9.0.x) to make the reason for the failure available
>> as a
>>> request attribute that the FailedRequestFilter can then use to provide a
>> better
>>> status code to the client.
>>>
>>> I'll give it a few days for folks to review and comment on the patch
>> before
>>> back-porting it.
>>
>> Should I wait for this in order to include it in 7.0.63?
> 
> Up to you.
> 
> Personally, I view this as more of an enhancement than a bug fix but YMMV.

+1

I think it can wait for 7.0.64.

-chris


Re: [Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by Mark Thomas <ma...@apache.org>.
On 24/06/2015 14:43, Violeta Georgieva wrote:
> Hi Mark,
> 
> 2015-06-24 16:27 GMT+03:00 <bu...@apache.org>:
>>
>> https://bz.apache.org/bugzilla/show_bug.cgi?id=58031
>>
>> --- Comment #10 from Mark Thomas <ma...@apache.org> ---
>> I've patched trunk (9.0.x) to make the reason for the failure available
> as a
>> request attribute that the FailedRequestFilter can then use to provide a
> better
>> status code to the client.
>>
>> I'll give it a few days for folks to review and comment on the patch
> before
>> back-porting it.
> 
> Should I wait for this in order to include it in 7.0.63?

Up to you.

Personally, I view this as more of an enhancement than a bug fix but YMMV.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by Violeta Georgieva <mi...@gmail.com>.
Hi Mark,

2015-06-24 16:27 GMT+03:00 <bu...@apache.org>:
>
> https://bz.apache.org/bugzilla/show_bug.cgi?id=58031
>
> --- Comment #10 from Mark Thomas <ma...@apache.org> ---
> I've patched trunk (9.0.x) to make the reason for the failure available
as a
> request attribute that the FailedRequestFilter can then use to provide a
better
> status code to the client.
>
> I'll give it a few days for folks to review and comment on the patch
before
> back-porting it.

Should I wait for this in order to include it in 7.0.63?

Thanks,
Violeta

> --
> You are receiving this mail because:
> You are the assignee for the bug.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>

[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

--- Comment #10 from Mark Thomas <ma...@apache.org> ---
I've patched trunk (9.0.x) to make the reason for the failure available as a
request attribute that the FailedRequestFilter can then use to provide a better
status code to the client.

I'll give it a few days for folks to review and comment on the patch before
back-porting it.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

Christopher L. Simons <ch...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32821|0                           |1
        is obsolete|                            |

--- Comment #3 from Christopher L. Simons <ch...@gmail.com> ---
Created attachment 32829
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=32829&action=edit
Add FailedRequestFilter note to maxPostSize documentation.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

Christopher L. Simons <ch...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|-----                       |default
            Product|Tomcat 9                    |Tomcat 6
          Component|Catalina                    |Catalina

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

--- Comment #4 from Christopher L. Simons <ch...@gmail.com> ---
(In reply to Christopher Schultz from comment #1)
> Look into how Globals.PARAMETER_PARSE_FAILED_ATTR is used to communicate
> this situation to the application and see if you can come up with something
> that might work better than your current proposal.

It appears that both checks for maxPostSize do result in
Globals.PARAMETER_PARSE_FAILED_ATTR being set: in Request#parseParts, an
exception is thrown, and in the finally block of the try/catch/finally block
parameters.setParseFailed(true) is set.  In Request#parseParameters, the
try/catch/finally block is exited without setting 'success' to true, and the
finally block calls parameters.setParseFailed(true) in this case.  So it
appears the original concern about return 200 in this case is addressed.

The documentation for maxParameterCount notes that FailedRequestFilter can be
used to reject requests that exceed this limit.  I've attached a patch to
include the same note in the maxPostSize documentation after verifying 400 is
returned in this case (as noted above).

> It does almost what you want, except that it returns a 400 response instead
> of 413

Unconditionally sending a 400 from FailedRequestFilter upon failure seems
incorrect as the HTTP spec states that 400 is to be used when the request
failed "due to malformed syntax,"[1] which isn't the case for maxPostSize and
maxParameterCount violations; in some parse failure cases 400 is correct, but
for these violations 413 seems more appropriate.  I'd propose implementing a
mechanism to indicate from the parsing code which status should be returned
from FailedRequestParameter.  Thoughts?

[1] HTTP/1.1: Status Code Definitions
http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.1

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

--- Comment #12 from Christopher L. Simons <ch...@gmail.com> ---
Any objections to applying the attached documentation patch?  I think it would
save many users from having to search and dig up this post to realize they can
use FailedRequestFilter to adjust the behavior that occurs when @maxPostSize is
exceeded.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

--- Comment #11 from Mark Thomas <ma...@apache.org> ---
No objections were raised so I have back-ported this to 8.0.x (for 8.0.25
onwards) and 7.0.x (for 7.0.64 onwards). I have also proposed it for 6.0.x.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

--- Comment #9 from Christopher Schultz <ch...@christopherschultz.net> ---
I'd prefer being able to set the response code using an init-param. If we want
to keep the current behavior and add on to it, it seems we need a second
attribute to indicate the second condition. In either case, I think it woul dbe
nice for the user to be able to customize the HTTP response code.

[I don't like returning 413 (Entity Too Large) for "too many request
parameters" unless it's actually a POST with a request entity.]

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #13 from Mark Thomas <ma...@apache.org> ---
Fixed in 6.0.x for 6.0.45 onwards.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

--- Comment #8 from Mark Thomas <ma...@apache.org> ---
(In reply to Konstantin Kolinko from comment #7)
> (In reply to Mark Thomas from comment #5)
> > I was thinking a using the existing attribute to trigger a 400 response and
> > a new attribute to trigger a 413 response. That should cover all the current
> > possibilities for parameter/part issues.
> 
> -1.

Fair enough. How about using a second attribute to set the response code? If
set use the defined code. If not set, use 400.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

Christopher L. Simons <ch...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 58031] Posting data exceeding maxPostSize should result in HTTP 413.

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58031

--- Comment #1 from Christopher Schultz <ch...@christopherschultz.net> ---
I'm iffy on this patch because it's not configurable (I'd like to point out
that the current behavior is also not configurable, and this definitely is an
improvement). The application itself might want to check this situation and
react differently, like it can when there are too many request parameters.

You should check out the FailedRequestFilter to see what happens in that case:
http://tomcat.apache.org/tomcat-8.0-doc/config/filter.html#Failed_Request_Filter

It does almost what you want, except that it returns a 400 response instead of
413, and only currently works for violations of maxParameterCount.

Look into how Globals.PARAMETER_PARSE_FAILED_ATTR is used to communicate this
situation to the application and see if you can come up with something that
might work better than your current proposal.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org