Merge r1619383, r1619444, r1662245, r1662246 from trunk:

A misplaced check for inflation limits prevented limiting relatively
small inputs.  PR56872

Submitted By: Edward Lu
Committed By: covener

mod_deflate: follow up to r1619383.

deflate_in_filter():
- use inflated bytes per inflate() call to compute the total output bytes,
- check zlib errors before limits,
- add missing check_ratio() when asked to flush.

deflate_out_filter():
- check ratio after each inflate() call.

mod_deflate: follow up to r1619383 and r1619444: CHANGES entry.

CHANGES: follow up to r1662245: Add PR number.
Submitted by: covener, ylavic, ylavic, ylavic
Reviewed/backported by: jim

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

diff --git a/CHANGES b/CHANGES
index 12e49354aff6723bf7cc8eec00613d537d75e4dc..360aaad41061aa5eb22b700df4099d4bd3609bfd 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -44,6 +44,9 @@ Changes with Apache 2.4.13
   *) mod_ssl: Fix renegotiation failures redirected to an ErrorDocument.
      PR 57334.  [Yann Ylavic].
 
+  *) mod_deflate: A misplaced check prevents limiting small bodies with the
+     new inflate limits. PR56872. [Edward Lu, Eric Covener, Yann Ylavic]
+
   *) mod_proxy_ajp: Forward SSL protocol name (SSLv3, TLSv1.1 etc.) as a
      request attribute to the backend. Recent Tomcat versions will extract
      it and provide it as a servlet request attribute named

diff --git a/STATUS b/STATUS
index c9873619c3995404d9f4d92d7fb642452e92a83f..778c8b544964e61e0e01af026cb63f8fb9ef6c54 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -106,16 +106,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_deflate: A misplaced check prevents limiting small bodies with the
-     new inflate limits.  PR56872.
-     trunk patch: http://svn.apache.org/r1619383
-                  http://svn.apache.org/r1619444
-                  http://svn.apache.org/r1662245 (CHANGES entry)
-                  http://svn.apache.org/r1662246 (updated CHANGES entry)
-     2.4.x patch: trunk works (modulo CHANGES)
-                  http://people.apache.org/~ylavic/httpd-2.4.x-mod_deflate-PR56872.patch
-     +1: ylavic, covener, jim
-
   *) mod_proxy: use the original (non absolute) form of the request-line's URI
      for requests embedded in CONNECT payloads used to connect SSL backends via
      a ProxyRemote forward-proxy. PR 55892.

diff --git a/modules/filters/mod_deflate.c b/modules/filters/mod_deflate.c
index b892fa7c762fc518e36df9fba65d4d8be24d6544..5e390d78750d4f13d7c3e1517d3aea9a86fc2725 100644 (file)
--- a/modules/filters/mod_deflate.c
+++ b/modules/filters/mod_deflate.c
@@ -1278,6 +1278,15 @@ static apr_status_t deflate_in_filter(ap_filter_t *f,
                     return APR_ENOSPC;
                 }
 
+                if (!check_ratio(r, ctx, dc)) {
+                    inflateEnd(&ctx->stream);
+                    ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO()
+                            "Inflated content ratio is larger than the "
+                            "configured limit %i by %i time(s)",
+                            dc->ratio_limit, dc->ratio_burst);
+                    return APR_EINVAL;
+                }
+
                 ctx->crc = crc32(ctx->crc, (const Bytef *)ctx->buffer, len);
                 tmp_b = apr_bucket_heap_create((char *)ctx->buffer, len,
                                                 NULL, f->c->bucket_alloc);
@@ -1335,26 +1344,6 @@ static apr_status_t deflate_in_filter(ap_filter_t *f,
                         ctx->stream.next_out = ctx->buffer;
                         len = c->bufferSize - ctx->stream.avail_out;
 
-                        ctx->inflate_total += len;
-                        if (inflate_limit && ctx->inflate_total > inflate_limit) { 
-                            inflateEnd(&ctx->stream);
-                            ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02648)
-                                    "Inflated content length of %" APR_OFF_T_FMT
-                                    " is larger than the configured limit"
-                                    " of %" APR_OFF_T_FMT, 
-                                    ctx->inflate_total, inflate_limit);
-                            return APR_ENOSPC;
-                        }
-
-                        if (!check_ratio(r, ctx, dc)) {
-                            inflateEnd(&ctx->stream);
-                            ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02649)
-                                    "Inflated content ratio is larger than the "
-                                    "configured limit %i by %i time(s)",
-                                    dc->ratio_limit, dc->ratio_burst);
-                            return APR_EINVAL;
-                        }
-
                         ctx->crc = crc32(ctx->crc, (const Bytef *)ctx->buffer, len);
                         tmp_heap = apr_bucket_heap_create((char *)ctx->buffer, len,
                                                           NULL, f->c->bucket_alloc);
@@ -1362,22 +1351,43 @@ static apr_status_t deflate_in_filter(ap_filter_t *f,
                         ctx->stream.avail_out = c->bufferSize;
                     }
 
+                    len = ctx->stream.avail_out;
                     zRC = inflate(&ctx->stream, Z_NO_FLUSH);
 
-                    if (zRC == Z_STREAM_END) {
-                        ctx->validation_buffer = apr_pcalloc(r->pool,
-                                                             VALIDATION_SIZE);
-                        ctx->validation_buffer_length = 0;
-                        break;
-                    }
-
-                    if (zRC != Z_OK) {
+                    if (zRC != Z_OK && zRC != Z_STREAM_END) {
                         inflateEnd(&ctx->stream);
                         ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01392)
                                       "Zlib error %d inflating data (%s)", zRC,
                                       ctx->stream.msg);
                         return APR_EGENERAL;
                     }
+
+                    ctx->inflate_total += len - ctx->stream.avail_out;
+                    if (inflate_limit && ctx->inflate_total > inflate_limit) { 
+                        inflateEnd(&ctx->stream);
+                        ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02648)
+                                "Inflated content length of %" APR_OFF_T_FMT
+                                " is larger than the configured limit"
+                                " of %" APR_OFF_T_FMT, 
+                                ctx->inflate_total, inflate_limit);
+                        return APR_ENOSPC;
+                    }
+
+                    if (!check_ratio(r, ctx, dc)) {
+                        inflateEnd(&ctx->stream);
+                        ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02649)
+                                "Inflated content ratio is larger than the "
+                                "configured limit %i by %i time(s)",
+                                dc->ratio_limit, dc->ratio_burst);
+                        return APR_EINVAL;
+                    }
+
+                    if (zRC == Z_STREAM_END) {
+                        ctx->validation_buffer = apr_pcalloc(r->pool,
+                                                             VALIDATION_SIZE);
+                        ctx->validation_buffer_length = 0;
+                        break;
+                    }
                 }
             }
 
@@ -1786,15 +1796,6 @@ static apr_status_t inflate_out_filter(ap_filter_t *f,
 
         while (ctx->stream.avail_in != 0) {
             if (ctx->stream.avail_out == 0) {
-
-                if (!check_ratio(r, ctx, dc)) {
-                    ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02650)
-                            "Inflated content ratio is larger than the "
-                            "configured limit %i by %i time(s)",
-                            dc->ratio_limit, dc->ratio_burst);
-                    return APR_EINVAL;
-                }
-
                 ctx->stream.next_out = ctx->buffer;
                 len = c->bufferSize - ctx->stream.avail_out;
 
@@ -1812,6 +1813,21 @@ static apr_status_t inflate_out_filter(ap_filter_t *f,
 
             zRC = inflate(&ctx->stream, Z_NO_FLUSH);
 
+            if (zRC != Z_OK && zRC != Z_STREAM_END) {
+                ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01409)
+                              "Zlib error %d inflating data (%s)", zRC,
+                              ctx->stream.msg);
+                return APR_EGENERAL;
+            }
+
+            if (!check_ratio(r, ctx, dc)) {
+                ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02650)
+                              "Inflated content ratio is larger than the "
+                              "configured limit %i by %i time(s)",
+                              dc->ratio_limit, dc->ratio_burst);
+                return APR_EINVAL;
+            }
+
             if (zRC == Z_STREAM_END) {
                 /*
                  * We have inflated all data. Now try to capture the
@@ -1826,21 +1842,15 @@ static apr_status_t inflate_out_filter(ap_filter_t *f,
                                   "Zlib: %d bytes of garbage at the end of "
                                   "compressed stream.",
                                   ctx->stream.avail_in - VALIDATION_SIZE);
-                } else if (ctx->stream.avail_in > 0) {
-                           ctx->validation_buffer_length = ctx->stream.avail_in;
+                }
+                else if (ctx->stream.avail_in > 0) {
+                    ctx->validation_buffer_length = ctx->stream.avail_in;
                 }
                 if (ctx->validation_buffer_length)
                     memcpy(ctx->validation_buffer, ctx->stream.next_in,
                            ctx->validation_buffer_length);
                 break;
             }
-
-            if (zRC != Z_OK) {
-                ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01409)
-                              "Zlib error %d inflating data (%s)", zRC,
-                              ctx->stream.msg);
-                return APR_EGENERAL;
-            }
         }
 
         apr_bucket_delete(e);