Return errors for failed SOAP responses

Propagate HTTP 4xx/5xx errors from SOAP and digest requests, preserve responses, and ensure PTZ zero coordinates are serialized. Add regression tests for both behaviors.

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
+	return resp, responseError(resp)
-	if resp.StatusCode >= 400 && resp.StatusCode < 600 {
+}
-		return resp, errors.Errorf("Server error: %d: %s", resp.StatusCode, resp.Status)
-	}
 
-	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"`
+	X     float64     `xml:"x,attr"`
-	Y     float64     `xml:"y,attr,omitempty"`
+	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"`
 }