From 70fdb50d55fca7ba84e55d9de4dee730944f0aaa Mon Sep 17 00:00:00 2001 From: "Daniil Kolomiiets -X (dkolomii - SOFTSERVE INC at Cisco)" Date: Wed, 18 Jun 2025 13:43:56 +0000 Subject: [PATCH] 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 Date: Mon Jun 16 04:41:20 2025 -0400 appid: fixed APPID_LOG macro for correct usage of log_level --- src/network_inspectors/appid/appid_debug.h | 3 +- .../appid/test/appid_debug_test.cc | 96 ++++++++++++++++++- .../appid/test/appid_discovery_test.cc | 1 + .../appid/test/appid_mock_definitions.h | 2 + .../appid/test/service_state_test.cc | 1 + 5 files changed, 98 insertions(+), 5 deletions(-) diff --git a/src/network_inspectors/appid/appid_debug.h b/src/network_inspectors/appid/appid_debug.h index e61d82ea6..18bbd40b7 100644 --- 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 564113bca..56197b9e4 100644 --- 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 5c6ba86d7..66d0b1e87 100644 --- 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 c36147c6d..0b14ad282 100644 --- 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 b7762342f..18d54a53a 100644 --- 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 THREAD_LOCAL bool TimeProfilerStats::enabled = false; +THREAD_LOCAL bool appid_trace_enabled = false; namespace snort { -- 2.47.2