Critical
- Renew busy-loop on persistent failure: after a failed
renewPullPoint, the loop re-read the (now in-past)
GrantedTermination and nextRenewInterval floored to 1s, hammering
the camera at 1 Hz until reconnect. nextRenewIntervalAfterError
decouples the failure path from the stale grant and backs off
at opts.RetryBackoff. renewLoop also routes through s.now() so
test clocks can drive it deterministically.
- Lost-update race on GrantedTermination: a renew result for an
old subscription could overwrite the grant on a new one if
attemptRecreate swapped pullPoint mid-flight. A generation
counter on Stream tracks subscription rotation; the renew loop
captures the generation before the SOAP call and discards the
result if the subscription was rotated.
- Credential leak when <Security> straddled the 64 KiB error cap:
the non-greedy regex required a closing tag and missed the
truncated case. wsseSecurityRE now matches close-tag-or-EOF.
Belt-and-braces wssePasswordRE redacts <Password> elements
outside any Security wrapper.
- addHeaderChildren accepted well-formed-but-element-free input
and produced a header-less request. Now errors out.
Important
- Wrapper detection in buildRefParamsHeader was HasSuffix-based and
misfired on children named *ReferenceParameters. Replaced with the
unambiguous wrapper-only contract: input must be the full
<*:ReferenceParameters> element returned by extractReferenceParameters.
- Renamed SoapOption → SendSoapOption and WithHeader → WithSOAPHeader
to disambiguate at call sites.
- Renamed WithHeader's `xml` parameter to headerContent to avoid
shadowing the encoding/xml package name.
- Documented terminationTimeRE's "first match only" semantics so
nobody re-uses it from PullMessages context where multiple
TerminationTime elements appear.
- goleak is now a direct require (go mod tidy).
Performance
- Added gosoap.AddStringHeaderContents (plural) for multi-root
header content. AddStringHeaderContent remains as-is for
backwards compatibility with external consumers. Device.go's
addHeaderChildren workaround is gone — one etree parse per
SendSoapWithOptions call instead of two.
Migration
- Device.go's own CallOnvifFunction and the three examples now
call SendSoapWithOptions, modelling the canonical path.
Docs / tests
- Trust-boundary warning on SendSoapWithHeader/Options godoc.
- Comments on createPullPointResp/Alt explain why TerminationTime
is intentionally omitted (renew timing fixtures).
- New tests: digest retry strips WS-Security, duplicate
WithSOAPHeader is last-wins, malformed TerminationTime yields
zero, margin==base falls to base/2, empty SubscriptionReference
yields empty ref params, Security straddling cap is redacted,
bare Password redacted, wrapper-only contract is enforced.
Critical
- Device.SendSoapWithHeader now parses the supplied header content
with etree and adds each top-level child as its own SOAP Header
block. gosoap.AddStringHeaderContent only accepts a single root
element; previously a multi-child ref-params header silently
produced a header-less request because the parse error was
discarded. Errors are now propagated.
- enrichSOAPErr scrubs <*:Security> blocks from response bodies
before fault extraction or excerpt slicing so a camera that
echoes the WS-Security header in a fault response cannot leak
Username/Password into operator logs.
Important
- extractSOAPFault falls back to the SOAP 1.2 Subcode (e.g.
ter:InvalidArgs) when Reason/Text is empty — consistent with
enrichSOAPErr and surfaces actionable detail on 200-OK fault
bodies reached via unmarshalNode.
- subscriptionRef captures the camera-granted TerminationTime
from CreatePullPointSubscription and Renew responses. renewLoop
schedules from it via the new nextRenewInterval helper so we
never miss a renew when the camera grants less than requested.
renew is now a sleep-loop driven by the latest granted time.
- enrichSOAPErr reads at most 64 KiB from the body (vs. 10 MiB
on success paths). Fault bodies are always small; the prior cap
let a wedged camera churn 10 MiB/s through the retry loop.
Suggestions
- extractReferenceParameters anchors to <SubscriptionReference> so
a wsa:ReplyTo / wsa:FaultTo that also carries ReferenceParameters
elsewhere in the envelope cannot leak through and break PullMessages.
- buildRefParamsHeader accepts either raw children or the full
<*:ReferenceParameters> wrapper, and propagates ancestor xmlns:*
onto each child so a vendor that declares the prefix on the
parent (not the child itself, as AXIS does) still produces valid
standalone children on the wire.
- SendSoapWithHeader documents that xmlHeaderContent must be
well-formed XML and that the caller is responsible for escaping
any externally sourced data.
- Error-message ordering is now context-first
("SOAP fault: X: <wrapped err>") per Go convention.
- Dead headerEnd slicing removed from the SendSoapWithHeader test.
Tests
- End-to-end multi-child wiring through pullMessages.
- Digest auth retry preserves the injected header.
- Malformed-XML header content fast-fails before any request.
- buildRefParamsHeader malformed / whitespace-only edge cases.
- goleak.VerifyTestMain in event/stream catches any pull/renew
goroutine that outlives its Stream.
No new behavioural surface added to onvif core; SendSoap retains
its signature, SendSoapWithHeader is the only new public method.