Conversation
A Hikvision dome failed every probe with "GetCapabilities: empty SOAP body" — HTTP 200, zero bytes, no fault to explain itself. Three separate causes, each of which produces that same unhelpful result. The client carried no credentials on its handler, so a 401 challenge was never answered. All it sent was a preemptive Basic header, and Hikvision wants Digest for ONVIF; the request was simply unauthorized and the camera declined to say so. Each (host, user) now gets an HttpClient whose handler holds the credentials, which is what lets HttpClient satisfy Basic or Digest as the camera asks. PreAuthenticate stays off so the camera states its terms first, and the preemptive Basic header stays for onvif_simple_server, which enforces Basic at the transport and never challenges. Every request went out as SOAP 1.2 only. Several firmwares are built for 1.1 and answer 1.2 with nothing at all. A response with no usable envelope is now retried once as SOAP 1.1 — different content type, action moved into its own SOAPAction header. A fault counts as an answer, so a camera that says why it refused is not asked twice. And GetStreamUri read Uri as a direct child of the response, which only matches the flatter shape onvif_simple_server sends. The spec nests it as MediaUri/Uri, so a compliant camera looked like it had no stream at all; SetPreset's token is nested the same way on some firmwares. Both now search the response instead of assuming its depth. When both versions come back empty the error names what to check — ONVIF switched off on the camera, or an account without ONVIF rights, which is what an empty body from a working camera almost always means — and the response is logged at debug level. Covered by a stub camera over a real socket, so the client's own HTTP stack does the work: the 401 handshake, the content types and the SOAPAction header are all exercised rather than mocked.
PR Summary by QodoFix ONVIF Digest auth, SOAP 1.1 fallback, and nested values
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
…-sent, authed clients recycled Two holes the review caught, both real. The SOAP 1.1 fallback retried every call whose response was unusable, including SetPreset and RemovePreset. An unusable response does not prove the request was not executed — a camera that ran SetPreset and answered garbage would get a duplicate preset from the resend, and a resend after a successful but unreadable remove would fault on the now-missing preset and report failure for a removal that worked. The dialect a host speaks is now learned once and remembered: the first time a host answers 1.2 with nothing usable and 1.1 with something, the flip is cached and later calls lead with 1.1. Since every authed operation is preceded by the unauthenticated clock probe on first contact, the dialect is already known by the time any mutation goes out. Mutations never cross-dialect retry — they use what the host's reads taught and fail honestly otherwise. The clock-skew retry stays, for mutations too: a fault means the camera refused the request, not that it ran it. And the per-credential HttpClient cache was keyed by host, user and password together with no eviction: every password a camera has ever had kept a live handler — old secret included — for the rest of the process, and two threads missing the cache at once could each construct a client only one of which was ever stored. Keyed by host:port now, since a camera has one credential at a time; a lookup that finds a different credential swaps the entry and disposes the superseded client (a request in flight on it was sent with the old password and failing anyway), and a plain lock replaces GetOrAdd so the losing constructor of a concurrent miss never exists. Growth is bounded by the camera addresses spoken to. Two tests pin the retry behaviour: the dialect is remembered (exactly one 1.2 request ever reaches a 1.1-only host), and a SetPreset whose response is empty is reported as failed after exactly one attempt.
|
Good PR. Three causes of one symptom are separated cleanly, and each is covered by a test 1. A
|
# Conflicts: # src/OpenIPC.Viewer.Devices/Onvif/SoapOnvifClient.cs
- Retry a VersionMismatch fault as SOAP 1.1; name a 401/403 as a bad login - Key the learned SOAP dialect by host:port, like the client cache - Candidate probes never flip the dialect; MoveStatus="1" reads as true - Prefer the FOV relative space when a camera declares both
|
Thanks a lot for this — the work is merged, with your commits and authorship kept, into #68 . The review items are fixed on top there, so I'm closing this one in favour of it. |
2 participants
Summary
Three separate causes of one unhelpful failure, found bringing up a Hikvision
dome: every probe died with
GetCapabilities: empty SOAP body— HTTP 200, zerobytes, no fault to explain itself.
A digest challenge is never answered. The client sent a preemptive
Basicheader, but the handler carried no credentials, so a401wentunanswered. Hikvision wants Digest for ONVIF, so the request was simply
unauthorized and the camera declined to say so. Each
(host, user)now getsan
HttpClientwhose handler holds the credentials, which is what letsHttpClientsatisfy Basic or Digest as the camera asks.PreAuthenticatestays off so the camera states its terms first, and the preemptive Basic
header stays for
onvif_simple_server, which enforces Basic at the transportand never challenges.
Requests went out as SOAP 1.2 only. Several firmwares are built for 1.1
and answer 1.2 with nothing. A response with no usable envelope is retried
once as SOAP 1.1 —
text/xml, action moved into its ownSOAPActionheader.A fault counts as an answer, so a camera that says why it refused is not
asked twice.
Values were read at the wrong depth.
GetStreamUrireadUrias adirect child of the response, which matches only the flatter shape
onvif_simple_serversends. The spec nests it asMediaUri/Uri, so acompliant camera looked like it had no stream at all.
SetPreset's token isnested the same way on some firmwares.
When both versions come back empty, the error now names what to check on the
camera — ONVIF switched off, or an account without ONVIF rights — instead of
saying "empty SOAP body", and the response is logged at debug level.
Related
No issue. Fixes the SOAP client introduced in #45; Phase 4 — ONVIF + PTZ.
Type
Checklist
TreatWarningsAsErrors=true). Desktopsolution and the Android head both clean; the iOS head was not built here
(workload not installed on this machine) — left to CI.
dotnet test); new Core logic has unit tests. Six new testsin
OpenIPC.Viewer.Devices.Tests; suites green at Core 313 / Devices 11 /Video 11.
Devicesand its test projectonly.
none did; no new commands, options or endpoints.
Platforms tested
Built and tested on macOS. No head was actually run: the fix lives below the
UI and is exercised by the stub camera described below.
Screenshots / notes
No UI change.
How it is covered. A stub camera on a real socket, so the client's own HTTP
stack does the work — the 401 handshake, the content types and the
SOAPActionheader are exercised rather than mocked:
MediaUri/UriWhat is not covered. There is no Hikvision in CI. Each failure mode is
reproduced in the stub as observed on the device, but the end-to-end fix is
confirmed against real hardware only by hand. Worth a second pair of eyes from
anyone with a non-OpenIPC camera to hand.