Merge pull request #14 from EpicMouse123/feature/endpoints-from-getservices

fix: reach DeviceIO services and unsubscribe with the correct namespace

Device.go

@@ -28,6 +28,7 @@ var Xlmns = map[string]string{
 	"trt":     "http://www.onvif.org/ver10/media/wsdl",
 	"tr2":     "http://www.onvif.org/ver20/media/wsdl",
 	"tev":     "http://www.onvif.org/ver10/events/wsdl",
+	"tmd":     "http://www.onvif.org/ver10/deviceIO/wsdl",
 	"tptz":    "http://www.onvif.org/ver20/ptz/wsdl",
 	"timg":    "http://www.onvif.org/ver20/imaging/wsdl",
 	"tan":     "http://www.onvif.org/ver20/analytics/wsdl",
@@ -189,9 +190,71 @@ func NewDevice(params DeviceParams) (*Device, error) {
 	}
 
 	dev.getSupportedServices(resp)
+	dev.addServicesFromGetServices()
 	return dev, nil
 }
 
+// addServicesFromGetServices registers the endpoints a device lists through
+// GetServices but not through GetCapabilities. Some devices only advertise
+// services such as DeviceIO this way, which would otherwise leave calls like
+// deviceio.GetDigitalInputs without an endpoint. Endpoints already known from
+// GetCapabilities are kept, and failures are ignored because GetServices is
+// optional for older devices.
+func (dev *Device) addServicesFromGetServices() {
+	resp, err := dev.CallMethod(device.GetServices{IncludeCapability: false})
+	if err != nil || resp == nil {
+		if resp != nil {
+			resp.Body.Close()
+		}
+		return
+	}
+	defer resp.Body.Close()
+
+	data, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return
+	}
+	dev.addServicesFromResponse(data)
+}
+
+func (dev *Device) addServicesFromResponse(data []byte) {
+	doc := etree.NewDocument()
+	if err := doc.ReadFromBytes(data); err != nil {
+		return
+	}
+	for _, service := range doc.FindElements("./Envelope/Body/GetServicesResponse/Service") {
+		namespace := service.SelectElement("Namespace")
+		xaddr := service.SelectElement("XAddr")
+		if namespace == nil || xaddr == nil {
+			continue
+		}
+		key := endpointKeyFromNamespace(namespace.Text())
+		if key == "" || strings.TrimSpace(xaddr.Text()) == "" {
+			continue
+		}
+		if _, exists := dev.endpoints[key]; exists {
+			continue
+		}
+		dev.addEndpoint(key, strings.TrimSpace(xaddr.Text()))
+	}
+}
+
+// endpointKeyFromNamespace maps a GetServices namespace such as
+// "http://www.onvif.org/ver10/deviceIO/wsdl" onto the endpoint key used by
+// CallMethod, which is the lowercased name of the request's package
+// ("deviceio"). The ver20 media service maps to "media2".
+func endpointKeyFromNamespace(namespace string) string {
+	parts := strings.Split(strings.Trim(strings.TrimSpace(namespace), "/"), "/")
+	if len(parts) < 3 || !strings.EqualFold(parts[len(parts)-1], "wsdl") {
+		return ""
+	}
+	key := strings.ToLower(parts[len(parts)-2])
+	if key == "media" && strings.EqualFold(parts[len(parts)-3], "ver20") {
+		return strings.ToLower(Media2WebService)
+	}
+	return key
+}
+
 func (dev *Device) addEndpoint(Key, Value string) {
 	//use lowCaseKey
 	//make key having ability to handle Mixed Case for Different vendor devcie (e.g. Events EVENTS, events)
@@ -292,11 +355,12 @@ func (dev Device) callMethodDo(endpoint string, method interface{}) (*http.Respo
 //     back to HTTP digest, which keeps cameras that authenticate exclusively
 //     through WS-Security working.
 //   - DigestAuth ("digest"):      HTTP digest only; no WS-Security header.
-//   - Both ("both") / unset (""): WS-Security credentials are added (when
-//     available) and HTTP digest is attempted only if the device answers with
-//     an authentication challenge (HTTP 401 Unauthorized). On that digest
-//     retry the WS-Security header is dropped so the credentials are not sent
-//     twice.
+//   - Both ("both") / unset (""): the request is first sent with WS-Security
+//     credentials (when available). If the device rejects it with an HTTP
+//     error (e.g. a 400/500 NotAuthorized SOAP fault from a WS-Security-only
+//     camera, or a 401 from a digest-only camera), it is retried through
+//     HTTP digest with the WS-Security header dropped so the credentials are
+//     not sent twice. This mirrors SendSoapWithOptions.
 func (dev Device) sendSOAP(endpoint string, soap gosoap.SoapMessage) (*http.Response, error) {
 	hasCredentials := dev.params.Username != "" || dev.params.Password != ""
 
@@ -314,8 +378,17 @@ func (dev Device) sendSOAP(endpoint string, soap gosoap.SoapMessage) (*http.Resp
 		return networking.SendSoapWithDigest(dev.params.HttpClient, endpoint, soap.String(), dev.params.Username, dev.params.Password)
 
 	default: // Both and the empty/unset default.
-		if hasCredentials {
-			soap.AddWSSecurity(dev.params.Username, dev.params.Password)
+		if !hasCredentials {
+			return networking.SendSoap(dev.params.HttpClient, endpoint, soap.String())
+		}
+		soap.AddWSSecurity(dev.params.Username, dev.params.Password)
+		resp, err := networking.SendSoap(dev.params.HttpClient, endpoint, soap.String())
+		if err == nil {
+			return resp, nil
+		}
+		// Close the rejected response so the connection can be reused.
+		if resp != nil {
+			resp.Body.Close()
 		}
 		return networking.SendSoapWithDigest(dev.params.HttpClient, endpoint, soap.String(), dev.params.Username, dev.params.Password)
 	}

Device_test.go

@@ -7,6 +7,9 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/kerberos-io/onvif/deviceio"
+	"github.com/kerberos-io/onvif/gosoap"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -266,3 +269,135 @@ func TestDevice_SendSoapWithOptions_DuplicateWithSOAPHeaderLastWins(t *testing.T
 	assert.NotContains(t, captured, "First", "first WithSOAPHeader must be overwritten")
 	assert.Contains(t, captured, "Second")
 }
+
+func newBothAuthTestSOAP() gosoap.SoapMessage {
+	soap := gosoap.NewEmptySOAP()
+	soap.AddStringBodyContent(`<tds:GetDeviceInformation xmlns:tds="http://www.onvif.org/ver10/device/wsdl"/>`)
+	soap.AddRootNamespaces(Xlmns)
+	return soap
+}
+
+// Regression: in "both" mode a WS-Security-only camera answers an
+// unauthenticated request with a NotAuthorized SOAP fault (HTTP 400), not a
+// 401 digest challenge. The first attempt must therefore carry the
+// UsernameToken, otherwise the credentials never reach the camera.
+func TestDevice_SendSOAP_BothSendsWSSecurityFirst(t *testing.T) {
+	var hits int
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		hits++
+		b, _ := io.ReadAll(r.Body)
+		if !strings.Contains(string(b), "UsernameToken") {
+			w.WriteHeader(http.StatusBadRequest)
+			_, _ = w.Write([]byte(`<s:Envelope><s:Body><s:Fault><s:Reason><s:Text>Sender not authorized</s:Text></s:Reason></s:Fault></s:Body></s:Envelope>`))
+			return
+		}
+		w.WriteHeader(http.StatusOK)
+	}))
+	t.Cleanup(srv.Close)
+
+	dev := Device{params: DeviceParams{
+		HttpClient: srv.Client(),
+		Username:   "admin",
+		Password:   "secret",
+		AuthMode:   Both,
+	}}
+	resp, err := dev.sendSOAP(srv.URL, newBothAuthTestSOAP())
+	require.NoError(t, err)
+	resp.Body.Close()
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+	assert.Equal(t, 1, hits, "WS-Security-only camera should succeed on the first request")
+}
+
+// "both" mode must still fall back to HTTP digest (without the WS-Security
+// header) for cameras that only accept digest.
+func TestDevice_SendSOAP_BothFallsBackToDigest(t *testing.T) {
+	var authedBody string
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get("Authorization") == "" {
+			w.Header().Set("WWW-Authenticate", `Digest realm="onvif", nonce="abc", qop="auth"`)
+			w.WriteHeader(http.StatusUnauthorized)
+			return
+		}
+		b, _ := io.ReadAll(r.Body)
+		authedBody = string(b)
+		w.WriteHeader(http.StatusOK)
+	}))
+	t.Cleanup(srv.Close)
+
+	dev := Device{params: DeviceParams{
+		HttpClient: srv.Client(),
+		Username:   "admin",
+		Password:   "secret",
+		AuthMode:   Both,
+	}}
+	resp, err := dev.sendSOAP(srv.URL, newBothAuthTestSOAP())
+	require.NoError(t, err)
+	resp.Body.Close()
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+	require.NotEmpty(t, authedBody, "expected an authenticated digest POST")
+	assert.NotContains(t, authedBody, "UsernameToken")
+}
+
+func TestEndpointKeyFromNamespace(t *testing.T) {
+	cases := map[string]string{
+		"http://www.onvif.org/ver10/device/wsdl":    "device",
+		"http://www.onvif.org/ver10/deviceIO/wsdl":  "deviceio",
+		"http://www.onvif.org/ver10/deviceio/wsdl":  "deviceio",
+		"http://www.onvif.org/ver10/events/wsdl":    "events",
+		"http://www.onvif.org/ver10/media/wsdl":     "media",
+		"http://www.onvif.org/ver20/media/wsdl":     "media2",
+		"http://www.onvif.org/ver20/ptz/wsdl":       "ptz",
+		"http://www.onvif.org/ver20/analytics/wsdl": "analytics",
+		"http://www.onvif.org/ver10/schema":         "",
+		"":                                          "",
+	}
+	for namespace, want := range cases {
+		assert.Equal(t, want, endpointKeyFromNamespace(namespace), namespace)
+	}
+}
+
+// Some devices list DeviceIO only through GetServices. Those endpoints must be
+// registered (with the host rewritten to Xaddr) without overriding endpoints
+// already learned from GetCapabilities.
+func TestDevice_AddServicesFromResponse(t *testing.T) {
+	dev := Device{
+		params:    DeviceParams{Xaddr: "127.0.0.1:8080"},
+		endpoints: map[string]string{"events": "http://127.0.0.1:8080/onvif/events_from_capabilities"},
+	}
+	response := `<?xml version="1.0" encoding="UTF-8"?>
+<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://www.w3.org/2003/05/soap-envelope" xmlns:tds="http://www.onvif.org/ver10/device/wsdl">
+<SOAP-ENV:Body><tds:GetServicesResponse>
+<tds:Service><tds:Namespace>http://www.onvif.org/ver10/events/wsdl</tds:Namespace><tds:XAddr>http://10.0.0.5:80/onvif/event_service</tds:XAddr></tds:Service>
+<tds:Service><tds:Namespace>http://www.onvif.org/ver10/deviceio/wsdl</tds:Namespace><tds:XAddr>http://10.0.0.5:80/onvif/deviceio_service</tds:XAddr></tds:Service>
+</tds:GetServicesResponse></SOAP-ENV:Body></SOAP-ENV:Envelope>`
+
+	dev.addServicesFromResponse([]byte(response))
+
+	endpoint, err := dev.getEndpoint("deviceio")
+	require.NoError(t, err)
+	assert.Equal(t, "http://127.0.0.1:8080/onvif/deviceio_service", endpoint)
+	assert.Equal(t, "http://127.0.0.1:8080/onvif/events_from_capabilities", dev.endpoints["events"],
+		"GetCapabilities endpoints must not be overridden")
+}
+
+// DeviceIO requests use the tmd prefix, which must be declared on the
+// envelope or the request is not well-formed XML.
+func TestDevice_CallMethod_DeclaresDeviceIONamespace(t *testing.T) {
+	var captured string
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		b, _ := io.ReadAll(r.Body)
+		captured = string(b)
+		w.WriteHeader(http.StatusOK)
+	}))
+	t.Cleanup(srv.Close)
+
+	dev := Device{
+		params:    DeviceParams{HttpClient: srv.Client()},
+		endpoints: map[string]string{"deviceio": srv.URL + "/onvif/deviceio_service"},
+	}
+	resp, err := dev.CallMethod(deviceio.GetDigitalInputs{})
+	require.NoError(t, err)
+	resp.Body.Close()
+	assert.Contains(t, captured, "<tmd:GetDigitalInputs")
+	assert.Contains(t, captured, `xmlns:tmd="http://www.onvif.org/ver10/deviceIO/wsdl"`)
+}

event/stream/doc.go

@@ -18,7 +18,8 @@
 //	}
 //
 // NewStream performs network I/O so auth and reachability failures
-// surface synchronously. Events and Errors close when the Stream stops;
+// surface synchronously, and rejects a client timeout that cannot
+// outlast PullTimeout with ErrInvalidOptions. Events and Errors close when the Stream stops;
 // Errors sends are non-blocking so a stalled consumer drops older
 // errors rather than blocking the pull loop. After a silent reconnect,
 // the next batch's events carry Event.AfterReconnect=true.

event/stream/pulltimeout.go

@@ -0,0 +1,51 @@
+package stream
+
+import (
+	"errors"
+	"fmt"
+	"time"
+
+	"github.com/kerberos-io/onvif"
+)
+
+// ErrInvalidOptions marks a configuration that cannot succeed. Callers
+// retry the pull/renew/recreate errors; retrying this one never helps,
+// so it is a distinct sentinel they can short-circuit on.
+var ErrInvalidOptions = errors.New("stream: invalid options")
+
+// minClientHeadroom is how far http.Client.Timeout must exceed
+// PullTimeout. The client ceiling covers dial, TLS and the response
+// transfer on top of the poll it has to outlast, and starts before the
+// camera has parsed the request; on a cellular bearer that overhead
+// runs to hundreds of milliseconds.
+const minClientHeadroom = 5 * time.Second
+
+// validateClientTimeout rejects a client ceiling that cannot outlast
+// the PullMessages long-poll plus minClientHeadroom.
+//
+// Zero means unbounded and is accepted: it is the SDK's default when a
+// caller passes no client, so rejecting it would break every default
+// consumer. Note it is not risk-free — the caller interface documents
+// that ctx cannot interrupt an in-flight SOAP call, so only the client
+// timeout can unwedge a stalled camera.
+func validateClientTimeout(clientTimeout, pullTimeout time.Duration) error {
+	if clientTimeout == 0 || clientTimeout >= pullTimeout+minClientHeadroom {
+		return nil
+	}
+	return fmt.Errorf(
+		"%w: http.Client.Timeout (%s) must exceed PullTimeout (%s) by at least %s; PullMessages is a long-poll and the client would abort every quiet pull",
+		ErrInvalidOptions, clientTimeout, pullTimeout, minClientHeadroom)
+}
+
+// clientTimeoutOf reports the device's HTTP client ceiling, or 0 when
+// the SDK is using its own default (unbounded) client.
+func clientTimeoutOf(dev *onvif.Device) time.Duration {
+	if dev == nil {
+		return 0
+	}
+	c := dev.GetDeviceParams().HttpClient
+	if c == nil {
+		return 0
+	}
+	return c.Timeout
+}

event/stream/pulltimeout_test.go

@@ -0,0 +1,68 @@
+package stream
+
+import (
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestValidateClientTimeout — PullMessages is a long-poll: the camera
+// holds the connection open for PullTimeout waiting for an event. An
+// http.Client.Timeout covers the whole exchange and starts before the
+// camera has parsed the request, so a client ceiling at or below
+// PullTimeout loses the race on every quiet interval and the pull can
+// only ever fail. This shipped once (both were 5s) and presented as a
+// slow camera rather than a misconfiguration.
+//
+// Strict inequality is not enough: the client also has to cover dial,
+// TLS and the response transfer, which on a cellular bearer runs to
+// hundreds of milliseconds. Hence a real headroom floor.
+func TestValidateClientTimeout(t *testing.T) {
+	tests := []struct {
+		name    string
+		client  time.Duration
+		pull    time.Duration
+		wantErr bool
+	}{
+		{"unbounded client is the caller's risk, not an error", 0, 30 * time.Second, false},
+		{"comfortable headroom", 40 * time.Second, 30 * time.Second, false},
+		{"exactly the minimum headroom", 30*time.Second + minClientHeadroom, 30 * time.Second, false},
+		{"a hair under the minimum headroom", 30*time.Second + minClientHeadroom - time.Millisecond, 30 * time.Second, true},
+		{"strictly greater but no headroom", 30*time.Second + time.Millisecond, 30 * time.Second, true},
+		{"equal timeouts always lose", 5 * time.Second, 5 * time.Second, true},
+		{"client below pull", 4 * time.Second, 30 * time.Second, true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateClientTimeout(tt.client, tt.pull)
+			if tt.wantErr {
+				require.Error(t, err, "client=%s pull=%s must be rejected", tt.client, tt.pull)
+				assert.ErrorIs(t, err, ErrInvalidOptions,
+					"callers need a sentinel to tell a permanent misconfiguration from a transient failure")
+				assert.Contains(t, err.Error(), "PullTimeout",
+					"the error must name the option the caller has to change")
+				return
+			}
+			assert.NoError(t, err, "client=%s pull=%s must be accepted", tt.client, tt.pull)
+		})
+	}
+}
+
+// TestErrInvalidOptions_IsDistinctFromStreamErrors — the pull/renew/
+// recreate errors are transient and callers retry them. A bad Options
+// never becomes valid by retrying, so it must not be mistaken for one.
+func TestErrInvalidOptions_IsDistinctFromStreamErrors(t *testing.T) {
+	err := validateClientTimeout(5*time.Second, 5*time.Second)
+	require.Error(t, err)
+
+	var pull ErrPullFailed
+	var renew ErrRenewFailed
+	var recreate ErrRecreateFailed
+	assert.False(t, errors.As(err, &pull))
+	assert.False(t, errors.As(err, &renew))
+	assert.False(t, errors.As(err, &recreate))
+}

event/stream/stream.go

@@ -39,7 +39,9 @@ type Options struct {
 	// server-side filtering is fragile across vendors and empty is
 	// required for AXIS.
 	RawTopicFilter string
-	// PullTimeout — zero means default (5s).
+	// PullTimeout — zero means default (5s). The device's
+	// http.Client.Timeout must exceed this by minClientHeadroom or
+	// NewStream returns ErrInvalidOptions.
 	PullTimeout time.Duration
 	// MessageLimit — zero means default (32). Busy AXIS cameras with
 	// many configured rules can burst beyond 10 per pull.
@@ -234,6 +236,12 @@ func (s *Stream) updateGrantedTerminationIfGen(gen uint64, t time.Time) {
 //
 // The returned Stream stops when ctx is cancelled or Close is called.
 func NewStream(ctx context.Context, dev *onvif.Device, opts Options) (*Stream, error) {
+	// Checked before the subscription call: this config can only fail,
+	// so surfacing it here beats a stream that appears to work and
+	// silently survives on reconnects alone.
+	if err := validateClientTimeout(clientTimeoutOf(dev), opts.withDefaults().PullTimeout); err != nil {
+		return nil, err
+	}
 	return newStream(ctx, deviceCaller{dev: dev}, opts)
 }
 

event/stream/topics.go

@@ -79,10 +79,19 @@ var topicRules = []struct {
 	// https://developer.axis.com/vapix/network-video/event-and-action-services/
 	{"MotionRegionDetector/Motion", KindMotion},
 
-	// AXIS Guard suite — vendor analytics apps with Camera<N>Profile<ID>
-	// suffixes. Treated as motion so they can drive motion-triggered
-	// recording on cameras using these apps instead of basic VMD.
+	// tns1:RuleEngine/tnsaxis:VMD3/vmd3_video_<N> — AXIS VMD 3, the
+	// firmware-builtin predecessor of the VMD 4 ACAP. It lives under
+	// RuleEngine, not CameraApplicationPlatform, so the VMD rule below
+	// does not cover it. Still shipping on deployed cameras.
+	// https://developer.axis.com/vapix/network-video/event-and-action-services/
+	{"RuleEngine/VMD3/", KindMotion},
+
+	// AXIS ACAP motion apps with Camera<N>Profile<ID> suffixes. VMD 4 is
+	// the stock app shipped on the camera; the Guard suite are the
+	// paid analytics products. Prefix-match because of the suffix.
+	// https://developer.axis.com/vapix/applications/vmd4
 	// https://developer.axis.com/vapix/applications/motion-guard
+	{"CameraApplicationPlatform/VMD/", KindMotion},
 	{"CameraApplicationPlatform/MotionGuard/", KindMotion},
 	{"CameraApplicationPlatform/FenceGuard/", KindMotion},
 	{"CameraApplicationPlatform/LoiteringGuard/", KindMotion},

event/stream/topics_test.go

@@ -25,6 +25,18 @@ func TestClassifyTopic(t *testing.T) {
 		{"axis_fence_guard", "tnsaxis:CameraApplicationPlatform/FenceGuard/Camera1ProfileANY", KindMotion},
 		{"axis_loitering_guard", "tnsaxis:CameraApplicationPlatform/LoiteringGuard/Camera1ProfileANY", KindMotion},
 
+		// AXIS VMD 4 — the stock motion app, and the one an installer
+		// reaches for before any Guard product. The profile suffix
+		// varies with the configured VMD profile.
+		{"axis_vmd4_profile_any", "tnsaxis:CameraApplicationPlatform/VMD/Camera1ProfileANY", KindMotion},
+		{"axis_vmd4_profile_numbered", "tnsaxis:CameraApplicationPlatform/VMD/Camera1Profile1", KindMotion},
+
+		// AXIS VMD 3 — the firmware-builtin predecessor, published under
+		// RuleEngine rather than CameraApplicationPlatform. Still
+		// shipping on deployed cameras.
+		{"axis_vmd3_video_1", "tns1:RuleEngine/tnsaxis:VMD3/vmd3_video_1", KindMotion},
+		{"axis_vmd3_video_2", "tns1:RuleEngine/tnsaxis:VMD3/vmd3_video_2", KindMotion},
+
 		// --- Tampering --------------------------------------------------
 
 		{"tamper_detector", "tns1:RuleEngine/TamperDetector/Tamper", KindTampering},
@@ -88,6 +100,16 @@ func TestClassifyTopic(t *testing.T) {
 		{"relay_failure_not_digital_output", "tns1:Device/HardwareFailure/RelayFailure", KindUnknown},
 		{"digital_input_config_not_digital_input", "tns1:Device/IO/DigitalInputConfiguration", KindUnknown},
 		{"tamper_detector_log_not_tampering", "tns1:Device/Diagnostics/TamperDetectorLog", KindUnknown},
+
+		// The two AXIS VMD needles carry a trailing slash so they match a
+		// whole path segment. Without it, any sibling app or rule whose
+		// name merely starts with VMD / VMD3 would classify as motion and
+		// drive recording.
+		{"vmd_statistics_app_not_motion", "tnsaxis:CameraApplicationPlatform/VMDStatistics/Camera1", KindUnknown},
+		{"vmd3_config_rule_not_motion", "tns1:RuleEngine/tnsaxis:VMD3Config/Changed", KindUnknown},
+		// VMD3 is scoped to RuleEngine; the same name under another
+		// container is a different thing.
+		{"vmd3_outside_rule_engine_not_motion", "tnsaxis:Storage/VMD3/Status", KindUnknown},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {

event/type_test.go

@@ -90,3 +90,16 @@ func TestRenewFunction_Response(t *testing.T) {
 	assert.Equal(t, xsd.String("2023-11-25T18:08:15Z"), *res.TerminationTime)
 	assert.Equal(t, xsd.String("2023-11-24T14:50:25Z"), *res.CurrentTime)
 }
+
+// Unsubscribe belongs to the WS-BaseNotification namespace (wsnt), not the
+// ONVIF events namespace. Devices reject tev:Unsubscribe with
+// ActionNotSupported.
+func TestUnsubscribeMarshalsAsWSBaseNotification(t *testing.T) {
+	body, err := xml.Marshal(Unsubscribe{})
+	if err != nil {
+		t.Fatalf("xml.Marshal(Unsubscribe{}) error = %v", err)
+	}
+	if got, want := string(body), "<wsnt:Unsubscribe></wsnt:Unsubscribe>"; got != want {
+		t.Fatalf("xml.Marshal(Unsubscribe{}) = %s, want %s", got, want)
+	}
+}

event/types.go

@@ -278,8 +278,8 @@ type RenewResponse struct { //http://docs.oasis-open.org/wsn/b-2.xsd
 
 // Unsubscribe action for Unsubscribe event topic
 type Unsubscribe struct { //http://docs.oasis-open.org/wsn/b-2.xsd
-	XMLName string `xml:"tev:Unsubscribe"`
-	Any     string
+	XMLName string `xml:"wsnt:Unsubscribe"`
+	Any     string `xml:",innerxml"`
 }
 
 // UnsubscribeResponse message for Unsubscribe event topic

networking/digest_test.go

@@ -1,10 +1,57 @@
 package networking
 
 import (
+	"net/http"
+	"net/http/httptest"
 	"strings"
 	"testing"
 )
 
+func TestSendSoapWithDigestReturnsServerError(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, _ *http.Request) {
+		writer.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer server.Close()
+
+	response, err := SendSoapWithDigest(server.Client(), server.URL, "<Envelope/>", "user", "password")
+	if response == nil {
+		t.Fatal("SendSoapWithDigest returned a nil response")
+	}
+	defer response.Body.Close()
+	if err == nil {
+		t.Fatal("SendSoapWithDigest returned nil error for HTTP 500")
+	}
+}
+
+func TestSendSoapWithDigestReturnsServerErrorAfterAuthentication(t *testing.T) {
+	requestCount := 0
+	server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+		requestCount++
+		if requestCount == 1 {
+			writer.Header().Set("WWW-Authenticate", `Digest realm="AXIS", nonce="nonce", qop="auth", algorithm=MD5`)
+			writer.WriteHeader(http.StatusUnauthorized)
+			return
+		}
+		if !strings.HasPrefix(request.Header.Get("Authorization"), "Digest ") {
+			t.Error("authenticated retry is missing Digest Authorization header")
+		}
+		writer.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer server.Close()
+
+	response, err := SendSoapWithDigest(server.Client(), server.URL, "<Envelope/>", "user", "password")
+	if response == nil {
+		t.Fatal("SendSoapWithDigest returned a nil response")
+	}
+	defer response.Body.Close()
+	if err == nil {
+		t.Fatal("SendSoapWithDigest returned nil error for authenticated HTTP 500")
+	}
+	if requestCount != 2 {
+		t.Fatalf("request count = %d, want 2", requestCount)
+	}
+}
+
 func TestParseDigestChallenge(t *testing.T) {
 	challenge := `Digest realm="testrealm@host.com", qop="auth,auth-int", nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093", opaque="5ccc069c403ebaf9f0171e9517f40e41", algorithm=MD5`
 	parts := parseDigestChallenge(challenge)

networking/networking.go

@@ -24,12 +24,14 @@ func SendSoap(httpClient *http.Client, endpoint, message string) (*http.Response
 		return resp, errors.Annotate(err, "Post")
 	}
 
-	// if resp.StatusCode is 4xx,5xx, return error
-	if resp.StatusCode >= 400 && resp.StatusCode < 600 {
-		return resp, errors.Errorf("Server error: %d: %s", resp.StatusCode, resp.Status)
-	}
+	return resp, responseError(resp)
+}
 
-	return resp, nil
+func responseError(resp *http.Response) error {
+	if resp.StatusCode >= 400 && resp.StatusCode < 600 {
+		return errors.Errorf("Server error: %d: %s", resp.StatusCode, resp.Status)
+	}
+	return nil
 }
 
 // SendSoapWithDigest sends a soap message and, when the device answers with an
@@ -57,13 +59,13 @@ func SendSoapWithDigest(httpClient *http.Client, endpoint, message, username, pa
 
 	// Only escalate to HTTP digest when the device explicitly asks for it.
 	if resp.StatusCode != http.StatusUnauthorized {
-		return resp, nil
+		return resp, responseError(resp)
 	}
 
 	challenge := resp.Header.Get("WWW-Authenticate")
 	if !strings.HasPrefix(strings.ToLower(strings.TrimSpace(challenge)), "digest") {
 		// Not a digest challenge (e.g. Basic) - nothing more we can do here.
-		return resp, nil
+		return resp, responseError(resp)
 	}
 
 	authorization := newDigestAuthorization(challenge, http.MethodPost, endpoint, username, password)
@@ -86,7 +88,7 @@ func SendSoapWithDigest(httpClient *http.Client, endpoint, message, username, pa
 		return resp, errors.Annotate(err, "Post with digest")
 	}
 
-	return resp, nil
+	return resp, responseError(resp)
 }
 
 // stripWSSecurityHeader removes the wsse:Security header block from a SOAP

ptz/types_test.go

@@ -0,0 +1,25 @@
+package ptz
+
+import (
+	"encoding/xml"
+	"strings"
+	"testing"
+
+	"github.com/kerberos-io/onvif/xsd/onvif"
+)
+
+func TestContinuousMoveIncludesZeroPanTiltCoordinates(t *testing.T) {
+	request := ContinuousMove{
+		Velocity: onvif.PTZSpeedPanTilt{
+			PanTilt: onvif.Vector2D{X: 0.5, Y: 0},
+		},
+	}
+
+	encoded, err := xml.Marshal(request)
+	if err != nil {
+		t.Fatalf("xml.Marshal() error = %v", err)
+	}
+	if !strings.Contains(string(encoded), `x="0.5" y="0"`) {
+		t.Fatalf("ContinuousMove PanTilt = %s, want explicit x and y attributes", encoded)
+	}
+}

xsd/onvif/onvif.go

@@ -666,13 +666,13 @@ type PTZSpeedPanTilt struct {
 }
 
 type Vector2D struct {
-	X     float64     `xml:"x,attr,omitempty"`
-	Y     float64     `xml:"y,attr,omitempty"`
+	X     float64     `xml:"x,attr"`
+	Y     float64     `xml:"y,attr"`
 	Space *xsd.AnyURI `xml:"space,attr,omitempty"`
 }
 
 type Vector1D struct {
-	X     float64     `xml:"x,attr,omitempty"`
+	X     float64     `xml:"x,attr"`
 	Space *xsd.AnyURI `xml:"space,attr,omitempty"`
 }
 
@@ -1176,7 +1176,7 @@ type PresetTour struct {
 	Status            PTZPresetTourStatus            `xml:"Status"`
 	AutoStart         xsd.Boolean                    `xml:"AutoStart"`
 	StartingCondition PTZPresetTourStartingCondition `xml:"StartingCondition"`
-	TourSpot          []PTZPresetTourSpot           	 `xml:"TourSpot"`
+	TourSpot          []PTZPresetTourSpot                    `xml:"TourSpot"`
 	Extension         PTZPresetTourExtension         `xml:"Extension"`
 }