Preallocate memory when SecStreamInBodyInspection is on. 20x speed improvement for 10mb upload. Also simplified modsecurity_request_body_to_stream.

Author: allanrbo

apache2/apache2_io.c

@@ -281,7 +281,6 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) {
             }
 
             if (msr->txcfg->stream_inbody_inspection == 1)   {
-                msr->stream_input_length+=buflen;
                 modsecurity_request_body_to_stream(msr, buf, buflen, error_msg);
             }
 

apache2/modsecurity.h

@@ -287,6 +287,7 @@ struct modsec_rec {
     unsigned int         resbody_contains_html;
 
     apr_size_t           stream_input_length;
+    apr_size_t           stream_input_allocated_length;
     char                *stream_input_data;
     apr_size_t           stream_output_length;
     char                *stream_output_data;

apache2/msc_reqbody.c

@@ -428,55 +428,62 @@ apr_status_t modsecurity_request_body_store(modsec_rec *msr,
 }
 
 apr_status_t modsecurity_request_body_to_stream(modsec_rec *msr, const char *buffer, int buflen, char **error_msg) {
-    char *stream_input_body = NULL;
-    char *data = NULL;
-    int first_pkt = 0;
+    apr_size_t allocate;
+    char* allocated;
 
-    if(msr->stream_input_data == NULL)  {
-        msr->stream_input_data = (char *)calloc(sizeof(char), msr->stream_input_length + 1);
-        first_pkt = 1;
-    }
-    else    {
-
-        data = (char *)malloc(msr->stream_input_length + 1 - buflen);
+    if (msr->stream_input_data == NULL)  {
+        // Is the request body length is known beforehand? (requests that are not Transfer-Encoding: chunked)
+        if (msr->request_content_length > 0) {
+            allocate = msr->request_content_length;
+        }
+        else {
+            // We don't know how this request is going to be, so hope for just buflen to begin with (requests that are Transfer-Encoding: chunked)
+            allocate = buflen;
+        }
 
-        if(data == NULL)
+        allocated = (char*) calloc(allocate, sizeof(char));
+        if (allocated) {
+            msr->stream_input_data = allocated;
+            msr->stream_input_allocated_length = allocate;
+        }
+        else {
+            *error_msg = apr_psprintf(
+                msr->mp,
+                "Unable to allocate memory to hold request body on stream. Asked for %" APR_SIZE_T_FMT " bytes.",
+                allocate);
             return -1;
-
-        memset(data, 0, msr->stream_input_length + 1 - buflen);
-        memcpy(data, msr->stream_input_data, msr->stream_input_length - buflen);
-
-        stream_input_body = (char *)realloc(msr->stream_input_data, msr->stream_input_length + 1);
-
-        msr->stream_input_data = (char *)stream_input_body;
-    }
-
-    if (msr->stream_input_data == NULL) {
-        if(data)    {
-            free(data);
-            data = NULL;
         }
-        *error_msg = apr_psprintf(msr->mp, "Unable to allocate memory to hold request body on stream. Asked for %" APR_SIZE_T_FMT " bytes.",
-                msr->stream_input_length + 1);
-        return -1;
     }
+    else {
+        // Do we need to expand the space we have previously allocated?
+        if ((msr->stream_input_length + buflen) > msr->stream_input_allocated_length) {
 
-    memset(msr->stream_input_data, 0, msr->stream_input_length+1);
+            // If this becomes a hotspot again, consider increasing by some percent extra each time, for fewer reallocs
+            allocate = msr->stream_input_length + buflen;
 
-    if(first_pkt)   {
-        memcpy(msr->stream_input_data, buffer, msr->stream_input_length);
-    } else {
-        memcpy(msr->stream_input_data, data, msr->stream_input_length - buflen);
-        memcpy(msr->stream_input_data+(msr->stream_input_length - buflen), buffer, buflen);
+            allocated = (char*) realloc(msr->stream_input_data, allocate);
+            if (allocated) {
+                msr->stream_input_data = allocated;
+                msr->stream_input_allocated_length = allocate;
+            }
+            else {
+                *error_msg = apr_psprintf(
+                    msr->mp,
+                    "Unable to reallocate memory to hold request body on stream. Asked for %" APR_SIZE_T_FMT " bytes.",
+                    allocate);
+                free(msr->stream_input_data);
+                return -1;
+            }
+        }
     }
 
-    if(data)    {
-        free(data);
-        data = NULL;
-    }
+    // Append buffer to msr->stream_input_data
+    memcpy(msr->stream_input_data + msr->stream_input_length, buffer, buflen);
+    msr->stream_input_length += buflen;
 
     return 1;
 }
+
 /**
  * Replace a bunch of chunks holding a request body with a single large chunk.
  */
@@ -884,15 +891,15 @@ apr_status_t modsecurity_request_body_clear(modsec_rec *msr, char **error_msg) {
 
         if (msr->msc_reqbody_filename != NULL) {
             if (keep_body) {
-                /* Move request body (which is a file) to the storage area. */
-                const char *put_filename = NULL;
-                const char *put_basename = NULL;
-
                 if (strcmp(msr->txcfg->upload_dir, msr->txcfg->tmp_dir) == 0) {
                     msr_log(msr, 4, "Not moving file to identical location.");
                     goto nullify;
                 }
 
+                /* Move request body (which is a file) to the storage area. */
+                const char *put_filename = NULL;
+                const char *put_basename = NULL;
+
                 /* Construct the new filename. */
                 put_basename = file_basename(msr->msc_reqbody_mp, msr->msc_reqbody_filename);
                 if (put_basename == NULL) {

apache2/re_operators.c

@@ -634,6 +634,7 @@ static int msre_op_rsub_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,
         free(msr->stream_input_data);
         msr->stream_input_data = NULL;
         msr->stream_input_length = 0;
+        msr->stream_input_allocated_length  = 0;
 
         msr->stream_input_data = (char *)malloc(size+1);
 
@@ -642,7 +643,8 @@ static int msre_op_rsub_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,
         }
 
         msr->stream_input_length = size;
-        memset(msr->stream_input_data, 0x0, size+1);
+        msr->stream_input_allocated_length = size + 1;
+        memset(msr->stream_input_data, 0x0, size + 1);
 
         msr->if_stream_changed = 1;
 

iis/dependencies/build_yajl.bat

@@ -28,7 +28,7 @@ copy /y "%WORK_DIR%\yajl\build\%YAJL_DIR%\lib\yajl_s.lib" "%OUTPUT_DIR%"
 @exit /B 0
 
 :file_not_found_bin
-@echo File not found: "%SOURCE_DIR%\%PCRE%"
+@echo File not found: "%SOURCE_DIR%\%YAJL%"
 @goto failed
 
 :build_failed