Mailing-List: contact cvs-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1199060 -
 /httpd/httpd/trunk/modules/filters/mod_substitute.c
Date: Tue, 08 Nov 2011 02:10:58 -0000
To: cvs@httpd.apache.org
From: sf@apache.org
Message-Id: <20111108021058.25E9923889DA@eris.apache.org>

Author: sf
Date: Tue Nov  8 02:10:57 2011
New Revision: 1199060

URL: http://svn.apache.org/viewvc?rev=1199060&view=rev
Log:
Add some comments and another line length check

Modified:
    httpd/httpd/trunk/modules/filters/mod_substitute.c

Modified: httpd/httpd/trunk/modules/filters/mod_substitute.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_substitute.c?rev=1199060&r1=1199059&r2=1199060&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/filters/mod_substitute.c (original)
+++ httpd/httpd/trunk/modules/filters/mod_substitute.c Tue Nov  8 02:10:57 2011
@@ -80,6 +80,10 @@ static void *merge_substitute_dcfg(apr_p
 }
 
 #define AP_MAX_BUCKETS 1000
+/*
+ * We want to limit the memory usage in a way that is predictable. Therefore
+ * we limit the resulting length of the line to this value.
+ */
 #define AP_SUBST_MAX_LINE_LENGTH (128*MAX_STRING_LEN)
 
 #define SEDRMPATBCKT(b, offset, tmp_b, patlen) do {  \
@@ -137,6 +141,10 @@ static apr_status_t do_pattmatch(ap_filt
                 vb.strlen = 0;
                 if (script->pattern) {
                     const char *repl;
+                    /*
+                     * space_left counts how many bytes we have left until the
+                     * line length reaches AP_SUBST_MAX_LINE_LENGTH.
+                     */
                     apr_size_t space_left = AP_SUBST_MAX_LINE_LENGTH;
                     apr_size_t repl_len = strlen(script->replacement);
                     while ((repl = apr_strmatch(script->pattern, buff, bytes)))
@@ -160,13 +168,19 @@ static apr_status_t do_pattmatch(ap_filt
                         }
                         else {
                             /*
-                             * We now split off the stuff before the regex
-                             * as its own bucket, then isolate the pattern
-                             * and delete it.
+                             * The string before the match but after the
+                             * previous match (if any) has length 'len'.
+                             * Check if we still have space for this string and
+                             * the replacement string.
                              */
                             if (space_left < len + repl_len)
                                 return APR_ENOMEM;
                             space_left -= len + repl_len;
+                            /*
+                             * We now split off the string before the match
+                             * as its own bucket, then isolate the matched
+                             * string and delete it.
+                             */
                             SEDRMPATBCKT(b, len, tmp_b, script->patlen);
                             /*
                              * Finally, we create a bucket that contains the
@@ -183,17 +197,31 @@ static apr_status_t do_pattmatch(ap_filt
                         bytes -= len;
                         buff += len;
                     }
-                    if (have_match && script->flatten && !force_quick) {
-                        /* XXX: we should check for AP_MAX_BUCKETS here and
-                         * XXX: call ap_pass_brigade accordingly
-                         */
-                        char *copy = ap_varbuf_pdup(pool, &vb, NULL, 0,
-                                                    buff, bytes, &len);
-                        tmp_b = apr_bucket_pool_create(copy, len, pool,
-                                                       f->r->connection->bucket_alloc);
-                        APR_BUCKET_INSERT_BEFORE(b, tmp_b);
-                        apr_bucket_delete(b);
-                        b = tmp_b;
+                    if (have_match) {
+                        if (script->flatten && !force_quick) {
+                            /* XXX: we should check for AP_MAX_BUCKETS here and
+                             * XXX: call ap_pass_brigade accordingly
+                             */
+                            char *copy = ap_varbuf_pdup(pool, &vb, NULL, 0,
+                                                        buff, bytes, &len);
+                            tmp_b = apr_bucket_pool_create(copy, len, pool,
+                                                           f->r->connection->bucket_alloc);
+                            APR_BUCKET_INSERT_BEFORE(b, tmp_b);
+                            apr_bucket_delete(b);
+                            b = tmp_b;
+                        }
+                        else {
+                            /*
+                             * We want the behaviour to be predictable.
+                             * Therefore we try to always error out if the
+                             * line length is larger than the limit,
+                             * regardless of the content of the line. So,
+                             * let's check if the remaining non-matching
+                             * string does not exceed the limit.
+                             */
+                            if (space_left < b->length)
+                                return APR_ENOMEM;
+                        }
                     }
                 }
                 else if (script->regexp) {