eliminate dedicated nhttp chunk buffer; minor nhttp cleanup in StreamSplitter

diff --git a/ChangeLog b/ChangeLog
index 0f993af5852aa64736c86c5c66041bb05e391f55..4b8677086486ae697a0b9ad215ce316ffbb69432 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+Pending - build 143
+
+-- eliminate dedicated nhttp chunk buffer
+-- minor nhttp cleanup in StreamSplitter
+
 15/03/18 - build 142
 
 -- fixed host lookup issue

diff --git a/src/service_inspectors/nhttp_inspect/nhttp_flow_data.cc b/src/service_inspectors/nhttp_inspect/nhttp_flow_data.cc
index f64c5bcf829bece359985f6df39109bc5d66b42e..1b11e17b35625e67c983d161d1089b5b03ef1fa7 100644 (file)
--- a/src/service_inspectors/nhttp_inspect/nhttp_flow_data.cc
+++ b/src/service_inspectors/nhttp_inspect/nhttp_flow_data.cc
@@ -50,7 +50,6 @@ NHttpFlowData::~NHttpFlowData()
         {
             delete[] section_buffer[k];
         }
-        delete[] chunk_buffer[k];
         delete transaction[k];
         delete splitter[k];
     }
@@ -90,8 +89,6 @@ void NHttpFlowData::show(FILE* out_file) const
         header_octets_visible[1]);
     fprintf(out_file, "Section buffer length: %d/%d\n", section_buffer_length[0],
         section_buffer_length[1]);
-    fprintf(out_file, "Chunk buffer length: %d/%d\n", chunk_buffer_length[0],
-        chunk_buffer_length[1]);
     fprintf(out_file, "Pipelining: front %d back %d overflow %d underflow %d\n", pipeline_front,
         pipeline_back,
         pipeline_overflow, pipeline_underflow);

diff --git a/src/service_inspectors/nhttp_inspect/nhttp_flow_data.h b/src/service_inspectors/nhttp_inspect/nhttp_flow_data.h
index 2da65fd37beaa4b258eff88279939984a6de9e5e..c76a7feb271ff56e2338ba6dad1955c55bcf3e92 100644 (file)
--- a/src/service_inspectors/nhttp_inspect/nhttp_flow_data.h
+++ b/src/service_inspectors/nhttp_inspect/nhttp_flow_data.h
@@ -60,12 +60,6 @@ private:
     uint8_t* section_buffer[2] = { nullptr, nullptr };
     int32_t section_buffer_length[2] = { 0, 0 };
     bool section_buffer_owned[2] = { true, true };
-    uint8_t* chunk_buffer[2] = { nullptr, nullptr };
-    int32_t chunk_buffer_length[2] = { 0, 0 };
-
-    // never actually false and not checked, just here to simplify code
-    bool chunk_buffer_owned[2] = { true, true };
-
     bool zero_chunk[2] = { false, false };
     NHttpInfractions chunk_infractions[2];
 

diff --git a/src/service_inspectors/nhttp_inspect/nhttp_msg_body.h b/src/service_inspectors/nhttp_inspect/nhttp_msg_body.h
index d689aabda1d2f5ab73d4677a9d392f0f197a26b2..a302063ebb00e81e93de65d326474858c3464351 100644 (file)
--- a/src/service_inspectors/nhttp_inspect/nhttp_msg_body.h
+++ b/src/service_inspectors/nhttp_inspect/nhttp_msg_body.h
@@ -39,7 +39,7 @@ public:
     void legacy_clients() override;
 
 protected:
-    int64_t data_length;
+    int64_t data_length; // FIXIT-M this has no meaning in chunk subclass. Potential source of errors.
     int64_t body_octets;
 
     Field data;

diff --git a/src/service_inspectors/nhttp_inspect/nhttp_normalizers.cc b/src/service_inspectors/nhttp_inspect/nhttp_normalizers.cc
index 753bdd502d41872e863446de9d0f4c49127d2d1d..3bfe3ece66885305e528c67567582f6883e6aba9 100644 (file)
--- a/src/service_inspectors/nhttp_inspect/nhttp_normalizers.cc
+++ b/src/service_inspectors/nhttp_inspect/nhttp_normalizers.cc
@@ -27,8 +27,8 @@
 using namespace NHttpEnums;
 
 // Collection of stock normalization functions. This will probably grow throughout the life of the
-// software. New functions must follow the standard signature.
-// The void* at the end is for any special configuration data the function requires.
+// software. New functions must follow the standard signature. The void* at the end is for any
+// special configuration data the function requires.
 
 int32_t norm_decimal_integer(const uint8_t* in_buf, int32_t in_length, uint8_t* out_buf,
     NHttpInfractions& infractions, const void*)

diff --git a/src/service_inspectors/nhttp_inspect/nhttp_scratch_pad.h b/src/service_inspectors/nhttp_inspect/nhttp_scratch_pad.h
index d552c192d4712815fbf8d32b480641322f187f1b..bce63b5a19d96219c627e0a93dd7f7863871d897 100644 (file)
--- a/src/service_inspectors/nhttp_inspect/nhttp_scratch_pad.h
+++ b/src/service_inspectors/nhttp_inspect/nhttp_scratch_pad.h
@@ -22,7 +22,7 @@
 
 //-------------------------------------------------------------------------
 // ScratchPad class
-// Memory management for NHttpMsgHeader class
+// Storage management class
 //-------------------------------------------------------------------------
 
 // Working space and storage for all the derived fields

diff --git a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc
index bcab3832563b9d4c15c36d57978331cdcd71a392..46fd47ee1d76cb2138cdc613638cb71b095a2dbf 100644 (file)
--- a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc
+++ b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc
@@ -48,7 +48,7 @@ void NHttpStreamSplitter::prepare_flush(NHttpFlowData* session_data, uint32_t* f
         paf_max = DATABLOCKSIZE;
         break;
     case SEC_CHUNK:
-        paf_max = DATABLOCKSIZE - session_data->chunk_buffer_length[source_id];
+        paf_max = DATABLOCKSIZE - session_data->section_buffer_length[source_id];
         break;
     default:
         paf_max = MAXOCTETS;
@@ -288,7 +288,8 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total,
     if (!NHttpTestManager::use_test_input() && NHttpTestManager::use_test_output())
     {
         printf("reassemble() from flow %p direction %d total %u length %u offset %u\n",
-            (void*)flow, 1 - (int)to_server(), total, len, offset); fflush(nullptr);
+            (void*)flow, 1 - (int)to_server(), total, len, offset);
+        fflush(stdout);
     }
 
     NHttpFlowData* session_data = (NHttpFlowData*)flow->get_application_data(
@@ -296,28 +297,31 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total,
     assert(session_data != nullptr);
     copied = len;
 
-    if (NHttpTestManager::use_test_input())
+    if (NHttpTestManager::use_test_output())
     {
-        if (!(flags & PKT_PDU_TAIL))
+        if (NHttpTestManager::use_test_input())
         {
-            return nullptr;
+            if (!(flags & PKT_PDU_TAIL))
+            {
+                return nullptr;
+            }
+            uint8_t* test_buffer;
+            NHttpTestManager::get_test_input_source()->reassemble(&test_buffer, len, source_id,
+                session_data, tcp_close);
+            if (test_buffer == nullptr)
+            {
+                // Source ID does not match test data, no test data was flushed, or there is no
+                // more test data
+                return nullptr;
+            }
+            data = test_buffer;
+            offset = 0;
         }
-        uint8_t* test_buffer;
-        NHttpTestManager::get_test_input_source()->reassemble(&test_buffer, len, source_id,
-            session_data, tcp_close);
-        if (test_buffer == nullptr)
+        else
         {
-            // Source ID does not match test data, no test data was flushed, or there is no more
-            // test data
-            return nullptr;
+            printf("Reassemble from flow data %p direction %d\n", (void*)session_data, source_id);
+            fflush(stdout);
         }
-        data = test_buffer;
-        offset = 0;
-    }
-    else if (NHttpTestManager::use_test_output())
-    {
-        printf("Reassemble from flow data %p direction %d\n", (void*)session_data, source_id);
-        fflush(stdout);
     }
 
     if (session_data->section_type[source_id] == SEC__NOTCOMPUTE)
@@ -346,28 +350,20 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total,
         return nullptr;
     }
 
-    bool is_chunk = (session_data->section_type[source_id] == SEC_CHUNK);
-
-    uint8_t*& chunk_buffer = session_data->chunk_buffer[source_id];
-    int32_t& chunk_buffer_length = session_data->chunk_buffer_length[source_id];
-    uint8_t*& buffer = !is_chunk ? session_data->section_buffer[source_id] : chunk_buffer;
-    int32_t& buffer_length = !is_chunk ? session_data->section_buffer_length[source_id] :
-        chunk_buffer_length;
-    bool& buffer_owned = !is_chunk ? session_data->section_buffer_owned[source_id] :
-        session_data->chunk_buffer_owned[source_id];
+    uint8_t*& buffer = session_data->section_buffer[source_id];
+    int32_t& buffer_length = session_data->section_buffer_length[source_id];
 
     if (buffer == nullptr)
     {
         buffer = new uint8_t[MAXOCTETS];
-        assert(buffer != nullptr);
-        buffer_owned = true;
+        session_data->section_buffer_owned[source_id] = true;
     }
 
     memcpy(buffer + buffer_length + offset, data, len);
     if (flags & PKT_PDU_TAIL)
     {
         ProcessResult send_to_detection;
-        if (!is_chunk)
+        if (session_data->section_type[source_id] != SEC_CHUNK)
         {
             // start line/headers/body individual section processing with aggregation prior to
             // being sent to detection
@@ -391,20 +387,20 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total,
             // FIXIT-M this implementation of the zero-length chunk is temporary until stream can
             // support a zero-
             // octet flush.
-            const int32_t total_chunk_len = chunk_buffer_length + offset + len -
-                session_data->zero_chunk[source_id];
+            const int32_t total_chunk_len = buffer_length + offset + len -
+                (int)session_data->zero_chunk[source_id];
             if ((total_chunk_len < DATABLOCKSIZE) && (!session_data->zero_chunk[source_id]) &&
                 !tcp_close)
             {
-                chunk_buffer_length = total_chunk_len;
+                buffer_length = total_chunk_len;
                 return nullptr;
             }
             if (total_chunk_len == 0)
             {
                 // Zero-length chunk cannot be processed by itself.
-                delete[] chunk_buffer;
-                chunk_buffer = nullptr;
-                chunk_buffer_length = 0;
+                delete[] buffer;
+                buffer = nullptr;
+                buffer_length = 0;
                 // zero-length chunk is not visible to inspector. Transition to trailer must be
                 // handled here.
                 session_data->section_type[source_id] = SEC__NOTCOMPUTE;
@@ -414,8 +410,8 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total,
             paf_max = DATABLOCKSIZE;
             session_data->infractions[source_id] = session_data->chunk_infractions[source_id];
             session_data->chunk_infractions[source_id] = NHttpInfractions();
-            send_to_detection = my_inspector->process(chunk_buffer, total_chunk_len, flow,
-                source_id, true);
+            send_to_detection = my_inspector->process(buffer, total_chunk_len, flow, source_id,
+                true);
             if (session_data->zero_chunk[source_id])
             {
                 // zero-length chunk is not visible to inspector. Transition to trailer must be
@@ -448,7 +444,7 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total,
             return nullptr;
         case RES_AGGREGATE:
             buffer_length += offset + len;
-            buffer_owned = false;
+            session_data->section_buffer_owned[source_id] = false;
             return nullptr;
         }
     }

diff --git a/src/service_inspectors/nhttp_inspect/nhttp_test_input.cc b/src/service_inspectors/nhttp_inspect/nhttp_test_input.cc
index a141e1eca5a17a62ef4befb9dd68e60d83f18804..fc29dac22c0b20d6b847766867322e9c0a69418e 100644 (file)
--- a/src/service_inspectors/nhttp_inspect/nhttp_test_input.cc
+++ b/src/service_inspectors/nhttp_inspect/nhttp_test_input.cc
@@ -302,7 +302,10 @@ void NHttpTestInput::reassemble(uint8_t** buffer, unsigned& length, SourceId sou
         // multiple times as we generate all the maximum size body sections needed for a single
         // flush.
         tcp_close = false;
-        const unsigned paf_max = DATABLOCKSIZE - session_data->chunk_buffer_length[source_id];
+        unsigned paf_max = DATABLOCKSIZE;
+        if (session_data->section_type[source_id] == SEC_CHUNK) {
+            paf_max -= session_data->section_buffer_length[source_id];
+        }
         length = (flush_octets <= paf_max) ? flush_octets : paf_max;
         for (uint32_t k = end_offset; k < length; k++)
         {