Improvements in transformation cache (add options, document).

Update CHANGES.

CHANGES

@@ -3,7 +3,7 @@
 
  * Cleaned up some documentation.
 
- * Performance improvements in caching transformations.
+ * Performance improvements and greater control over caching transformations.
 
  * Stricter validation for @validateUtf8Encoding.
 
@@ -17,6 +17,8 @@
 27 July 2007 - 2.1.2
 --------------------
 
+ * Cleaned up and clarified some documentation.
+
  * Update included core rules to latest version (1.4.3).
 
  * Enhanced ability to alert/audit failed requests.

apache2/apache2_config.c

@@ -95,6 +95,8 @@ void *create_directory_config(apr_pool_t *mp, char *path) {
 
     /* Cache */
     dcfg->cache_trans = NOT_SET;
+    dcfg->cache_trans_min = NOT_SET;
+    dcfg->cache_trans_max = NOT_SET;
 
     return dcfg;
 }
@@ -396,6 +398,10 @@ void *merge_directory_configs(apr_pool_t *mp, void *_parent, void *_child) {
     /* Cache */
     merged->cache_trans = (child->cache_trans == NOT_SET
         ? parent->cache_trans : child->cache_trans);
+    merged->cache_trans_min = (child->cache_trans_min == (apr_size_t)NOT_SET
+        ? parent->cache_trans_min : child->cache_trans_min);
+    merged->cache_trans_max = (child->cache_trans_max == (apr_size_t)NOT_SET
+        ? parent->cache_trans_max : child->cache_trans_max);
 
     return merged;
 }
@@ -471,7 +477,9 @@ void init_directory_config(directory_config *dcfg) {
     if (dcfg->geo == NOT_SET_P) dcfg->geo = NULL;
 
     /* Cache */
-    if (dcfg->cache_trans == NOT_SET_P) dcfg->cache_trans = MODSEC_CACHE_ENABLED;
+    if (dcfg->cache_trans == NOT_SET) dcfg->cache_trans = MODSEC_CACHE_ENABLED;
+    if (dcfg->cache_trans_min == (apr_size_t)NOT_SET) dcfg->cache_trans_min = 15;
+    if (dcfg->cache_trans_max == (apr_size_t)NOT_SET) dcfg->cache_trans_max = 0;
 }
 
 /**
@@ -1247,9 +1255,8 @@ static const char *cmd_geo_lookups_db(cmd_parms *cmd, void *_dcfg,
 
 static const char *cmd_cache_transformations(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) {
     directory_config *dcfg = (directory_config *)_dcfg;
-    if (dcfg == NULL) return NULL;
 
-    // TODO: p2 is options
+    if (dcfg == NULL) return NULL;
 
     if (strcasecmp(p1, "on") == 0)
         dcfg->cache_trans = MODSEC_CACHE_ENABLED;
@@ -1258,6 +1265,53 @@ static const char *cmd_cache_transformations(cmd_parms *cmd, void *_dcfg, const
     else
         return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecCacheTransformations: %s", p1);
 
+    /* Process options */
+    if (p2 != NULL) {
+        apr_table_t *vartable = apr_table_make(cmd->pool, 10);
+        apr_status_t rc;
+        char *error_msg = NULL;
+        const char *charval = NULL;
+        apr_int64_t intval = 0;
+
+        if (vartable == NULL) {
+            return apr_psprintf(cmd->pool, "ModSecurity: Unable to process options for SecCacheTransformations");
+        }
+        rc = msre_parse_generic(cmd->pool, p2, vartable, &error_msg);
+        if (rc < 0) {
+            return apr_psprintf(cmd->pool, "ModSecurity: Unable to parse options for SecCacheTransformations: %s", error_msg);
+        }
+
+        /* minval */
+        charval = apr_table_get(vartable, "minlen");
+        if (charval != NULL) {
+            intval = apr_atoi64(charval);
+            if (intval < 0) {
+                return apr_psprintf(cmd->pool, "ModSecurity: SecCacheTransformations minlen must be positive: %s", charval);
+            }
+            if (intval >= (apr_size_t)NOT_SET) {
+                return apr_psprintf(cmd->pool, "ModSecurity: SecCacheTransformations minlen must be less than: %u", (apr_size_t)NOT_SET);
+            }
+            dcfg->cache_trans_min = (apr_size_t)intval;
+        }
+
+        /* maxval */
+        charval = apr_table_get(vartable, "maxlen");
+        if (charval != NULL) {
+            intval = apr_atoi64(charval);
+            if (intval < 0) {
+                return apr_psprintf(cmd->pool, "ModSecurity: SecCacheTransformations maxlen must be positive: %s", charval);
+            }
+            if (intval >= (apr_size_t)NOT_SET) {
+                return apr_psprintf(cmd->pool, "ModSecurity: SecCacheTransformations maxlen must be less than: %u", (apr_size_t)NOT_SET);
+            }
+            if ((intval != 0) && (intval < dcfg->cache_trans_min)) {
+                return apr_psprintf(cmd->pool, "ModSecurity: SecCacheTransformations maxlen must not be less than minlen: %u < %u", (apr_size_t)intval, dcfg->cache_trans_min);
+            }
+            dcfg->cache_trans_max = (apr_size_t)intval;
+
+        }
+    }
+
     return NULL;
 }
 

apache2/modsecurity.h

@@ -420,6 +420,8 @@ struct directory_config {
 
     /* Cache */
     int                  cache_trans;
+    apr_size_t           cache_trans_min;
+    apr_size_t           cache_trans_max;
 };
 
 struct error_message {

apache2/re.c

@@ -1363,30 +1363,45 @@ apr_status_t msre_rule_process(msre_rule *rule, modsec_rec *msr) {
 
         /* Is this var cacheable? */
         if (msr->txcfg->cache_trans != MODSEC_CACHE_DISABLED) {
-            msr_log(msr, 9, "CACHE: Enabled");
+            usecache = 1;
-            if (var->metadata->is_cacheable == VAR_CACHE) {
-                usecache = 1;
 
-                /* Fetch cache table for this target */
+            /* check the cache options */
-                carr = (apr_table_t **)apr_hash_get(msr->tcache, var->name, APR_HASH_KEY_STRING);
+            if (var->value_len < msr->txcfg->cache_trans_min) {
-                if (carr != NULL) {
+                msr_log(msr, 9, "CACHE: Disabled - %s value length=%d, smaller than minlen=%d", var->name, var->value_len, msr->txcfg->cache_trans_min);
-                    cachetab = carr[msr->phase];
+                usecache = 0;
+            }
+            if ((msr->txcfg->cache_trans_max != 0) && (var->value_len > msr->txcfg->cache_trans_max)) {
+                msr_log(msr, 9, "CACHE: Disabled - %s value length=%d, larger than maxlen=%d", var->name, var->value_len, msr->txcfg->cache_trans_max);
+                usecache = 0;
+            }
+
+            /* if cache is still enabled, check the VAR for cacheablity */
+            if (usecache) {
+                if (var->metadata->is_cacheable == VAR_CACHE) {
+                    msr_log(msr, 9, "CACHE: Enabled");
+
+                    /* Fetch cache table for this target */
+                    carr = (apr_table_t **)apr_hash_get(msr->tcache, var->name, APR_HASH_KEY_STRING);
+                    if (carr != NULL) {
+                        cachetab = carr[msr->phase];
+                    }
+                    else {
+                        /* Create an array of cache tables (one table per phase) */
+                        carr = (apr_table_t **)apr_pcalloc(msr->mp, (sizeof(apr_table_t *) * (PHASE_LAST + 1)));
+                        if (carr == NULL) return -1;
+                        memset(carr, 0, (sizeof(apr_table_t *) * (PHASE_LAST + 1)));
+                        apr_hash_set(msr->tcache, var->name, APR_HASH_KEY_STRING, carr);
+                    }
+
+                    /* Create an empty cache table if this is the first time */
+                    if (cachetab == NULL) {
+                        cachetab = carr[msr->phase] = apr_table_make(msr->mp, 5);
+                    }
                 }
                 else {
-                    /* Create an array of cache tables (one table per phase) */
+                    usecache = 0;
-                    carr = (apr_table_t **)apr_pcalloc(msr->mp, (sizeof(apr_table_t *) * (PHASE_LAST + 1)));
+                    msr_log(msr, 9, "CACHE: %s transformations are not cacheable", var->name);
-                    if (carr == NULL) return -1;
-                    memset(carr, 0, (sizeof(apr_table_t *) * (PHASE_LAST + 1)));
-                    apr_hash_set(msr->tcache, var->name, APR_HASH_KEY_STRING, carr);
                 }
-
-                /* Create an empty cache table if this is the first time */
-                if (cachetab == NULL) {
-                    cachetab = carr[msr->phase] = apr_table_make(msr->mp, 5);
-                }
-            }
-            else {
-                msr_log(msr, 9, "CACHE: %s transformations are not cacheable", var->name);
             }
         }
         else {
@@ -1579,9 +1594,9 @@ apr_status_t msre_rule_process(msre_rule *rule, modsec_rec *msr) {
                     crec->changed = changed;
                     crec->num = k + 1;
                     crec->path = tfnspath;
-                    crec->val = changed ? apr_pmemdup(msr->mp, rval, rval_length) : NULL;
+                    crec->val = changed ? apr_pmemdup(msr->mp, var->value, var->value_len) : NULL;
-                    crec->val_len = changed ? rval_length : -1;
+                    crec->val_len = changed ? var->value_len : 0;
-                    msr_log(msr, 9, "CACHE: Caching %s=\"%.*s\"", tfnskey, crec->val_len, crec->val);
+                    msr_log(msr, 9, "CACHE: Caching %s=\"%.*s\"", tfnskey, var->value_len, var->value);
                     apr_table_setn(cachetab, tfnskey, (void *)crec);
                 }
 
@@ -1619,8 +1634,6 @@ apr_status_t msre_rule_process(msre_rule *rule, modsec_rec *msr) {
         }
     }
 
-
-    msr_log(msr, 9, "CACHE: size=%u", apr_hash_count(msr->tcache));
     #ifdef CACHE_DEBUG
     if (msr->txcfg->debuglog_level >= 9) {
         apr_hash_index_t *hi;
@@ -1629,11 +1642,10 @@ apr_status_t msre_rule_process(msre_rule *rule, modsec_rec *msr) {
         const apr_array_header_t *ctarr;
         const apr_table_entry_t *ctelts;
         msre_cache_rec *rec;
-        int hn = 0;
+        int cn = 0;
         int ti, ri;
 
         for (hi = apr_hash_first(msr->mp, msr->tcache); hi; hi = apr_hash_next(hi)) {
-            hn++;
             apr_hash_this(hi, NULL, NULL, &dummy);
             tab = (apr_table_t **)dummy;
             if (tab == NULL) continue;
@@ -1643,12 +1655,13 @@ apr_status_t msre_rule_process(msre_rule *rule, modsec_rec *msr) {
                 ctarr = apr_table_elts(tab[ti]);
                 ctelts = (const apr_table_entry_t*)ctarr->elts;
                 for (ri = 0; ri < ctarr->nelts; ri++) {
+                    cn++;
                     rec = (msre_cache_rec *)ctelts[ri].val;
                     if (rec->changed) {
-                        msr_log(msr, 9, "CACHE: %5d) phase=%d hits=%d %x;%s=\"%s\"", hn, msr->phase, rec->hits, rec->num, rec->path, log_escape_nq_ex(mptmp, rec->val, rec->val_len));
+                        msr_log(msr, 9, "CACHE: %5d) phase=%d hits=%d %x;%s=\"%s\"", cn, msr->phase, rec->hits, rec->num, rec->path, log_escape_nq_ex(mptmp, rec->val, rec->val_len));
                     }
                     else {
-                        msr_log(msr, 9, "CACHE: %5d) phase=%d hits=%d %x;%s=<no change>", hn, msr->phase, rec->hits, rec->num, rec->path);
+                        msr_log(msr, 9, "CACHE: %5d) phase=%d hits=%d %x;%s=<no change>", cn, msr->phase, rec->hits, rec->num, rec->path);
                     }
                 }
             }

doc/modsecurity2-apache-reference.xml

@@ -783,6 +783,59 @@ SecAuditLogStorageDir logs/audit
       </orderedlist>
     </section>
 
+    <section>
+      <title><literal>SecCacheTransformations</literal></title>
+
+      <para><emphasis role="bold">Description:</emphasis> Controls caching of
+      transformations.</para>
+
+      <para><emphasis role="bold">Syntax:</emphasis> <literal
+      moreinfo="none">SecCacheTransformations On|Off
+      [options]</literal></para>
+
+      <para><emphasis role="bold">Example Usage:</emphasis> <literal
+      moreinfo="none">SecCacheTransformations On
+      "minlen:64,maxlen:0"</literal></para>
+
+      <para><emphasis role="bold">Processing Phase:</emphasis> N/A</para>
+
+      <para><emphasis role="bold">Scope:</emphasis> Any</para>
+
+      <para><emphasis role="bold">Dependencies/Notes:</emphasis> N/A</para>
+
+      <para>First parameter:</para>
+
+      <itemizedlist>
+        <listitem>
+          <para><literal moreinfo="none">On</literal> - cache transformations
+          (per transaction, per phase) allowing identical transformations to
+          be performed only once. (default)</para>
+        </listitem>
+
+        <listitem>
+          <para><literal moreinfo="none">Off</literal> - do not cache any
+          transformations, forcing all transformations to be performed for
+          each rule executed.</para>
+        </listitem>
+      </itemizedlist>
+
+      <para>The following options are allowed (comma seperated):</para>
+
+      <itemizedlist>
+        <listitem>
+          <para><literal moreinfo="none">minlen:N</literal> - do not cache the
+          transformation if the value's length is less than N bytes. (default:
+          15)</para>
+        </listitem>
+
+        <listitem>
+          <para><literal moreinfo="none">maxlen:N</literal> - do not cache the
+          transformation if the value's length is more than N bytes. A zero
+          value is interpreted as "unlimited". (default: 0)</para>
+        </listitem>
+      </itemizedlist>
+    </section>
+
     <section>
       <title><literal>SecChrootDir</literal></title>
 
@@ -4666,4 +4719,4 @@ SecAction "pass,setvar:'tx.allowed_methods=get,post,head'"
 SecRule REQUEST_METHOD "!<emphasis role="bold">@within %{tx.allowed_methods}</emphasis>" t:lowercase,deny,status:403</programlisting>
     </section>
   </section>
-</article>
+</article>