Add the input filter if we have read the body (even if a sub-request). See #335.

CHANGES

@@ -21,7 +21,7 @@
    a level 1-3 message to the debug log are now marked as 'relevant' and may
    generate an audit log entry.
 
- * Do not process subrequests in phase 2-4.
+ * Do not process subrequests in phase 2-4, but do hand off the request data.
 
  * Fixed deprecatevar:var=N/S action so that it decrements N every S seconds
    as documented instead of decrementing by a rate.

apache2/apache2_io.c

@@ -41,6 +41,11 @@ apr_status_t input_filter(ap_filter_t *f, apr_bucket_brigade *bb_out,
 
     msr->r = f->r;
 
+    if (msr->phase > PHASE_REQUEST_BODY) {
+            msr_log(msr, 1, "Internal error: Still in input filter in phase %d", msr->phase);
+            return APR_EGENERAL;
+    }
+
     if ((msr->if_status == IF_STATUS_COMPLETE)||(msr->if_status == IF_STATUS_NONE)) {
         if (msr->txcfg->debuglog_level >= 4) {
             msr_log(msr, 4, "Input filter: Input forwarding already complete, skipping (f %x, r %x).", f, f->r);

apache2/mod_security2.c

@@ -888,6 +888,19 @@ static int hook_log_transaction(request_rec *r) {
 static void hook_insert_filter(request_rec *r) {
     modsec_rec *msr = NULL;
 
+    /* Find the transaction context first. */
+    msr = retrieve_tx_context(r);
+    if (msr == NULL) return;
+
+    /* Add the input filter, but only if we need it to run. */
+    if (msr->if_status == IF_STATUS_WANTS_TO_RUN) {
+        if (msr->txcfg->debuglog_level >= 4) {
+            msr_log(msr, 4, "Hook insert_filter: Adding input forwarding filter %s(r %x).", (((r->main != NULL)||(r->prev != NULL)) ? "for subrequest " : ""), r);
+        }
+
+        ap_add_input_filter("MODSECURITY_IN", msr, r, r->connection);
+    }
+
     /* This function needs to run only once per transaction
      * (i.e. subrequests and redirects are excluded).
      */
@@ -895,10 +908,6 @@ static void hook_insert_filter(request_rec *r) {
         return;
     }
 
-    /* Find the transaction context first. */
-    msr = retrieve_tx_context(r);
-    if (msr == NULL) return;
-
     /* We always add the PDF XSS protection filter. */
     if (msr->txcfg->debuglog_level >= 4) {
         msr_log(msr, 4, "Hook insert_filter: Adding PDF XSS protection output filter (r %x).", r);
@@ -907,6 +916,7 @@ static void hook_insert_filter(request_rec *r) {
     ap_add_output_filter("PDFP_OUT", msr, r, r->connection);
 
     /* Only proceed to add the second filter if the engine is enabled. */
+    // TODO: Do we need this anymore?
     if (msr->txcfg->is_enabled == 0) {
         if (msr->txcfg->debuglog_level >= 4) {
             msr_log(msr, 4, "Hook insert_filter: Processing disabled, skipping.");
@@ -915,15 +925,6 @@ static void hook_insert_filter(request_rec *r) {
         return;
     }
 
-    /* Add the input filter, but only if we need it to run. */
-    if (msr->if_status == IF_STATUS_WANTS_TO_RUN) {
-        if (msr->txcfg->debuglog_level >= 4) {
-            msr_log(msr, 4, "Hook insert_filter: Adding input forwarding filter (r %x).", r);
-        }
-
-        ap_add_input_filter("MODSECURITY_IN", msr, r, r->connection);
-    }
-
     /* We always add the output filter because that's where we need to
      * initiate our 3rd and 4th processing phases from. The filter is
      * smart enough not to buffer the data if it is not supposed to.