Pull request #4764: appid: fix APPID_LOG macro for correct usage of log_level

Merge in SNORT/snort3 from ~DKOLOMII/snort3:APPID_LOG_macro_fix to master

Squashed commit of the following:

commit 09023c0f0cb9cc0b625fde8236b0067369a55702
Author: Daniil Kolomiiets <dkolomii@cisco.com>
Date:   Mon Jun 16 04:41:20 2025 -0400

    appid: fixed APPID_LOG macro for correct usage of log_level

diff --git a/src/network_inspectors/appid/appid_debug.h b/src/network_inspectors/appid/appid_debug.h
index e61d82ea69c5736bd917f7836f23838ad7a1a70c..18bbd40b7ddfc07c2c3dba7f2e3272943ad121fe 100644 (file)
--- a/src/network_inspectors/appid/appid_debug.h
+++ b/src/network_inspectors/appid/appid_debug.h
@@ -48,7 +48,8 @@ namespace snort
 void appid_log(const snort::Packet*, const uint8_t log_level, const char*, ...);
 
 #define APPID_LOG(pkt, log_level, ...) do { \
-    if ((log_level > 2) || (appidDebug and appidDebug->is_active()) || (appid_trace_enabled)) { \
+    if ((log_level >= TRACE_CRITICAL_LEVEL and log_level <= TRACE_INFO_LEVEL) || \
+        (appidDebug and appidDebug->is_active()) || (appid_trace_enabled)) { \
                appid_log(pkt, log_level, __VA_ARGS__); \
     } \
 } while(0)

diff --git a/src/network_inspectors/appid/test/appid_debug_test.cc b/src/network_inspectors/appid/test/appid_debug_test.cc
index 564113bca8154034b6f903a8238cf463ec5eddaa..56197b9e41ee6e4044ab5dab1318045ef1c7368e 100644 (file)
--- a/src/network_inspectors/appid/test/appid_debug_test.cc
+++ b/src/network_inspectors/appid/test/appid_debug_test.cc
@@ -48,10 +48,27 @@ Packet::~Packet() = default;
 FlowData::FlowData(unsigned, Inspector*) { }
 FlowData::~FlowData() = default;
 AppIdSessionApi::AppIdSessionApi(const AppIdSession* asd, const SfIp& ip) : asd(asd), initiator_ip(ip) { }
-[[noreturn]] void FatalError(const char*,...) {  exit(-1); }
-void ErrorMessage(const char*, va_list&) { }
-void WarningMessage(const char*, va_list&) { }
-void LogMessage(const char*, va_list&) { }
+// Stubs for logs
+char test_log[256];
+void FatalError(const char* format,...) 
+{
+    va_list ap;
+    va_start(ap,format);
+    vsprintf(test_log, format, ap); 
+    va_end(ap);  
+}
+void ErrorMessage(const char* format, va_list& args)
+{
+    vsprintf(test_log, format, args);
+}
+void WarningMessage(const char* format, va_list& args)
+{
+    vsprintf(test_log, format, args);
+}
+void LogMessage(const char* format, va_list& args)
+{
+    vsprintf(test_log, format, args);
+}
 void TraceApi::filter(snort::Packet const&) { }
 void trace_vprintf(const char*, unsigned char, const char*, const Packet*, const char*, va_list) { }
 uint8_t TraceApi::get_constraints_generation() { return 0; }
@@ -542,3 +559,74 @@ int main(int argc, char** argv)
     int rc = CommandLineTestRunner::RunAllTests(argc, argv);
     return rc;
 }
+
+TEST(appid_debug, no_appidDebug_logging)
+{
+    test_log[0] = '\0';
+    APPID_LOG(nullptr, TRACE_CRITICAL_LEVEL, "TRACE_CRITICAL_LEVEL log message");
+    STRCMP_EQUAL(test_log, "TRACE_CRITICAL_LEVEL log message");
+
+    test_log[0] = '\0';
+    APPID_LOG(nullptr, TRACE_ERROR_LEVEL, "TRACE_ERROR_LEVEL log message");
+    STRCMP_EQUAL(test_log, "TRACE_ERROR_LEVEL log message");
+
+    test_log[0] = '\0';
+    APPID_LOG(nullptr, TRACE_WARNING_LEVEL, "TRACE_WARNING_LEVEL log message");
+    STRCMP_EQUAL(test_log, "TRACE_WARNING_LEVEL log message");
+
+    test_log[0] = '\0';
+    APPID_LOG(nullptr, TRACE_INFO_LEVEL, "TRACE_INFO_LEVEL log message");
+    STRCMP_EQUAL(test_log, "TRACE_INFO_LEVEL log message");
+
+    test_log[0] = '\0';
+    APPID_LOG(nullptr, TRACE_DEBUG_LEVEL, "TRACE_DEBUG_LEVEL log message");
+    STRCMP_EQUAL(test_log, "");
+}
+
+TEST(appid_debug, appidDebug_logging)
+{
+    // Activating appidDebug for debug messages
+    SfIp sip;
+    SfIp dip;
+    dip.set("10.1.2.3");
+    AppIdInspector inspector;
+    AppIdSession session(IpProtocol::PROTO_NOT_SET, &dip, 0, inspector, stub_odp_ctxt, 0
+#ifndef DISABLE_TENANT_ID
+    ,0
+#endif
+    );
+    // This packet...
+    sip.set("10.9.8.7");    // this would be a reply back
+    uint16_t sport = 80;
+    uint16_t dport = 48620;
+    IpProtocol protocol = IpProtocol::TCP;
+    uint32_t address_space_id = 0;
+    // The session...
+    session.initiator_port = dport;    // session initiator is now dst
+    // activate()
+    appidDebug->activate(sip.get_ip6_ptr(), dip.get_ip6_ptr(), sport, dport,
+        protocol, 4, address_space_id, &session, true, 0);
+
+    test_log[0] = '\0';
+    APPID_LOG(nullptr , TRACE_DEBUG_LEVEL, "TRACE_DEBUG_LEVEL log message");
+    STRCMP_EQUAL(test_log, "TRACE_DEBUG_LEVEL log message");
+
+    Packet packet(true);
+    test_log[0] = '\0';
+    APPID_LOG(&packet , TRACE_DEBUG_LEVEL, "TRACE_DEBUG_LEVEL log message");
+    STRCMP_EQUAL(test_log, (string("AppIdDbg ") + appidDebug->get_debug_session() + 
+        " TRACE_DEBUG_LEVEL log message").c_str());
+
+    delete &session.get_api();
+}
+
+TEST(appid_debug, trace_on_logging)
+{
+    appid_trace_enabled = true; // Enable appid_trace for debug messages
+
+    test_log[0] = '\0';
+    APPID_LOG(nullptr , TRACE_DEBUG_LEVEL, "TRACE_DEBUG_LEVEL log message");
+    STRCMP_EQUAL(test_log, "TRACE_DEBUG_LEVEL log message");
+
+    appid_trace_enabled = false; // Disable appid_trace
+}
\ No newline at end of file

diff --git a/src/network_inspectors/appid/test/appid_discovery_test.cc b/src/network_inspectors/appid/test/appid_discovery_test.cc
index 5c6ba86d723ada56eb6dd7eebe287e4993310407..66d0b1e876a3cd3e4b87595d8d6233f3dee0b889 100644 (file)
--- a/src/network_inspectors/appid/test/appid_discovery_test.cc
+++ b/src/network_inspectors/appid/test/appid_discovery_test.cc
@@ -44,6 +44,7 @@
 
 uint32_t ThirdPartyAppIdContext::next_version = 0;
 THREAD_LOCAL bool TimeProfilerStats::enabled = false;
+THREAD_LOCAL bool appid_trace_enabled = false;
 
 namespace snort
 {

diff --git a/src/network_inspectors/appid/test/appid_mock_definitions.h b/src/network_inspectors/appid/test/appid_mock_definitions.h
index c36147c6deb6532df7881f825c802a26ac9b4fa6..0b14ad282a4b1c271bee1d6da17b8b8062056ce5 100644 (file)
--- a/src/network_inspectors/appid/test/appid_mock_definitions.h
+++ b/src/network_inspectors/appid/test/appid_mock_definitions.h
@@ -29,6 +29,8 @@
 #include "appid_peg_counts.h"
 #include "service_inspectors/http_inspect/http_msg_header.h"
 
+THREAD_LOCAL bool appid_trace_enabled = false;
+
 class ThirdPartyAppIdContext;
 
 ThirdPartyAppIdContext* tp_appid_ctxt = nullptr;

diff --git a/src/network_inspectors/appid/test/service_state_test.cc b/src/network_inspectors/appid/test/service_state_test.cc
index b7762342f65fb8b3af0f517cc3350d8e51454eec..18d54a53aa533d2817402dce3999df94ee289629 100644 (file)
--- a/src/network_inspectors/appid/test/service_state_test.cc
+++ b/src/network_inspectors/appid/test/service_state_test.cc
@@ -29,6 +29,7 @@
 #include <vector>
 
 THREAD_LOCAL bool TimeProfilerStats::enabled = false;
+THREAD_LOCAL bool appid_trace_enabled = false;
 
 namespace snort
 {