Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (nike.apache.org: domain of ewald@mailbox.org designates
 80.241.60.215 as permitted sender)
Message-ID: <54AE989C.7050901@mailbox.org>
Date: Thu, 08 Jan 2015 15:47:56 +0100
From: Ewald Dieterich <ewald@mailbox.org>
User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64;
 rv:31.0) Gecko/20100101 Thunderbird/31.2.0
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: Reverse proxy: invalid Content-Length leads to 413 + 400 errors
 mixed up
References: <54AE5030.6030406@mailbox.org>
 <CALK=YjPf_z90=bzKXEn4aBdp3PKp_oQoatPYiicGoKNnHfpjcg@mail.gmail.com>
In-Reply-To: 
 <CALK=YjPf_z90=bzKXEn4aBdp3PKp_oQoatPYiicGoKNnHfpjcg@mail.gmail.com>
Content-Type: multipart/mixed;
 boundary="------------090006070500080101000403"

This is a multi-part message in MIME format.
--------------090006070500080101000403
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

On 01/08/2015 01:39 PM, Eric Covener wrote:
> On Thu, Jan 8, 2015 at 4:38 AM, Ewald Dieterich <ewald@mailbox.org> wrote:
>> Any ideas how to fix this so that this situation is handled as a single
>> error and not as two errors mixed up?
>
> in mod_proxy.c you will see at least 1 stanza like this:
>
>          status = ap_get_brigade(r->input_filters, temp_brigade,
>                                  AP_MODE_READBYTES, APR_BLOCK_READ,
>                                  MAX_MEM_SPOOL - bytes_read);
>          if (status != APR_SUCCESS) {
>              ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(01095)
>                            "prefetch request body failed to %pI (%s)"
>                            " from %s (%s)",
>                            p_conn->addr, p_conn->hostname ? p_conn->hostname: "",
>                            c->client_ip, c->remote_host ? c->remote_host: "");
>              return HTTP_BAD_REQUEST;
>          }
>
> The proper pattern in 2.4.x and later is to not return an error like that:
>
>              return ap_map_http_request_error(status, HTTP_BAD_REQUEST);
>
> In the case of that -102 error, the -102 will be returned verbatim
> instead (AP_FILTER_ERROR). Are you able to test and verify?

Hope I tested the right thing. ap_map_http_request_error() is not 
available in 2.4.x, so I added it from trunk and replaced the return 
statements in the stanzas above as suggested. I attached a patch with my 
changes to 2.4.10.

The response looks good now:

$ curl -i -H "Content-Length: a" http://frontend/
HTTP/1.1 413 Request Entity Too Large
Date: Thu, 08 Jan 2015 14:22:09 GMT
Server: Apache/2.4.10 (Debian)
Connection: close
Content-Type: text/html; charset=iso-8859-1

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>413 Request Entity Too Large</title>
</head><body>
<h1>Request Entity Too Large</h1>
The requested resource<br />/<br />
does not allow request data with GET requests, or the amount of data 
provided in
the request exceeds the capacity limit.
<hr>
<address>Apache/2.4.10 (Debian) Server at frontend Port 80</address>
</body></html>

But the access log entry is still wrong. Now a 200 is logged:

[...] "GET / HTTP/1.1" 200 590 "-" "curl/7.26.0"

I still see the -102 error:

[...] (-102)Unknown error -102: [client 10.128.128.95:46766] AH01095: 
prefetch request body failed to 10.8.19.114:80 (backend) from 
10.128.128.95 ()

I guess there are more changes in trunk that I would need to add?

--------------090006070500080101000403
Content-Type: text/x-patch;
 name="invalid_content_length.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="invalid_content_length.diff"

--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -324,7 +324,7 @@
                           " from %s (%s)", p_conn->addr,
                           p_conn->hostname ? p_conn->hostname: "",
                           c->client_ip, c->remote_host ? c->remote_host: "");
-            return HTTP_BAD_REQUEST;
+            return ap_map_http_request_error(status, HTTP_BAD_REQUEST);
         }
     }
 
@@ -475,7 +475,7 @@
                           " from %s (%s)", p_conn->addr,
                           p_conn->hostname ? p_conn->hostname: "",
                           c->client_ip, c->remote_host ? c->remote_host: "");
-            return HTTP_BAD_REQUEST;
+            return ap_map_http_request_error(status, HTTP_BAD_REQUEST);
         }
     }
 
@@ -624,7 +624,7 @@
                           " from %s (%s)", p_conn->addr,
                           p_conn->hostname ? p_conn->hostname: "",
                           c->client_ip, c->remote_host ? c->remote_host: "");
-            return HTTP_BAD_REQUEST;
+            return ap_map_http_request_error(status, HTTP_BAD_REQUEST);
         }
     }
 
@@ -807,7 +807,7 @@
                           " from %s (%s)",
                           p_conn->addr, p_conn->hostname ? p_conn->hostname: "",
                           c->client_ip, c->remote_host ? c->remote_host: "");
-            return HTTP_BAD_REQUEST;
+            return ap_map_http_request_error(status, HTTP_BAD_REQUEST);
         }
 
         apr_brigade_length(temp_brigade, 1, &bytes);
--- a/include/http_protocol.h
+++ b/include/http_protocol.h
@@ -502,6 +502,23 @@
  */
 AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer, apr_size_t bufsiz);
 
+/*
+ * Map specific APR codes returned by the filter stack to HTTP error
+ * codes, or the default status code provided. Use it as follows:
+ *
+ * return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
+ *
+ * If the filter has already handled the error, AP_FILTER_ERROR will
+ * be returned, which is cleanly passed through.
+ *
+ * These mappings imply that the filter stack is reading from the
+ * downstream client, the proxy will map these codes differently.
+ * @param rv APR status code
+ * @param status Default HTTP code should the APR code not be recognised
+ * @return Mapped HTTP status code
+ */
+AP_DECLARE(int) ap_map_http_request_error(apr_status_t rv, int status);
+
 /**
  * In HTTP/1.1, any method can have a body.  However, most GET handlers
  * wouldn't know what to do with a request body if they received one.
--- a/modules/http/http_filters.c
+++ b/modules/http/http_filters.c
@@ -1416,6 +1416,42 @@
     return ap_pass_brigade(f->next, b);
 }
 
+/*
+ * Map specific APR codes returned by the filter stack to HTTP error
+ * codes, or the default status code provided. Use it as follows:
+ *
+ * return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
+ *
+ * If the filter has already handled the error, AP_FILTER_ERROR will
+ * be returned, which is cleanly passed through.
+ *
+ * These mappings imply that the filter stack is reading from the
+ * downstream client, the proxy will map these codes differently.
+ */
+AP_DECLARE(int) ap_map_http_request_error(apr_status_t rv, int status)
+{
+    switch (rv) {
+    case AP_FILTER_ERROR: {
+        return AP_FILTER_ERROR;
+    }
+    case APR_EGENERAL: {
+        return HTTP_BAD_REQUEST;
+    }
+    case APR_ENOSPC: {
+        return HTTP_REQUEST_ENTITY_TOO_LARGE;
+    }
+    case APR_ENOTIMPL: {
+        return HTTP_NOT_IMPLEMENTED;
+    }
+    case APR_ETIMEDOUT: {
+        return HTTP_REQUEST_TIME_OUT;
+    }
+    default: {
+        return status;
+    }
+    }
+}
+
 /* In HTTP/1.1, any method can have a body.  However, most GET handlers
  * wouldn't know what to do with a request body if they received one.
  * This helper routine tests for and reads any message body in the request,

--------------090006070500080101000403--