Pull request #5177: appid: address FIXIT comments related to http inspector

Merge in SNORT/snort3 from ~BHRYNIV/snort3:fixit_http to master

Squashed commit of the following:

commit 5e941db6ac32560ca1e0960912f4deeb0bfdf8d6
Author: Bohdan Hryniv <bhryniv@cisco>
Date:   Tue Feb 17 08:40:27 2026 -0500

    appid: address FIXIT comments related to http inspector

diff --git a/src/network_inspectors/appid/appid_http_event_handler.cc b/src/network_inspectors/appid/appid_http_event_handler.cc
index 9205ca02f7e9605e39f9799cc7dcf947e60c5f15..5da34f5c63fbb71304c005fb934c816a4bf0733c 100644 (file)
--- a/src/network_inspectors/appid/appid_http_event_handler.cc
+++ b/src/network_inspectors/appid/appid_http_event_handler.cc
@@ -148,10 +148,6 @@ void HttpEventHandler::handle(DataEvent& event, Flow* flow)
         header_start = http_event->get_referer(header_length);
         hsession->set_field(REQ_REFERER_FID, header_start, header_length, change_bits);
         hsession->set_is_webdav(http_event->contains_webdav_method());
-
-        // FIXIT-M: Should we get request body (may be expensive to copy)?
-        //      It is not currently set in callback in 2.9.x, only via
-        //      third-party.
     }
     else    // Response headers.
     {
@@ -174,11 +170,6 @@ void HttpEventHandler::handle(DataEvent& event, Flow* flow)
             if ( ret < sizeof(tmpstr) )
                 hsession->set_field(MISC_RESP_CODE_FID, (const uint8_t*)tmpstr, ret, change_bits);
         }
-
-        // FIXIT-M: Get Location header data.
-        // FIXIT-M: Should we get response body (may be expensive to copy)?
-        //      It is not currently set in callback in 2.9.x, only via
-        //      third-party.
     }
 
     header_start = http_event->get_x_working_with(header_length);

diff --git a/src/network_inspectors/appid/appid_http_session.cc b/src/network_inspectors/appid/appid_http_session.cc
index 6996ebbeb8f2c190026106c74195e2bce227ea76..5c31edb2bc99eda18d9101aff8bada64c82cc243 100644 (file)
--- a/src/network_inspectors/appid/appid_http_session.cc
+++ b/src/network_inspectors/appid/appid_http_session.cc
@@ -814,6 +814,9 @@ void AppIdHttpSession::set_field(HttpFieldIds id, const uint8_t* str, int32_t le
 {
     if (str and len)
     {
+        if (len > HTTP_FIELD_LEN_LIMIT)
+            len = HTTP_FIELD_LEN_LIMIT;
+
         delete meta_data[id];
         meta_data[id] = new std::string((const char*)str, len);
         set_http_change_bits(change_bits, id);
@@ -837,13 +840,13 @@ void AppIdHttpSession::set_req_body_field(HttpFieldIds id, const uint8_t* str, i
         }
 
         if (!meta_data[id])
-            meta_data[id] = new std::string((const char*)str, len);
-        else
+            meta_data[id] = new std::string((const char*)str,
+                len > HTTP_FIELD_LEN_LIMIT ? HTTP_FIELD_LEN_LIMIT : len);
+        else if (meta_data[id]->size() < HTTP_FIELD_LEN_LIMIT)
         {
-            std::string* req_body = new std::string(*meta_data[id]);
-            delete meta_data[id];
-            req_body->append((const char*)str, len);
-            meta_data[id] = req_body;
+            size_t remaining = HTTP_FIELD_LEN_LIMIT - meta_data[id]->size();
+            const_cast<std::string*>(meta_data[id])->append((const char*)str,
+                (size_t)len > remaining ? remaining : len);
         }
         set_http_change_bits(change_bits, id);
         set_scan_flags(id);
@@ -854,12 +857,16 @@ void AppIdHttpSession::set_req_body_field(HttpFieldIds id, const uint8_t* str, i
 
 void AppIdHttpSession::print_field(HttpFieldIds id, const std::string* field)
 {
-    string field_name;
+    const char* proto;
+    const char* field_name;
+
+    if (!appid_trace_enabled and !(appidDebug and appidDebug->is_active()))
+        return;
 
     if (asd.get_session_flags(APPID_SESSION_SPDY_SESSION))
-        field_name = "SPDY ";
+        proto = "SPDY";
     else if (asd.get_session_flags(APPID_SESSION_HTTP_SESSION))
-        field_name = "HTTP ";
+        proto = "HTTP";
     else
         // This could be RTMP session; not printing RTMP fields for now
         return;
@@ -867,51 +874,51 @@ void AppIdHttpSession::print_field(HttpFieldIds id, const std::string* field)
     switch (id)
     {
     case REQ_AGENT_FID:
-        field_name += "user agent";
+        field_name = "user agent";
         break;
 
     case REQ_HOST_FID:
-        field_name += "host";
+        field_name = "host";
         break;
 
     case REQ_REFERER_FID:
-        field_name += "referer";
+        field_name = "referer";
         break;
 
     case REQ_URI_FID:
-        field_name += "URI";
+        field_name = "URI";
         break;
 
     case REQ_COOKIE_FID:
-        field_name += "cookie";
+        field_name = "cookie";
         break;
 
     case REQ_BODY_FID:
-        field_name += "request body";
+        field_name = "request body";
         break;
 
     case RSP_CONTENT_TYPE_FID:
-        field_name += "content type";
+        field_name = "content type";
         break;
 
     case RSP_LOCATION_FID:
-        field_name += "location";
+        field_name = "location";
         break;
 
     case MISC_VIA_FID:
-        field_name += "via";
+        field_name = "via";
         break;
 
     case MISC_RESP_CODE_FID:
-        field_name += "response code";
+        field_name = "response code";
         break;
 
     case MISC_SERVER_FID:
-        field_name += "server";
+        field_name = "server";
         break;
 
     case MISC_XWW_FID:
-        field_name += "x-working-with";
+        field_name = "x-working-with";
         break;
 
     // don't print these fields
@@ -922,9 +929,9 @@ void AppIdHttpSession::print_field(HttpFieldIds id, const std::string* field)
     }
 
     if (httpx_stream_id >= 0)
-        APPID_LOG(CURRENT_PACKET, TRACE_DEBUG_LEVEL, "stream %" PRId64 ": %s is %s\n",
-            httpx_stream_id, field_name.c_str(), field->c_str());
-    else 
-        APPID_LOG(CURRENT_PACKET, TRACE_DEBUG_LEVEL, "%s is %s\n", field_name.c_str(), field->c_str());
+        APPID_LOG(CURRENT_PACKET, TRACE_DEBUG_LEVEL, "stream %" PRId64 ": %s %s is %s\n",
+            httpx_stream_id, proto, field_name, field->c_str());
+    else
+        APPID_LOG(CURRENT_PACKET, TRACE_DEBUG_LEVEL, "%s %s is %s\n", proto, field_name, field->c_str());
 }
 

diff --git a/src/network_inspectors/appid/appid_http_session.h b/src/network_inspectors/appid/appid_http_session.h
index 2ddd7025c5575ced0b7c9e764c0f3c0de1d7833d..856f5d08dbdce1cc4aa34cf89e2c6cc1c0697c5c 100644 (file)
--- a/src/network_inspectors/appid/appid_http_session.h
+++ b/src/network_inspectors/appid/appid_http_session.h
@@ -33,6 +33,8 @@
 #include "appid_types.h"
 #include "application_ids.h"
 
+#define HTTP_FIELD_LEN_LIMIT 2048
+
 class AppIdSession;
 class ChpMatchDescriptor;
 class HttpPatternMatchers;
@@ -170,13 +172,6 @@ protected:
 
     AppIdSession& asd;
 
-    // FIXIT-M the meta data buffers in this array are only set from
-    // third party (tp_appid_utils.cc) and from http inspect
-    // (appid_http_event_handler.cc). The set_field functions should
-    // only be accessible to those functions/classes, but the process
-    // functions in tp_appid_utils.cc are static. Thus the public
-    // set_field() functions in AppIdHttpSession. We do need set functions
-    // for this array, as old pointers need to be deleted upon set().
     const std::string* meta_data[NUM_METADATA_FIELDS] = { };
 
     bool is_webdav = false;