From fe86ea942d965af6f2999839d72198148fa592ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Verstraeten?= Date: Wed, 5 Aug 2026 13:13:42 +0200 Subject: [PATCH] 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 | 47 +++++++++++++++++++++++++++++++++++++++ networking/networking.go | 18 ++++++++------- ptz/types_test.go | 25 +++++++++++++++++++++ xsd/onvif/onvif.go | 8 +++---- 4 files changed, 86 insertions(+), 12 deletions(-) create mode 100644 ptz/types_test.go diff --git a/networking/digest_test.go b/networking/digest_test.go index 9eae6b9..6d8aa91 100644 --- a/networking/digest_test.go +++ b/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, "", "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, "", "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) diff --git a/networking/networking.go b/networking/networking.go index d86cc61..5129fdc 100644 --- a/networking/networking.go +++ b/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 diff --git a/ptz/types_test.go b/ptz/types_test.go new file mode 100644 index 0000000..646fdf9 --- /dev/null +++ b/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) + } +} diff --git a/xsd/onvif/onvif.go b/xsd/onvif/onvif.go index 9cc246a..15d15e0 100644 --- a/xsd/onvif/onvif.go +++ b/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"` }