fix(onvif): make events work with strict ONVIF servers (Reolink) (#374)

Reolink cameras' ONVIF events stack was rejecting every PullMessages
call with HTTP 400 + Code=SOAP-ENV:Sender and an empty Reason, even
though CreatePullPointSubscription succeeded. For a second Reolink
model (Argus) we couldn't even discover the events XAddr from
GetServices. Four changes land together:

A. Add WS-Addressing headers (wsa:MessageID, wsa:To, wsa:Action) to
   every SOAP request this file emits, and set the SOAP 1.2 `action=`
   parameter on the Content-Type header. Permissive ONVIF servers
   (Hikvision, Dahua, Axis, Amcrest) accept our requests either way,
   but strict stacks (Reolink, some Vivotek firmwares) reject anything
   without these. Thread a `const char *action` through
   create_onvif_request / send_onvif_request_to_url /
   send_onvif_request; action URIs per the ONVIF WSDLs for
   GetServices, CreatePullPointSubscription, and PullMessages. Use the
   full request URL as `wsa:To` for requests to subscription-specific
   endpoints (PullMessages landing on `…/PullSubManager?Idx=…` URLs).

B. The Content-Type change is folded into the same threading: when an
   action is provided we emit
     Content-Type: application/soap+xml; charset=utf-8; action="…"
   which SOAP 1.2 spec names as the canonical way to convey the
   action; some strict servers use this instead of the wsa header.

C. Improve SOAP fault logging. When the parsed fault has a Code but
   no Reason/Text element (Reolink's exact failure shape), dump a
   truncated snippet of the raw response XML at ERROR so future bug
   reports carry the full vendor payload — previously the diagnostic
   was "Code=SOAP-ENV:Sender, Reason=(none)" and no pcap, which left
   us unable to see what the camera was actually complaining about.
   Added a unit test for the Reolink fault shape.

D. Rewrite discover_event_service_url to parse GetServices with ezxml
   and walk <Service> children comparing <Namespace> text. The old
   substring-and-512-byte-window heuristic missed the events XAddr
   whenever a service block included a Capabilities subtree (#374,
   Argus case). The old heuristic is kept as a last-resort fallback
   with a wider 2048-byte window, and the /onvif/Events path is added
   to the fallback probe list for firmwares that serve events there.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: matteius

src/video/onvif_detection.c

@@ -22,6 +22,18 @@
 #include "video/onvif_motion_recording.h"
 #include "video/zone_filter.h"
 #include "database/db_detections.h"
+#include "ezxml.h"
+
+/* WS-Addressing action URIs for the ONVIF operations this file emits.
+ * Strict ONVIF servers (Reolink, some Vivotek) reject requests that don't
+ * carry a matching wsa:Action header and `action=` Content-Type parameter —
+ * see #374. */
+#define ONVIF_ACTION_GET_SERVICES        "http://www.onvif.org/ver10/device/wsdl/GetServices"
+#define ONVIF_ACTION_CREATE_PULLPOINT    "http://www.onvif.org/ver10/events/wsdl/EventPortType/CreatePullPointSubscriptionRequest"
+#define ONVIF_ACTION_PULL_MESSAGES       "http://www.onvif.org/ver10/events/wsdl/PullPointSubscription/PullMessagesRequest"
+
+/* External UUID generator (used for wsa:MessageID). */
+extern void generate_uuid(char *uuid, size_t size);
 
 // Global variables
 static bool initialized = false;
@@ -70,8 +82,15 @@ static size_t write_memory_callback(void *contents, size_t size, size_t nmemb, v
     return realsize;
 }
 
-// Create ONVIF SOAP request with WS-Security (if credentials provided)
-static char *create_onvif_request(const char *username, const char *password, const char *request_body) {
+/*
+ * Create ONVIF SOAP request with WS-Security (if credentials provided) and
+ * WS-Addressing headers. `action` and `to` are the WS-Addressing action URI
+ * and destination URL respectively; both are required (strict ONVIF servers
+ * such as Reolink reject requests missing either — #374).
+ */
+static char *create_onvif_request(const char *username, const char *password,
+                                  const char *request_body,
+                                  const char *action, const char *to) {
     bool has_credentials = (username && strlen(username) > 0 && password && strlen(password) > 0);
     char *security_header = NULL;
 
@@ -87,8 +106,22 @@ static char *create_onvif_request(const char *username, const char *password, co
         }
     }
 
-    // Allocate envelope dynamically to accommodate variable-length body and header
-    size_t envelope_size = strlen(request_body) + strlen(security_header) + 1024;
+    /* Build a WS-Addressing fragment if an action was provided. Using
+     * `urn:uuid:<v4>` for MessageID and leaving mustUnderstand off on To/Action
+     * — cameras that ignore WS-Addressing shouldn't then fault because they
+     * "didn't understand" an optional header. */
+    char uuid[48] = {0};
+    generate_uuid(uuid, sizeof(uuid));
+
+    const char *safe_action = action ? action : "";
+    const char *safe_to     = to     ? to     : "";
+
+    /* The WS-Addressing block inflates the envelope; reserve enough for
+     * MessageID (urn:uuid:… = ~45 chars), the action URI, the destination URL,
+     * the security header, and the body. 2 KB of slack above those sizes is
+     * plenty. */
+    size_t envelope_size = strlen(request_body) + strlen(security_header)
+                           + strlen(safe_action) + strlen(safe_to) + 2048;
     char *soap_request = malloc(envelope_size);
     if (!soap_request) {
         free(security_header);
@@ -97,28 +130,43 @@ static char *create_onvif_request(const char *username, const char *password, co
 
     snprintf(soap_request, envelope_size,
         "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
-        "<s:Envelope xmlns:s=\"http://www.w3.org/2003/05/soap-envelope\">"
-        "<s:Header>%s</s:Header>"
+        "<s:Envelope xmlns:s=\"http://www.w3.org/2003/05/soap-envelope\" "
+                   "xmlns:wsa=\"http://www.w3.org/2005/08/addressing\">"
+        "<s:Header>"
+            "<wsa:MessageID>urn:uuid:%s</wsa:MessageID>"
+            "<wsa:To>%s</wsa:To>"
+            "<wsa:Action>%s</wsa:Action>"
+            "%s"
+        "</s:Header>"
         "<s:Body xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" "
-        "xmlns:xsd=\"http://www.w3.org/2001/XMLSchema\">%s</s:Body>"
+                "xmlns:xsd=\"http://www.w3.org/2001/XMLSchema\">%s</s:Body>"
         "</s:Envelope>",
-        security_header, request_body);
+        uuid, safe_to, safe_action, security_header, request_body);
 
     free(security_header);
     return soap_request;
 }
 
-// Send ONVIF request to a full URL (bypassing the base URL + /onvif/ + service construction)
+/*
+ * Send ONVIF request to a full URL (bypassing the base URL + /onvif/ + service
+ * construction). `action` is the WS-Addressing action URI; it's also added as
+ * the `action=` parameter on the Content-Type header per SOAP 1.2, which
+ * strict ONVIF servers require (#374).
+ */
 static char *send_onvif_request_to_url(const char *full_url, const char *username,
-                                       const char *password, const char *request_body) {
+                                       const char *password, const char *request_body,
+                                       const char *action) {
     if (!initialized || !curl_handle) {
         log_error("ONVIF detection system not initialized");
         return NULL;
     }
 
     log_info("ONVIF Detection: Sending request to %s", full_url);
 
-    char *soap_request = create_onvif_request(username, password, request_body);
+    /* Use full_url as the WS-Addressing To, which is exactly what the request
+     * is being POSTed to. */
+    char *soap_request = create_onvif_request(username, password, request_body,
+                                              action, full_url);
     if (!soap_request) {
         log_error("Failed to create ONVIF request");
         return NULL;
@@ -131,9 +179,20 @@ static char *send_onvif_request_to_url(const char *full_url, const char *usernam
     curl_easy_setopt(curl_handle, CURLOPT_POSTFIELDS, soap_request);
     curl_easy_setopt(curl_handle, CURLOPT_POSTFIELDSIZE, strlen(soap_request));
 
-    // Set headers
+    /* Content-Type carries the SOAP 1.2 action parameter in addition to the
+     * charset. Fall back to the plain type when no action is provided. */
+    char content_type[512];
+    if (action && action[0] != '\0') {
+        snprintf(content_type, sizeof(content_type),
+                 "Content-Type: application/soap+xml; charset=utf-8; action=\"%s\"",
+                 action);
+    } else {
+        snprintf(content_type, sizeof(content_type),
+                 "Content-Type: application/soap+xml; charset=utf-8");
+    }
+
     struct curl_slist *headers = NULL;
-    headers = curl_slist_append(headers, "Content-Type: application/soap+xml; charset=utf-8");
+    headers = curl_slist_append(headers, content_type);
     curl_easy_setopt(curl_handle, CURLOPT_HTTPHEADER, headers);
 
     // Set up response buffer
@@ -179,8 +238,9 @@ static char *send_onvif_request_to_url(const char *full_url, const char *usernam
 }
 
 // Send ONVIF request and get response
-static char *send_onvif_request(const char *url, const char *username, const char *password, 
-                               const char *request_body, const char *service) {
+static char *send_onvif_request(const char *url, const char *username, const char *password,
+                               const char *request_body, const char *service,
+                               const char *action) {
     if (!initialized || !curl_handle) {
         log_error("ONVIF detection system not initialized");
         return NULL;
@@ -190,7 +250,7 @@ static char *send_onvif_request(const char *url, const char *username, const cha
     char full_url[512];
     snprintf(full_url, sizeof(full_url), "%s/onvif/%s", url, service);
 
-    return send_onvif_request_to_url(full_url, username, password, request_body);
+    return send_onvif_request_to_url(full_url, username, password, request_body, action);
 }
 
 // Extract subscription address from response
@@ -219,45 +279,92 @@ static char *extract_subscription_address(const char *response) {
     return NULL;
 }
 
-// Query GetServices and return the event service URL, or NULL if not found.
-// The returned string is heap-allocated; caller must free() it.
+/* Return the local name of an ezxml element (strips any "prefix:" prefix). */
+static const char *local_name(ezxml_t el) {
+    if (!el || !el->name) return "";
+    const char *colon = strchr(el->name, ':');
+    return colon ? colon + 1 : el->name;
+}
+
+/* Find a direct child with the given local name, ignoring namespace prefix. */
+static ezxml_t child_by_local_name(ezxml_t parent, const char *local) {
+    if (!parent || !local) return NULL;
+    for (ezxml_t c = parent->child; c; c = c->ordered) {
+        if (strcmp(local_name(c), local) == 0) return c;
+    }
+    return NULL;
+}
+
+/*
+ * Query GetServices and return the event service URL, or NULL if not found.
+ * The returned string is heap-allocated; caller must free() it.
+ *
+ * Previous implementation used substring matching with a 512-byte window,
+ * which misses the XAddr when a Service element carries a Capabilities
+ * subtree (#374, Reolink Argus). We now parse the response with ezxml and
+ * walk Service elements, comparing the Namespace text to the events WSDL.
+ */
 static char *discover_event_service_url(const char *url, const char *username, const char *password) {
     const char *request_body =
         "<GetServices xmlns=\"http://www.onvif.org/ver10/device/wsdl\">"
             "<IncludeCapability>false</IncludeCapability>"
         "</GetServices>";
 
     // GetServices is always sent to the standard device management endpoint
-    char *response = send_onvif_request(url, username, password, request_body, "device_service");
+    char *response = send_onvif_request(url, username, password, request_body,
+                                        "device_service", ONVIF_ACTION_GET_SERVICES);
     if (!response) {
         log_warn("ONVIF: GetServices to device_service endpoint failed");
         return NULL;
     }
 
-    // Find the events service namespace in the response
-    const char *events_ns = "http://www.onvif.org/ver10/events/wsdl";
-    const char *ns_pos = strstr(response, events_ns);
-    if (!ns_pos) {
-        log_warn("ONVIF: Events service namespace not found in GetServices response");
-        free(response);
-        return NULL;
-    }
-
-    // Search forward from the namespace position for XAddr (prefixed and un-prefixed variants)
-    const char *xaddr_open[]  = {"<tds:XAddr>", "<XAddr>", NULL};
-    const char *xaddr_close[] = {"</tds:XAddr>", "</XAddr>", NULL};
+    /* ezxml_parse_str modifies the buffer in place, and the returned
+     * tree references it, so keep `response` alive until we copy the
+     * matched URL out. */
+    ezxml_t xml = ezxml_parse_str(response, strlen(response));
     char *event_url = NULL;
 
-    for (int i = 0; xaddr_open[i] != NULL; i++) {
-        const char *tag = strstr(ns_pos, xaddr_open[i]);
-        // Only accept the XAddr if it is within the same Service element (~512 bytes window)
-        if (tag && (tag - ns_pos) < 512) {
-            const char *val_start = tag + strlen(xaddr_open[i]);
-            const char *val_end   = strstr(val_start, xaddr_close[i]);
-            if (val_end) {
-                size_t url_len = (size_t)(val_end - val_start);
-                event_url = strndup(val_start, url_len);
-                break;
+    if (xml) {
+        ezxml_t body = child_by_local_name(xml, "Body");
+        ezxml_t resp = body ? child_by_local_name(body, "GetServicesResponse") : NULL;
+
+        if (resp) {
+            const char *events_ns = "http://www.onvif.org/ver10/events/wsdl";
+            for (ezxml_t svc = resp->child; svc && !event_url; svc = svc->ordered) {
+                if (strcmp(local_name(svc), "Service") != 0) continue;
+
+                ezxml_t ns    = child_by_local_name(svc, "Namespace");
+                ezxml_t xaddr = child_by_local_name(svc, "XAddr");
+                const char *ns_text    = ns    ? ezxml_txt(ns)    : "";
+                const char *xaddr_text = xaddr ? ezxml_txt(xaddr) : "";
+
+                if (ns_text && xaddr_text && xaddr_text[0] != '\0'
+                    && strcmp(ns_text, events_ns) == 0) {
+                    event_url = strdup(xaddr_text);
+                }
+            }
+        }
+        ezxml_free(xml);
+    }
+
+    /* Fallback: the old substring heuristic, for responses that ezxml
+     * can't parse (malformed XML from buggy firmware). */
+    if (!event_url) {
+        const char *events_ns = "http://www.onvif.org/ver10/events/wsdl";
+        const char *ns_pos = strstr(response, events_ns);
+        if (ns_pos) {
+            const char *xaddr_open[]  = {"<tds:XAddr>", "<XAddr>", NULL};
+            const char *xaddr_close[] = {"</tds:XAddr>", "</XAddr>", NULL};
+            for (int i = 0; xaddr_open[i] != NULL; i++) {
+                const char *tag = strstr(ns_pos, xaddr_open[i]);
+                if (tag && (tag - ns_pos) < 2048) {
+                    const char *val_start = tag + strlen(xaddr_open[i]);
+                    const char *val_end   = strstr(val_start, xaddr_close[i]);
+                    if (val_end) {
+                        event_url = strndup(val_start, (size_t)(val_end - val_start));
+                        break;
+                    }
+                }
             }
         }
     }
@@ -311,16 +418,18 @@ static onvif_subscription_t *get_subscription(const char *url, const char *usern
 
     if (discovered_url) {
         log_info("ONVIF: Sending CreatePullPointSubscription to discovered URL: %s", discovered_url);
-        response = send_onvif_request_to_url(discovered_url, username, password, request_body);
+        response = send_onvif_request_to_url(discovered_url, username, password, request_body,
+                                              ONVIF_ACTION_CREATE_PULLPOINT);
         free(discovered_url);
     }
 
     if (!response) {
         // Fall back through common event service path suffixes
-        const char *fallback_services[] = {"service", "event_service", NULL};
+        const char *fallback_services[] = {"service", "event_service", "Events", NULL};
         for (int i = 0; fallback_services[i] && !response; i++) {
             log_info("ONVIF: Trying fallback event endpoint: onvif/%s", fallback_services[i]);
-            response = send_onvif_request(url, username, password, request_body, fallback_services[i]);
+            response = send_onvif_request(url, username, password, request_body,
+                                          fallback_services[i], ONVIF_ACTION_CREATE_PULLPOINT);
         }
     }
 
@@ -554,7 +663,8 @@ int detect_motion_onvif(const char *onvif_url, const char *username, const char
         response = send_onvif_request_to_url(subscription->subscription_address,
                                              subscription->username,
                                              subscription->password,
-                                             request_body);
+                                             request_body,
+                                             ONVIF_ACTION_PULL_MESSAGES);
     } else {
         // Legacy fallback: extract last path component and re-append under /onvif/
         log_warn("ONVIF Detection: subscription_address is not a full URL ('%s'), using legacy path extraction",
@@ -565,7 +675,8 @@ int detect_motion_onvif(const char *onvif_url, const char *username, const char
                                          subscription->username,
                                          subscription->password,
                                          request_body,
-                                         service);
+                                         service,
+                                         ONVIF_ACTION_PULL_MESSAGES);
             free(service);
         }
     }

src/video/onvif_soap.c

@@ -238,21 +238,23 @@ void onvif_log_soap_fault(const char *response, size_t response_len, const char
         if (faultstring) reason_str = ezxml_txt(faultstring);
     }
 
+    bool have_reason = (reason_str && reason_str[0] != '\0');
+
     /* Build and log the message */
     if (code_str && code_str[0] != '\0') {
         if (subcode_str && subcode_str[0] != '\0') {
             log_error("[%s] SOAP Fault: Code=%s, Subcode=%s, Reason=%s",
                       ctx,
                       code_str,
                       subcode_str,
-                      (reason_str && reason_str[0] != '\0') ? reason_str : "(none)");
+                      have_reason ? reason_str : "(none)");
         } else {
             log_error("[%s] SOAP Fault: Code=%s, Reason=%s",
                       ctx,
                       code_str,
-                      (reason_str && reason_str[0] != '\0') ? reason_str : "(none)");
+                      have_reason ? reason_str : "(none)");
         }
-    } else if (reason_str && reason_str[0] != '\0') {
+    } else if (have_reason) {
         log_error("[%s] SOAP Fault: %s", ctx, reason_str);
     } else {
         /* Could not extract anything useful — log raw */
@@ -263,6 +265,18 @@ void onvif_log_soap_fault(const char *response, size_t response_len, const char
         log_error("[%s] SOAP Fault (could not extract details): %s", ctx, snippet);
     }
 
+    /* Extra diagnostic: when the structured fault lacks a Reason — as happens
+     * with strict ONVIF servers like Reolink (#374) — dump a snippet of the
+     * raw XML so future bug reports include enough for us to spot vendor
+     * quirks (Subcode values, Detail elements, etc.) without needing a pcap. */
+    if (!have_reason && code_str && code_str[0] != '\0') {
+        char snippet[768];
+        size_t n = response_len < sizeof(snippet) - 1 ? response_len : sizeof(snippet) - 1;
+        memcpy(snippet, response, n);
+        snippet[n] = '\0';
+        log_error("[%s] SOAP Fault raw XML (no Reason/Text): %s", ctx, snippet);
+    }
+
     ezxml_free(xml);
     free(buf);
 }

tests/unit/test_onvif_soap_fault.c

@@ -162,6 +162,21 @@ void test_soap_env_namespace(void) {
     TEST_PASS();
 }
 
+/* ================================================================
+ * Reolink-style fault: Code present but no Reason/Text element (#374)
+ * Exercises the raw-XML-snippet diagnostic branch in onvif_log_soap_fault.
+ * ================================================================ */
+void test_fault_code_no_reason(void) {
+    const char *xml =
+        "<SOAP-ENV:Envelope xmlns:SOAP-ENV=\"http://www.w3.org/2003/05/soap-envelope\">"
+        "<SOAP-ENV:Body><SOAP-ENV:Fault>"
+        "<SOAP-ENV:Code><SOAP-ENV:Value>SOAP-ENV:Sender</SOAP-ENV:Value></SOAP-ENV:Code>"
+        "</SOAP-ENV:Fault></SOAP-ENV:Body></SOAP-ENV:Envelope>";
+
+    onvif_log_soap_fault(xml, strlen(xml), "TestReolink");
+    TEST_PASS();
+}
+
 /* ================================================================
  * Uppercase S namespace prefix
  * ================================================================ */
@@ -196,6 +211,7 @@ int main(void) {
     RUN_TEST(test_null_context);
     RUN_TEST(test_empty_fault);
     RUN_TEST(test_soap_env_namespace);
+    RUN_TEST(test_fault_code_no_reason);
     RUN_TEST(test_uppercase_s_namespace);
 
     return UNITY_END();