Merge 2.5.x changes into trunk.

CHANGES

@@ -1,6 +1,8 @@
-22 Apr 2009 - trunk
+20 May 2009 - trunk
 -------------------
 
+ * Correctly resolve chained rule actions in logs.
+
  * Cleanup some code for portability.
 
  * AIX does not support hidden visibility with xlc compiler.

apache2/re.c

@@ -68,9 +68,12 @@ char *msre_actionset_generate_action_string(apr_pool_t *pool, const msre_actions
     const apr_array_header_t *tarr = NULL;
     const apr_table_entry_t *telts = NULL;
     char *actions = NULL;
+    int chain;
     int i;
 
-    if (actionset == NULL) return apr_pstrdup(pool, "");
+    if (actionset == NULL) return NULL;
+
+    chain = ((actionset->rule != NOT_SET_P) && actionset->rule->chain_starter) ? 1 : 0;
 
     tarr = apr_table_elts(actionset->actions);
     telts = (const apr_table_entry_t*)tarr->elts;
@@ -79,6 +82,22 @@ char *msre_actionset_generate_action_string(apr_pool_t *pool, const msre_actions
         msre_action *action = (msre_action *)telts[i].val;
         int use_quotes = 0;
 
+        if (chain) {
+            /* Skip some actions that are not used in a chain. */
+            if (   (action->metadata->type == ACTION_DISRUPTIVE)
+                || (action->metadata->type == ACTION_METADATA)
+                || (strcmp("log", action->metadata->name) == 0)
+                || (strcmp("auditlog", action->metadata->name) == 0)
+                || (strcmp("nolog", action->metadata->name) == 0)
+                || (strcmp("noauditlog", action->metadata->name) == 0)
+                || (strcmp("severity", action->metadata->name) == 0)
+                || (strcmp("tag", action->metadata->name) == 0)
+                || (strcmp("phase", action->metadata->name) == 0)) 
+            {
+                continue;
+            }
+        }
+
         /* Check if we need any quotes */
         if (action->param != NULL) {
             int j;
@@ -102,7 +121,7 @@ char *msre_actionset_generate_action_string(apr_pool_t *pool, const msre_actions
             NULL);
     }
 
-    return (actions == NULL) ? apr_pstrdup(pool, "") : actions;
+    return actions;
 }
 
 /**

apache2/re_actions.c

@@ -2120,6 +2120,7 @@ void msre_engine_register_default_actions(msre_engine *engine) {
     );
 
     /* phase */
+    /* ENH: This should be ACTION_NON_DISRUPTIVE or ACTION_FLOW??? */
     msre_engine_action_register(engine,
         "phase",
         ACTION_DISRUPTIVE,
@@ -2354,6 +2355,7 @@ void msre_engine_register_default_actions(msre_engine *engine) {
     );
 
     /* tag */
+    /* ENH: This should be ACTION_METADATA??? */
     msre_engine_action_register(engine,
         "tag",
         ACTION_NON_DISRUPTIVE,

apache2/t/regression/rule/00-basics.t

@@ -22,3 +22,70 @@
 		GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
 	),
 },
+
+# SecRule
+{
+	type => "rule",
+	comment => "SecRule (no action)",
+	conf => qq(
+		SecRuleEngine On
+		SecDebugLog $ENV{DEBUG_LOG}
+		SecDebugLogLevel 5
+        SecDefaultAction "phase:2,deny,status:403"
+        SecRule ARGS:test "value"
+	),
+	match_log => {
+		error => [ qr/ModSecurity: /, 1 ],
+		debug => [ qr/Rule [0-9a-f]+: SecRule "ARGS:test" "\@rx value" "phase:2,deny,status:403"$/m, 1 ],
+	},
+	match_response => {
+		status => qr/^403$/,
+	},
+	request => new HTTP::Request(
+		GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt?test=value",
+	),
+},
+{
+	type => "rule",
+	comment => "SecRule (action)",
+	conf => qq(
+		SecRuleEngine On
+		SecDebugLog $ENV{DEBUG_LOG}
+		SecDebugLogLevel 5
+        SecDefaultAction "phase:2,pass"
+        SecRule ARGS:test "value" "deny,status:403"
+	),
+	match_log => {
+		error => [ qr/ModSecurity: /, 1 ],
+		debug => [ qr/Rule [0-9a-f]+: SecRule "ARGS:test" "\@rx value" "phase:2,deny,status:403"$/m, 1 ],
+	},
+	match_response => {
+		status => qr/^403$/,
+	},
+	request => new HTTP::Request(
+		GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt?test=value",
+	),
+},
+{
+	type => "rule",
+	comment => "SecRule (chain)",
+	conf => qq(
+		SecRuleEngine On
+		SecDebugLog $ENV{DEBUG_LOG}
+		SecDebugLogLevel 5
+        SecDefaultAction "phase:2,log,noauditlog,pass,tag:foo"
+        SecRule ARGS:test "value" "chain,phase:2,deny,status:403"
+        SecRule &ARGS "\@eq 1" "chain,setenv:tx.foo=bar"
+        SecRule REQUEST_METHOD "\@streq GET"
+	),
+	match_log => {
+		error => [ qr/ModSecurity: /, 1 ],
+		debug => [ qr/Rule [0-9a-f]+: SecRule "ARGS:test" "\@rx value" "phase:2,log,noauditlog,tag:foo,chain,deny,status:403"\r?\n.*Rule [0-9a-f]+: SecRule "&ARGS" "\@eq 1" "chain,setenv:tx.foo=bar"\r?\n.*Rule [0-9a-f]+: SecRule "REQUEST_METHOD" "\@streq GET"\r?\n/s, 1 ],
+	},
+	match_response => {
+		status => qr/^403$/,
+	},
+	request => new HTTP::Request(
+		GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt?test=value",
+	),
+},