Re: [PATCH] Return HTTP 431 (Request Header Fields Too Large) for requests with large headers

[Ivan Zhakov]

On Tue, 27 Aug 2019 at 19:04, Roy T. Fielding  wrote:
>
> > On Aug 27, 2019, at 5:19 AM, Ivan Zhakov  wrote:
> >
> > On Wed, 14 Mar 2018 at 10:05, Ivan Zhakov  wrote:
> >>
> >> Hi,
> >>
> >> Please find patch that changes HTTPD to return HTTP 431 (Request
> >> Header Fields Too Large) for requests with large headers. This status
> >> code is defined by RFC 6585 [1]. Currently HTTPD returns generic HTTP
> >> 400 (Bad Request) status code for requests with large headers.
> >>
> >> [1] https://tools.ietf.org/html/rfc6585#section-5
> >>
> > Hey folks,
> >
> > Is there any feedback on the proposed patch?
> >
> > I believe that using the custom status code would be more appropriate
> > in these cases.
>
> I missed the patch, but +1 to the idea.  We should always use a specific code 
> when available.
>
The patch is available at the beginning of this thread:
https://lists.apache.org/thread.html/6d709e8882cd5bfa59c5e48bf3011fe914efa5c586b7d5a4e1f1b887@%3Cdev.httpd.apache.org%3E


-- 
Ivan Zhakov

Re: [PATCH] Return HTTP 431 (Request Header Fields Too Large) for requests with large headers

[Roy T. Fielding]

> On Aug 27, 2019, at 5:19 AM, Ivan Zhakov  wrote:
> 
> On Wed, 14 Mar 2018 at 10:05, Ivan Zhakov  wrote:
>> 
>> Hi,
>> 
>> Please find patch that changes HTTPD to return HTTP 431 (Request
>> Header Fields Too Large) for requests with large headers. This status
>> code is defined by RFC 6585 [1]. Currently HTTPD returns generic HTTP
>> 400 (Bad Request) status code for requests with large headers.
>> 
>> [1] https://tools.ietf.org/html/rfc6585#section-5
>> 
> Hey folks,
> 
> Is there any feedback on the proposed patch?
> 
> I believe that using the custom status code would be more appropriate
> in these cases.

I missed the patch, but +1 to the idea.  We should always use a specific code 
when available.

Roy

Re: [PATCH] Return HTTP 431 (Request Header Fields Too Large) for requests with large headers

[Ivan Zhakov]

On Wed, 14 Mar 2018 at 10:05, Ivan Zhakov  wrote:
>
> Hi,
>
> Please find patch that changes HTTPD to return HTTP 431 (Request
> Header Fields Too Large) for requests with large headers. This status
> code is defined by RFC 6585 [1]. Currently HTTPD returns generic HTTP
> 400 (Bad Request) status code for requests with large headers.
>
> [1] https://tools.ietf.org/html/rfc6585#section-5
>
Hey folks,

Is there any feedback on the proposed patch?

I believe that using the custom status code would be more appropriate
in these cases.

Thanks!

-- 
Ivan Zhakov

[PATCH] Return HTTP 431 (Request Header Fields Too Large) for requests with large headers

[Ivan Zhakov]

Hi,

Please find patch that changes HTTPD to return HTTP 431 (Request
Header Fields Too Large) for requests with large headers. This status
code is defined by RFC 6585 [1]. Currently HTTPD returns generic HTTP
400 (Bad Request) status code for requests with large headers.

[1] https://tools.ietf.org/html/rfc6585#section-5

-- 
Ivan Zhakov
Index: include/httpd.h
===
--- include/httpd.h (revision 1801146)
+++ include/httpd.h (working copy)
@@ -567,6 +567,7 @@
 ((x) == HTTP_LENGTH_REQUIRED)   || \
 ((x) == HTTP_REQUEST_ENTITY_TOO_LARGE) || \
 ((x) == HTTP_REQUEST_URI_TOO_LARGE) || \
+((x) == 
HTTP_REQUEST_HEADER_FIELDS_TOO_LARGE) || \
 ((x) == HTTP_INTERNAL_SERVER_ERROR) || \
 ((x) == HTTP_SERVICE_UNAVAILABLE) || \
 ((x) == HTTP_NOT_IMPLEMENTED))
Index: server/protocol.c
===
--- server/protocol.c   (revision 1801146)
+++ server/protocol.c   (working copy)
@@ -915,7 +915,7 @@
 if (value == NULL || r->server->limit_req_fieldsize >= strlen(value) )
 return 1;
 
-r->status = HTTP_BAD_REQUEST;
+r->status = HTTP_REQUEST_HEADER_FIELDS_TOO_LARGE;
 apr_table_setn(r->notes, "error-notes",
"Size of a request header field exceeds server limit.");
 ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00560) "Request "
@@ -952,15 +952,12 @@
 if (APR_STATUS_IS_TIMEUP(rv)) {
 r->status = HTTP_REQUEST_TIME_OUT;
 }
-else {
-r->status = HTTP_BAD_REQUEST;
-}
-
-/* ap_rgetline returns APR_ENOSPC if it fills up the buffer before
- * finding the end-of-line.  This is only going to happen if it
- * exceeds the configured limit for a field size.
- */
-if (rv == APR_ENOSPC) {
+else if (rv == APR_ENOSPC) {
+/* ap_rgetline returns APR_ENOSPC if it fills up the buffer 
before
+ * finding the end-of-line.  This is only going to happen if it
+ * exceeds the configured limit for a field size.
+ */
+r->status = HTTP_REQUEST_HEADER_FIELDS_TOO_LARGE;
 apr_table_setn(r->notes, "error-notes",
"Size of a request header field "
"exceeds server limit.");
@@ -971,6 +968,10 @@
   (field) ? field_name_len(field) : 0,
   (field) ? field : "");
 }
+else {
+r->status = HTTP_BAD_REQUEST;
+}
+
 return;
 }
 
@@ -1020,7 +1021,7 @@
 fold_len = last_len + len + 1; /* trailing null */
 
 if (fold_len >= (apr_size_t)(r->server->limit_req_fieldsize)) {
-r->status = HTTP_BAD_REQUEST;
+r->status = HTTP_REQUEST_HEADER_FIELDS_TOO_LARGE;
 /* report what we have accumulated so far before the
  * overflow (last_field) as the field with the problem
  */
Index: t/apache/limits.t
===
--- t/apache/limits.t   (revision 1801149)
+++ t/apache/limits.t   (working copy)
@@ -36,13 +36,13 @@
 my %xrcs = ('requestline-succeed' => 200,
 'requestline-fail'=> 414,
 'fieldsize-succeed'   => 200,
-'fieldsize-fail'  => 400,
+'fieldsize-fail'  => 431,
 'fieldcount-succeed'  => 200,
 'fieldcount-fail' => 400,
 'bodysize-succeed'=> 200,
 'bodysize-fail'   => 413,
 'merged_fieldsize-succeed' => 200,
-'merged_fieldsize-fail'=> 400,
+'merged_fieldsize-fail'=> 431,
 );
 
 my $res;