From a5bd1bf81b2119c57de2ef76127dae35177a167e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 18:41:18 +0000 Subject: [PATCH 1/5] Initial plan From 2f77e98a16c23d4c04c57ccc1cd92021a58d067c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 18:45:37 +0000 Subject: [PATCH 2/5] Add ONVIF subscription retry logic with improved error handling Co-authored-by: connortechnology <925519+connortechnology@users.noreply.github.com> --- src/zm_monitor_onvif.cpp | 107 ++++++++++++++++++++++++++++----------- 1 file changed, 78 insertions(+), 29 deletions(-) diff --git a/src/zm_monitor_onvif.cpp b/src/zm_monitor_onvif.cpp index 7a2fa382b29..bbd1e82d57a 100644 --- a/src/zm_monitor_onvif.cpp +++ b/src/zm_monitor_onvif.cpp @@ -19,8 +19,10 @@ #include "zm_monitor.h" +#include #include #include +#include #include "url.hpp" std::string SOAP_STRINGS[] = { @@ -101,41 +103,88 @@ void Monitor::ONVIF::start() { std::string full_url = url.str() + parent->onvif_events_path; proxyEvent.soap_endpoint = full_url.c_str(); set_credentials(soap); + + // Add initial delay to allow any previous subscription to expire + Debug(2, "ONVIF: Waiting 2 seconds before creating subscription to allow stale subscriptions to expire"); + std::this_thread::sleep_for(std::chrono::seconds(2)); + const char *RequestMessageID = parent->soap_wsa_compl ? soap_wsa_rand_uuid(soap) : "RequestMessageID"; if ((!parent->soap_wsa_compl) || (soap_wsa_request(soap, RequestMessageID, proxyEvent.soap_endpoint, "CreatePullPointSubscriptionRequest") == SOAP_OK)) { Debug(1, "ONVIF Endpoint: %s", proxyEvent.soap_endpoint); - int rc = proxyEvent.CreatePullPointSubscription(&request, response); -#if 0 - std::stringstream ss; - soap->os = &ss; // assign a stringstream to write output to - soap_write__tev__CreatePullPointSubscriptionResponse(soap, &response); - soap->os = NULL; // no longer writing to the stream - Debug(1, "Response was %s", ss.str().c_str()); -#endif - - if (rc != SOAP_OK) { - const char *detail = soap_fault_detail(soap); - if (rc > 8) { - Error("ONVIF Couldn't create subscription at %s! %d, fault:%s, detail:%s", full_url.c_str(), - rc, soap_fault_string(soap), detail ? detail : "null"); + + // Retry logic for CreatePullPointSubscription + const int max_retries = 3; + const int retry_delay_seconds = 3; + int rc = SOAP_OK; + bool subscription_successful = false; + + for (int attempt = 1; attempt <= max_retries && !subscription_successful; attempt++) { + if (attempt > 1) { + Debug(1, "ONVIF: Retry attempt %d/%d after %d second delay", attempt, max_retries, retry_delay_seconds); + std::this_thread::sleep_for(std::chrono::seconds(retry_delay_seconds)); + // Reset soap context for retry + soap_destroy(soap); + soap_end(soap); + set_credentials(soap); + RequestMessageID = parent->soap_wsa_compl ? soap_wsa_rand_uuid(soap) : "RequestMessageID"; + if (parent->soap_wsa_compl) { + if (soap_wsa_request(soap, RequestMessageID, proxyEvent.soap_endpoint, "CreatePullPointSubscriptionRequest") != SOAP_OK) { + Error("ONVIF: Failed to set WSA headers on retry attempt %d", attempt); + continue; + } + } + } + + rc = proxyEvent.CreatePullPointSubscription(&request, response); + + if (rc == SOAP_OK) { + subscription_successful = true; + Debug(1, "ONVIF: Successfully created pull point subscription on attempt %d", attempt); } else { - Error("ONVIF Couldn't create subscription at %s! %d %s, fault:%s, detail:%s", full_url.c_str(), - rc, SOAP_STRINGS[rc].c_str(), - soap_fault_string(soap), detail ? detail : "null"); + // Check for specific fault types + const char *detail = soap_fault_detail(soap); + const char *fault_string = soap_fault_string(soap); + bool is_subscribe_creation_failed = (detail && std::strstr(detail, "SubscribeCreationFailedFault")) || + (fault_string && std::strstr(fault_string, "SubscribeCreationFailedFault")); + + if (rc > 8) { + Error("ONVIF Couldn't create subscription at %s (attempt %d/%d)! error_code=%d, fault:%s, detail:%s", + full_url.c_str(), attempt, max_retries, rc, + fault_string ? fault_string : "null", + detail ? detail : "null"); + } else { + Error("ONVIF Couldn't create subscription at %s (attempt %d/%d)! error_code=%d (%s), fault:%s, detail:%s", + full_url.c_str(), attempt, max_retries, rc, SOAP_STRINGS[rc].c_str(), + fault_string ? fault_string : "null", + detail ? detail : "null"); + } + + if (is_subscribe_creation_failed) { + Warning("ONVIF: SubscribeCreationFailedFault detected. This typically means:"); + Warning(" - Camera has reached max concurrent subscriptions"); + Warning(" - Stale subscriptions from previous sessions exist"); + Warning(" - Camera doesn't automatically expire old subscriptions"); + if (attempt < max_retries) { + Info("ONVIF: Will retry after %d seconds to allow subscriptions to expire...", retry_delay_seconds); + } + } + + // Only log detailed response on last attempt + if (attempt == max_retries) { + std::stringstream ss; + std::ostream *old_stream = soap->os; + soap->os = &ss; + soap_write__tev__CreatePullPointSubscriptionResponse(soap, &response); + soap->os = old_stream; + Debug(1, "Response was %s", ss.str().c_str()); + } } - - std::stringstream ss; - std::ostream *old_stream = soap->os; - soap->os = &ss; // assign a stringstream to write output to - proxyEvent.CreatePullPointSubscription(&request, response); - soap_write__tev__CreatePullPointSubscriptionResponse(soap, &response); - soap->os = old_stream; // no longer writing to the stream - Debug(1, "Response was %s", ss.str().c_str()); - - _wsnt__Unsubscribe wsnt__Unsubscribe; - _wsnt__UnsubscribeResponse wsnt__UnsubscribeResponse; - proxyEvent.Unsubscribe(response.SubscriptionReference.Address, nullptr, &wsnt__Unsubscribe, wsnt__UnsubscribeResponse); + } // end retry loop + + if (!subscription_successful) { + // Clean up and exit after all retries failed + Error("ONVIF: Failed to create subscription after %d attempts", max_retries); soap_destroy(soap); soap_end(soap); soap_free(soap); From 1ecc9c7e68a6b8299293899b24f4ff0c3677c5c0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 18:47:16 +0000 Subject: [PATCH 3/5] Address code review feedback: add bounds checking and optimize string search Co-authored-by: connortechnology <925519+connortechnology@users.noreply.github.com> --- src/zm_monitor_onvif.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/zm_monitor_onvif.cpp b/src/zm_monitor_onvif.cpp index bbd1e82d57a..2e77ee724f4 100644 --- a/src/zm_monitor_onvif.cpp +++ b/src/zm_monitor_onvif.cpp @@ -114,8 +114,9 @@ void Monitor::ONVIF::start() { Debug(1, "ONVIF Endpoint: %s", proxyEvent.soap_endpoint); // Retry logic for CreatePullPointSubscription - const int max_retries = 3; - const int retry_delay_seconds = 3; + // These values can be tuned for different camera behaviors + constexpr int max_retries = 3; + constexpr int retry_delay_seconds = 3; int rc = SOAP_OK; bool subscription_successful = false; @@ -145,10 +146,13 @@ void Monitor::ONVIF::start() { // Check for specific fault types const char *detail = soap_fault_detail(soap); const char *fault_string = soap_fault_string(soap); - bool is_subscribe_creation_failed = (detail && std::strstr(detail, "SubscribeCreationFailedFault")) || - (fault_string && std::strstr(fault_string, "SubscribeCreationFailedFault")); + const char *subscribe_creation_failed_in_detail = detail ? std::strstr(detail, "SubscribeCreationFailedFault") : nullptr; + const char *subscribe_creation_failed_in_fault = fault_string ? std::strstr(fault_string, "SubscribeCreationFailedFault") : nullptr; + bool is_subscribe_creation_failed = subscribe_creation_failed_in_detail || subscribe_creation_failed_in_fault; - if (rc > 8) { + // Safely access SOAP_STRINGS array (has 13 elements, indices 0-12) + constexpr int SOAP_STRINGS_SIZE = 13; + if (rc >= SOAP_STRINGS_SIZE || rc < 0) { Error("ONVIF Couldn't create subscription at %s (attempt %d/%d)! error_code=%d, fault:%s, detail:%s", full_url.c_str(), attempt, max_retries, rc, fault_string ? fault_string : "null", From 04edbb5278f7c625ef2b5f0af9f6685c391090e9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 18:48:26 +0000 Subject: [PATCH 4/5] Implementation complete Co-authored-by: connortechnology <925519+connortechnology@users.noreply.github.com> --- _codeql_detected_source_root | 1 + 1 file changed, 1 insertion(+) create mode 120000 _codeql_detected_source_root diff --git a/_codeql_detected_source_root b/_codeql_detected_source_root new file mode 120000 index 00000000000..945c9b46d68 --- /dev/null +++ b/_codeql_detected_source_root @@ -0,0 +1 @@ +. \ No newline at end of file From 14ed022161054b11759217e2726f5578fc1082f9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 18:49:05 +0000 Subject: [PATCH 5/5] Remove codeql symlink and update gitignore Co-authored-by: connortechnology <925519+connortechnology@users.noreply.github.com> --- .gitignore | 1 + _codeql_detected_source_root | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 120000 _codeql_detected_source_root diff --git a/.gitignore b/.gitignore index ee6ac35e77d..9e0a0c12244 100644 --- a/.gitignore +++ b/.gitignore @@ -216,3 +216,4 @@ misc/com.zoneminder.arp-scan.rules misc/nginx.conf misc/zm-sudo misc/zoneminder.desktop +_codeql_detected_source_root diff --git a/_codeql_detected_source_root b/_codeql_detected_source_root deleted file mode 120000 index 945c9b46d68..00000000000 --- a/_codeql_detected_source_root +++ /dev/null @@ -1 +0,0 @@ -. \ No newline at end of file