]> granicus.if.org Git - apache/commitdiff
Merge r1827362, r1828926, r1828927, r1829557, r1829573, r1829645, r1829657 from trunk:
authorYann Ylavic <ylavic@apache.org>
Fri, 22 Jun 2018 10:04:49 +0000 (10:04 +0000)
committerYann Ylavic <ylavic@apache.org>
Fri, 22 Jun 2018 10:04:49 +0000 (10:04 +0000)
core: ap_getline_core() reads nothing for n == 0.

PR62199: add worker parameter ResponseFieldSize to mod_proxy

Submitted By: Hank Ibell
Committed By: covener

add log id for r1828926

core: Add and handle AP_GETLINE_NOSPC_EOL flag in ap_rgetline_core().

This tells the ap_getline() family of functions to consume the end of line
when the buffer is exhausted.

PR 62198.

mod_proxy_http: make use of AP_GETLINE_NOSPC_EOL in ap_proxygetline().

Fixes response header thrown away after the previous one was considered too
large and truncated.

PR 62196.

core: forward flags to recursive/folding call to ap_rgetline_core().

We still need them when folding, other than AP_GETLINE_FOLD itself of course.

mod_proxy_http: follow up to r1829573: remain EBCDIC friendly.

Keep using ap_rgetline() as before r1829573, since ap_rgetline_core() is
EBCDIC agnostic.

Submitted by: ylavic, covener, covener, ylavic, ylavic, ylavic, ylavic
Reviewed by: ylavic, covener, rpluem

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1834093 13f79535-47bb-0310-9956-ffa450edef68

CHANGES
docs/manual/mod/mod_proxy.xml
include/ap_mmn.h
include/http_protocol.h
modules/proxy/mod_proxy.c
modules/proxy/mod_proxy.h
modules/proxy/mod_proxy_http.c
server/protocol.c

diff --git a/CHANGES b/CHANGES
index 352d44cc509a70c10a1d766a26171dd718c94676..948bb81ef1a1fcd8a4f44474dba4e61b0a7b43b2 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,17 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.34
 
+  *) mod_proxy_http: Fix response header thrown away after the previous one
+     was considered too large and truncated. PR 62196. [Yann Ylavic]
+
+  *) core: Add and handle AP_GETLINE_NOSPC_EOL flag for ap_getline() family
+     of functions to consume the end of line when the buffer is exhausted.
+     PR 62198. [Yann Ylavic]
+
+  *) mod_proxy_http: Add new worker parameter 'responsefieldsize' to
+     allow maximum HTTP response header size to be increased past 8192 
+     bytes.  PR62199.  [Hank Ibell <hwibell gmail.com>]
+
   *) mod_ssl: Extend SSLOCSPEnable with mode 'leaf' that only checks the leaf
      of a certificate chain.  PR62112.
      [Ricardo Martin Camarero <rickyepoderi yahoo.es>]
index 1f0c4e127a29b0c0e3ccdd84d3bbbb3c31a21a79..bc3dc5239b93b08eddb644081b51bdbdde656278 100644 (file)
@@ -1111,6 +1111,14 @@ ProxyPass "/mirror/foo/i" "!"
         to override the <directive>ProxyIOBufferSize</directive> for a specific worker.
         This must be at least 512 or set to 0 for the system default of 8192.
     </td></tr>
+    <tr><td>responsefieldsize</td>
+        <td>8192</td>
+        <td>Adjust the size of the proxy response field buffer. The buffer size
+            should be at least the size of the largest expected header size from
+            a proxied response. Setting the value to 0 will use the system
+            default of 8192 bytes.<br />
+        Available in Apache HTTP Server 2.4.34 and later.
+    </td></tr>
     <tr><td>keepalive</td>
         <td>Off</td>
         <td><p>This parameter should be used when you have a firewall between your
index 539fc1e119c0f9c481ef3520628c7b97346fd1bb..839228e29bfa9c3c6adef8896aed61ae688a3157 100644 (file)
  *                          ap_regcomp_default_cflag_by_name
  * 20120211.75 (2.4.30-dev) Add hostname_ex to proxy_worker_shared
  * 20120211.76 (2.4.30-dev) Add CONN_STATE_NUM to enum conn_state_e
- * 20120211.77 (2.4.34-dev)  add ap_exists_directive()
+ * 20120211.77 (2.4.34-dev) Add ap_exists_directive()
+ * 20120211.78 (2.4.34-dev) Add response_field_size to proxy_worker_shared 
+ * 20120211.79 (2.4.34-dev) Add AP_GETLINE_NOSPC_EOL flag to http_protocol.h
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20120211
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 77                  /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 79                  /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
index 29d887c61e7f09da94c69a9ccc179fea2f64adcd..11c7b2d187a88bb22f71a15a9b31388053ed4794 100644 (file)
@@ -606,7 +606,9 @@ AP_DECLARE(apr_status_t) ap_get_basic_auth_components(const request_rec *r,
 AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r, const char *uri);
 
 #define AP_GETLINE_FOLD 1 /* Whether to merge continuation lines */
-#define AP_GETLINE_CRLF 2 /*Whether line ends must be in the form CR LF */
+#define AP_GETLINE_CRLF 2 /* Whether line ends must be in the form CR LF */
+#define AP_GETLINE_NOSPC_EOL 4 /* Whether to consume up to and including the
+                                  end of line on APR_ENOSPC */
 
 /**
  * Get the next line of input for the request
index bc3410c45aecb1026f1f70d9fdd54e2a8beebb88..23364969191fad7598e7041fbf077b634cf071fd 100644 (file)
@@ -319,6 +319,14 @@ static const char *set_worker_param(apr_pool_t *p,
                                 (int)sizeof(worker->s->upgrade));
         }
     }
+    else if (!strcasecmp(key, "responsefieldsize")) {
+        long s = atol(val);
+        if (s < 0) {
+            return "ResponseFieldSize must be greater than 0 bytes, or 0 for system default.";
+        }
+        worker->s->response_field_size = (s ? s : HUGE_STRING_LEN);
+        worker->s->response_field_size_set = 1;
+    }
     else {
         if (set_worker_hc_param_f) {
             return set_worker_hc_param_f(p, s, worker, key, val, NULL);
index 63c2195ffe538c23e7d2f2d47ef188d11bb5607d..f74b7bc5415111b4b89544ebe029368e8d6fb5c3 100644 (file)
@@ -451,6 +451,8 @@ typedef struct {
     apr_interval_time_t interval;
     char      upgrade[PROXY_WORKER_MAX_SCHEME_SIZE];/* upgrade protocol used by mod_proxy_wstunnel */
     char      hostname_ex[PROXY_RFC1035_HOSTNAME_SIZE];  /* RFC1035 compliant version of the remote backend address */
+    apr_size_t   response_field_size; /* Size of proxy response buffer in bytes. */
+    unsigned int response_field_size_set:1;
 } proxy_worker_shared;
 
 #define ALIGNED_PROXY_WORKER_SHARED_SIZE (APR_ALIGN_DEFAULT(sizeof(proxy_worker_shared)))
index 7377a118c53973480b788b31f2084efa3ffd3740..01dc509010a6688f692b70593e83a496daf012c6 100644 (file)
@@ -28,6 +28,9 @@ static apr_status_t ap_proxy_http_cleanup(const char *scheme,
                                           request_rec *r,
                                           proxy_conn_rec *backend);
 
+static apr_status_t ap_proxygetline(apr_bucket_brigade *bb, char *s, int n,
+                                    request_rec *r, int flags, int *read);
+
 /*
  * Canonicalise http-like URLs.
  *  scheme is the scheme for the URL
@@ -1028,11 +1031,12 @@ static void ap_proxy_read_headers(request_rec *r, request_rec *rr,
 {
     int len;
     char *value, *end;
-    char field[MAX_STRING_LEN];
     int saw_headers = 0;
     void *sconf = r->server->module_config;
     proxy_server_conf *psc;
     proxy_dir_conf *dconf;
+    apr_status_t rc;
+    apr_bucket_brigade *tmp_bb;
 
     dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
     psc = (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module);
@@ -1047,8 +1051,29 @@ static void ap_proxy_read_headers(request_rec *r, request_rec *rr,
      */
     ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r,
                   "Headers received from backend:");
-    while ((len = ap_getline(buffer, size, rr, 1)) > 0) {
-        ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r, "%s", buffer);
+
+    tmp_bb = apr_brigade_create(r->pool, c->bucket_alloc);
+    while (1) {
+        rc = ap_proxygetline(tmp_bb, buffer, size, rr,
+                             AP_GETLINE_FOLD | AP_GETLINE_NOSPC_EOL, &len);
+
+        if (len <= 0)
+            break;
+
+        if (APR_STATUS_IS_ENOSPC(rc)) {
+            /* The header could not fit in the provided buffer, warn.
+             * XXX: falls through with the truncated header, 5xx instead?
+             */
+            int trunc = (len > 128 ? 128 : len) / 2;
+            ap_log_rerror(APLOG_MARK, APLOG_WARNING, rc, r, APLOGNO(10124)
+                    "header size is over the limit allowed by "
+                    "ResponseFieldSize (%d bytes). "
+                    "Bad response header: '%.*s[...]%s'",
+                    size, trunc, buffer, buffer + len - trunc);
+        }
+        else {
+            ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r, "%s", buffer);
+        }
 
         if (!(value = strchr(buffer, ':'))) {     /* Find the colon separator */
 
@@ -1116,16 +1141,6 @@ static void ap_proxy_read_headers(request_rec *r, request_rec *rr,
          */
         process_proxy_header(r, dconf, buffer, value);
         saw_headers = 1;
-
-        /* the header was too long; at the least we should skip extra data */
-        if (len >= size - 1) {
-            while ((len = ap_getline(field, MAX_STRING_LEN, rr, 1))
-                    >= MAX_STRING_LEN - 1) {
-                /* soak up the extra data */
-            }
-            if (len == 0) /* time to exit the larger loop as well */
-                break;
-        }
     }
 }
 
@@ -1137,23 +1152,19 @@ static int addit_dammit(void *v, const char *key, const char *val)
     return 1;
 }
 
-static
-apr_status_t ap_proxygetline(apr_bucket_brigade *bb, char *s, int n, request_rec *r,
-                             int fold, int *writen)
+static apr_status_t ap_proxygetline(apr_bucket_brigade *bb, char *s, int n,
+                                    request_rec *r, int flags, int *read)
 {
-    char *tmp_s = s;
     apr_status_t rv;
     apr_size_t len;
 
-    rv = ap_rgetline(&tmp_s, n, &len, r, fold, bb);
+    rv = ap_rgetline(&s, n, &len, r, flags, bb);
     apr_brigade_cleanup(bb);
 
-    if (rv == APR_SUCCESS) {
-        *writen = (int) len;
-    } else if (APR_STATUS_IS_ENOSPC(rv)) {
-        *writen = n;
+    if (rv == APR_SUCCESS || APR_STATUS_IS_ENOSPC(rv)) {
+        *read = (int)len;
     } else {
-        *writen = -1;
+        *read = -1;
     }
 
     return rv;
@@ -1184,7 +1195,8 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
                                             proxy_server_conf *conf,
                                             char *server_portstr) {
     conn_rec *c = r->connection;
-    char buffer[HUGE_STRING_LEN];
+    char *buffer;
+    char fixed_buffer[HUGE_STRING_LEN];
     const char *buf;
     char keepchar;
     apr_bucket *e;
@@ -1193,6 +1205,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
     int len, backasswards;
     int interim_response = 0; /* non-zero whilst interim 1xx responses
                                * are being read. */
+    apr_size_t response_field_size = 0;
     int pread_len = 0;
     apr_table_t *save_table;
     int backend_broke = 0;
@@ -1220,6 +1233,20 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
     bb = apr_brigade_create(p, c->bucket_alloc);
     pass_bb = apr_brigade_create(p, c->bucket_alloc);
 
+    /* Only use dynamically sized buffer if user specifies ResponseFieldSize */
+    if(backend->worker->s->response_field_size_set) {
+        response_field_size = backend->worker->s->response_field_size;
+
+        if (response_field_size != HUGE_STRING_LEN)
+            buffer = apr_pcalloc(p, response_field_size);
+        else
+            buffer = fixed_buffer;
+    }
+    else {
+        response_field_size = HUGE_STRING_LEN;
+        buffer = fixed_buffer;
+    }
+
     /* Setup for 100-Continue timeout if appropriate */
     if (do_100_continue) {
         apr_socket_timeout_get(backend->sock, &old_timeout);
@@ -1249,11 +1276,11 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
 
         apr_brigade_cleanup(bb);
 
-        rc = ap_proxygetline(backend->tmp_bb, buffer, sizeof(buffer),
+        rc = ap_proxygetline(backend->tmp_bb, buffer, response_field_size,
                              backend->r, 0, &len);
         if (len == 0) {
             /* handle one potential stray CRLF */
-            rc = ap_proxygetline(backend->tmp_bb, buffer, sizeof(buffer),
+            rc = ap_proxygetline(backend->tmp_bb, buffer, response_field_size,
                                  backend->r, 0, &len);
         }
         if (len <= 0) {
@@ -1342,7 +1369,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
             /* If not an HTTP/1 message or
              * if the status line was > 8192 bytes
              */
-            if ((major != 1) || (len >= sizeof(buffer)-1)) {
+            if ((major != 1) || (len >= response_field_size - 1)) {
                 return ap_proxyerror(r, HTTP_BAD_GATEWAY,
                 apr_pstrcat(p, "Corrupt status line returned by remote "
                             "server: ", buffer, NULL));
@@ -1385,7 +1412,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
                          "Set-Cookie", NULL);
 
             /* shove the headers direct into r->headers_out */
-            ap_proxy_read_headers(r, backend->r, buffer, sizeof(buffer), origin,
+            ap_proxy_read_headers(r, backend->r, buffer, response_field_size, origin,
                                   &pread_len);
 
             if (r->headers_out == NULL) {
index 334757799e5c42c7ee2dbb8b888fc1a071ed883f..708160f30bc709168d1cda1835c810fbf664f40c 100644 (file)
@@ -224,9 +224,12 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
     int do_alloc = (*s == NULL), saw_eos = 0;
     int fold = flags & AP_GETLINE_FOLD;
     int crlf = flags & AP_GETLINE_CRLF;
+    int nospc_eol = flags & AP_GETLINE_NOSPC_EOL;
+    int saw_eol = 0, saw_nospc = 0;
 
     if (!n) {
         /* Needs room for NUL byte at least */
+        *read = 0;
         return APR_BADARG;
     }
 
@@ -238,7 +241,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
     if (last_char)
         *last_char = '\0';
 
-    for (;;) {
+    do {
         apr_brigade_cleanup(bb);
         rv = ap_get_brigade(r->proto_input_filters, bb, AP_MODE_GETLINE,
                             APR_BLOCK_READ, 0);
@@ -282,8 +285,52 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
 
             /* Would this overrun our buffer?  If so, we'll die. */
             if (n < bytes_handled + len) {
-                rv = APR_ENOSPC;
-                goto cleanup;
+                /* Before we die, let's fill the buffer up to its limit (i.e.
+                 * fall through with the remaining length, if any), setting
+                 * saw_eol on LF to stop the outer loop appropriately; we may
+                 * come back here once the buffer is filled (no LF seen), and
+                 * either be done at that time or continue to wait for LF here
+                 * if nospc_eol is set.
+                 *
+                 * But there is also a corner cases which we want to address,
+                 * namely if the buffer is overrun by the final LF only (i.e.
+                 * the CR fits in); this is not really an overrun since we'll
+                 * strip the CR finally (and use it for NUL byte), but anyway
+                 * we have to handle the case so that it's not returned to the
+                 * caller as part of the truncated line (it's not!). This is
+                 * easier to consider that LF is out of counting and thus fall
+                 * through with no error (saw_eol is set to 2 so that we later
+                 * ignore LF handling already done here), while folding and
+                 * nospc_eol logics continue to work (or fail) appropriately.
+                 */
+                saw_eol = (str[len - 1] == APR_ASCII_LF);
+                if (/* First time around */
+                    saw_eol && !saw_nospc
+                    /*  Single LF completing the buffered CR, */
+                    && ((len == 1 && ((*s)[bytes_handled - 1] == APR_ASCII_CR))
+                    /*  or trailing CRLF overuns by LF only */
+                        || (len > 1 && str[len - 2] == APR_ASCII_CR
+                            && n - bytes_handled + 1 == len))) {
+                    /* In both cases *last_char is (to be) the CR stripped by
+                     * later 'bytes_handled = last_char - *s'.
+                     */
+                    saw_eol = 2;
+                }
+                else {
+                    /* In any other case we'd lose data. */
+                    rv = APR_ENOSPC;
+                    saw_nospc = 1;
+                }
+                len = n - bytes_handled;
+                if (!len) {
+                    if (saw_eol) {
+                        break;
+                    }
+                    if (nospc_eol) {
+                        continue;
+                    }
+                    goto cleanup;
+                }
             }
 
             /* Do we have to handle the allocation ourselves? */
@@ -322,18 +369,28 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
 
         /* If we got a full line of input, stop reading */
         if (last_char && (*last_char == APR_ASCII_LF)) {
-            break;
+            saw_eol = 1;
         }
+    } while (!saw_eol);
+
+    if (rv != APR_SUCCESS) {
+        /* End of line after APR_ENOSPC above */
+        goto cleanup;
     }
 
     /* Now terminate the string at the end of the line;
-     * if the last-but-one character is a CR, terminate there */
-    if (last_char > *s && last_char[-1] == APR_ASCII_CR) {
-        last_char--;
-    }
-    else if (crlf) {
-        rv = APR_EINVAL;
-        goto cleanup;
+     * if the last-but-one character is a CR, terminate there.
+     * LF is handled above (not accounted) when saw_eol == 2,
+     * the last char is CR to terminate at still.
+     */
+    if (saw_eol < 2) {
+        if (last_char > *s && last_char[-1] == APR_ASCII_CR) {
+            last_char--;
+        }
+        else if (crlf) {
+            rv = APR_EINVAL;
+            goto cleanup;
+        }
     }
     bytes_handled = last_char - *s;
 
@@ -407,8 +464,8 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
 
                     next_size = n - bytes_handled;
 
-                    rv = ap_rgetline_core(&tmp, next_size,
-                                          &next_len, r, 0, bb);
+                    rv = ap_rgetline_core(&tmp, next_size, &next_len, r,
+                                          flags & ~AP_GETLINE_FOLD, bb);
                     if (rv != APR_SUCCESS) {
                         goto cleanup;
                     }
@@ -442,22 +499,22 @@ cleanup:
     if (bytes_handled >= n) {
         bytes_handled = n - 1;
     }
+
+    *read = bytes_handled;
     if (*s) {
         /* ensure the string is NUL terminated */
-        (*s)[bytes_handled] = '\0';
-    }
-    *read = bytes_handled;
-
-    if (rv != APR_SUCCESS) {
-        return rv;
-    }
+        (*s)[*read] = '\0';
 
-    /* PR#43039: We shouldn't accept NULL bytes within the line */
-    if (strlen(*s) < bytes_handled) {
-        return APR_EINVAL;
+        /* PR#43039: We shouldn't accept NULL bytes within the line */
+        bytes_handled = strlen(*s);
+        if (bytes_handled < *read) {
+            *read = bytes_handled;
+            if (rv == APR_SUCCESS) {
+                rv = APR_EINVAL;
+            }
+        }
     }
-
-    return APR_SUCCESS;
+    return rv;
 }
 
 #if APR_CHARSET_EBCDIC