From 2e6193f4bd273027c781ace27243c01707668beb Mon Sep 17 00:00:00 2001 From: Hugo Haas Date: Sun, 21 Jun 2026 14:20:13 -0500 Subject: [PATCH 1/7] =?UTF-8?q?feat(deps):=20upgrade=20resty=20v2=20?= =?UTF-8?q?=E2=86=92=20v3=20(resty.dev/v3=20v3.0.0-rc.2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Update import path to resty.dev/v3 - Rename resp.IsError() → IsStatusFailure(), IsSuccess() → IsStatusSuccess() - Replace removed resp.Body() with resp.Bytes() - Add Close() error to Gateway interface and GatewayCommon - Add SetRetryAllowNonIdempotent(true) to preserve POST retry behaviour - Move Nokia SID cookie from client-level to per-request injection, since v3 has no ClearCookies() API; update tests accordingly - Update BaseURL field access to BaseURL() method call Co-Authored-By: Claude Sonnet 4.6 --- arcadyan.go | 12 +++++----- arcadyan_test.go | 2 +- gateway.go | 15 +++++++++++-- gateway_test.go | 4 ++-- go.mod | 2 +- go.sum | 6 ++--- nokia.go | 18 +++++++-------- nokia_test.go | 58 ++++++++++++++++++++++++------------------------ 8 files changed, 63 insertions(+), 54 deletions(-) diff --git a/arcadyan.go b/arcadyan.go index f52e17f..beac9b2 100644 --- a/arcadyan.go +++ b/arcadyan.go @@ -67,7 +67,7 @@ func (a *ArcadyanGateway) Login(ctx context.Context) error { return fmt.Errorf("login request failed: %w", err) } - if resp.IsError() { + if resp.IsStatusFailure() { return NewAuthError(resp.StatusCode(), resp.String(), nil) } @@ -102,7 +102,7 @@ func (a *ArcadyanGateway) Reboot(ctx context.Context) error { return fmt.Errorf("reboot request failed: %w", err) } - if resp.IsError() { + if resp.IsStatusFailure() { status := resp.StatusCode() if status == http.StatusUnauthorized || status == http.StatusForbidden { a.logout() @@ -129,12 +129,12 @@ func (a *ArcadyanGateway) Request(ctx context.Context, method, path string) (*In return nil, fmt.Errorf("request failed: %w", err) } - if resp.IsError() { + if resp.IsStatusFailure() { return nil, fmt.Errorf("%w: HTTP %d", ErrRequestFailed, resp.StatusCode()) } contentType := resp.Header().Get("Content-Type") - body := resp.Body() + body := resp.Bytes() var data map[string]any if strings.HasPrefix(contentType, jsonContentType) { @@ -172,7 +172,7 @@ func (a *ArcadyanGateway) Status(ctx context.Context) (*StatusResult, error) { if webResult.Error == nil { webResult.Error = NewGatewayError("status", 0, "failed to get registration status", err) } - case resp.IsError(): + case resp.IsStatusFailure(): if webResult.Error == nil { webResult.Error = NewGatewayError( "status", @@ -199,7 +199,7 @@ func (a *ArcadyanGateway) Signal(ctx context.Context) (*SignalResult, error) { return nil, NewGatewayError("signal", 0, "failed to get signal info", err) } - if resp.IsError() { + if resp.IsStatusFailure() { return nil, NewGatewayError( "signal", resp.StatusCode(), diff --git a/arcadyan_test.go b/arcadyan_test.go index c1f3b19..16522e9 100644 --- a/arcadyan_test.go +++ b/arcadyan_test.go @@ -7,9 +7,9 @@ import ( "testing" "time" - "github.com/go-resty/resty/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "resty.dev/v3" ) func newArcadyan(gc *GatewayCommon, token string, exp time.Time) *ArcadyanGateway { diff --git a/gateway.go b/gateway.go index 25180f1..fcd2403 100644 --- a/gateway.go +++ b/gateway.go @@ -6,7 +6,7 @@ import ( "net" "strings" - "github.com/go-resty/resty/v2" + "resty.dev/v3" ) // Gateway defines the interface for T-Mobile gateway implementations. @@ -14,6 +14,7 @@ import ( // Implementations are not safe for concurrent use: Login mutates shared // client state such as auth headers and cookies. type Gateway interface { + Close() error Login(ctx context.Context) error Reboot(ctx context.Context) error Request(ctx context.Context, method, path string) (*InfoResult, error) @@ -54,6 +55,7 @@ func NewGatewayCommon(cfg *GatewayConfig) *GatewayCommon { if cfg.Retries > 0 { client.SetRetryCount(cfg.Retries) + client.SetRetryAllowNonIdempotent(true) } if cfg.Debug { @@ -66,6 +68,15 @@ func NewGatewayCommon(cfg *GatewayConfig) *GatewayCommon { } } +// Close releases resources held by the underlying HTTP client. +func (gc *GatewayCommon) Close() error { + if err := gc.client.Close(); err != nil { + return fmt.Errorf("close client: %w", err) + } + + return nil +} + // CheckWebInterface checks if the gateway web interface is accessible. func (gc *GatewayCommon) CheckWebInterface(ctx context.Context) *StatusResult { resp, err := gc.client.R().SetContext(ctx).Head("/") @@ -79,7 +90,7 @@ func (gc *GatewayCommon) CheckWebInterface(ctx context.Context) *StatusResult { } result.StatusCode = resp.StatusCode() - result.WebInterfaceUp = resp.IsSuccess() + result.WebInterfaceUp = resp.IsStatusSuccess() return result } diff --git a/gateway_test.go b/gateway_test.go index 2ea541a..aa498a2 100644 --- a/gateway_test.go +++ b/gateway_test.go @@ -7,8 +7,8 @@ import ( "testing" "time" - "github.com/go-resty/resty/v2" "github.com/stretchr/testify/assert" + "resty.dev/v3" ) const testServerErrMsg = "server error" @@ -85,7 +85,7 @@ func TestNewGatewayCommon_HostForms(t *testing.T) { for _, tc := range cases { t.Run(tc.host, func(t *testing.T) { gc := NewGatewayCommon(&GatewayConfig{Host: tc.host}) - assert.Equal(t, tc.want, gc.client.BaseURL) + assert.Equal(t, tc.want, gc.client.BaseURL()) }) } } diff --git a/go.mod b/go.mod index 0f770f1..4595fdb 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module github.com/hugoh/tmhi-gateway/v2 go 1.26.0 require ( - github.com/go-resty/resty/v2 v2.17.2 github.com/stretchr/testify v1.11.1 + resty.dev/v3 v3.0.0-rc.2 ) require ( diff --git a/go.sum b/go.sum index d5e48c1..8ffb06c 100644 --- a/go.sum +++ b/go.sum @@ -1,16 +1,14 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/go-resty/resty/v2 v2.17.2 h1:FQW5oHYcIlkCNrMD2lloGScxcHJ0gkjshV3qcQAyHQk= -github.com/go-resty/resty/v2 v2.17.2/go.mod h1:kCKZ3wWmwJaNc7S29BRtUhJwy7iqmn+2mLtQrOyQlVA= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= -golang.org/x/time v0.12.0 h1:ScB/8o8olJvc+CQPWrK3fPZNfh7qgwCrY0zJmoEQLSE= -golang.org/x/time v0.12.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +resty.dev/v3 v3.0.0-rc.2 h1:NE6RqPkUd27eQ5dFSb5tfyT3s6XX9ZySp9qZ7Nw1bn0= +resty.dev/v3 v3.0.0-rc.2/go.mod h1:NTOerrC/4T7/FE6tXIZGIysXXBdgNqwMZuKtxpea9NM= diff --git a/nokia.go b/nokia.go index aa4f2da..93ebfc2 100644 --- a/nokia.go +++ b/nokia.go @@ -65,8 +65,6 @@ func (n *NokiaGateway) Login(ctx context.Context) error { n.credentials.SID = loginResp.Sid n.credentials.csrfToken = loginResp.CsrfToken - //nolint:gosec // Secure/HttpOnly/SameSite only apply to response cookies, not outgoing requests. - n.client.SetCookie(&http.Cookie{Name: sidCookieName, Value: n.credentials.SID}) return nil } @@ -85,12 +83,17 @@ func (n *NokiaGateway) Reboot(ctx context.Context) error { return nil } - resp, err := n.client.R().SetContext(ctx).SetFormData(formData).Post("/reboot_web_app.cgi") + //nolint:gosec // Secure/HttpOnly/SameSite only apply to response cookies, not outgoing requests. + resp, err := n.client.R(). + SetContext(ctx). + SetCookie(&http.Cookie{Name: sidCookieName, Value: n.credentials.SID}). + SetFormData(formData). + Post("/reboot_web_app.cgi") if err != nil { return fmt.Errorf("error sending reboot request: %w", err) } - if resp.IsError() { + if resp.IsStatusFailure() { status := resp.StatusCode() if status == http.StatusUnauthorized || status == http.StatusForbidden { n.logout() @@ -131,9 +134,6 @@ func (n *NokiaGateway) isLoggedIn() bool { func (n *NokiaGateway) logout() { n.credentials = nokiaLoginData{} - // resty.Client.SetCookie/SetCookies both append; assign directly to - // replace the slice so re-login doesn't accumulate stale sid cookies. - n.client.Cookies = nil } func (n *NokiaGateway) getCredentials( @@ -166,7 +166,7 @@ func (n *NokiaGateway) getCredentials( return nil, NewAuthError(0, "login request failed", err) } - if resp.IsError() { + if resp.IsStatusFailure() { return nil, NewAuthError(resp.StatusCode(), resp.String(), nil) } @@ -191,7 +191,7 @@ func (n *NokiaGateway) getNonce(ctx context.Context) (*nokiaNonce, error) { return nil, fmt.Errorf("error getting nonce: %w", err) } - if resp.IsError() { + if resp.IsStatusFailure() { return nil, NewGatewayError("nonce", resp.StatusCode(), resp.String(), ErrAuthentication) } diff --git a/nokia_test.go b/nokia_test.go index e3fd18b..f54bdfa 100644 --- a/nokia_test.go +++ b/nokia_test.go @@ -6,9 +6,9 @@ import ( "strings" "testing" - "github.com/go-resty/resty/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "resty.dev/v3" ) const ( @@ -331,25 +331,37 @@ func TestNokiaGateway_NotImplemented(t *testing.T) { }) } -func TestNokiaGateway_logout_ClearsCookie(t *testing.T) { - ts := newTestServer(t, func(w http.ResponseWriter, _ *http.Request) { +func TestNokiaGateway_logout_ClearsCredentials(t *testing.T) { + gw := newNokia(&GatewayCommon{config: &GatewayConfig{}}, testValidSID, testValidToken) + + gw.logout() + + assert.False(t, gw.isLoggedIn(), "logout should clear credentials") +} + +func TestNokiaGateway_Reboot_SendsSIDCookie(t *testing.T) { + // Reboot must include the current session SID in the request cookie. + var gotCookie string + + ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) + + if r.Method == http.MethodPost && r.URL.Path == "/reboot_web_app.cgi" { + if c, err := r.Cookie(sidCookieName); err == nil { + gotCookie = c.Value + } + } }) gw := nokiaTestGw(ts, nokiaConfig(ts), testValidSID, testValidToken) - //nolint:gosec // Secure/HttpOnly/SameSite only apply to response cookies, not outgoing requests. - gw.client.SetCookie(&http.Cookie{Name: sidCookieName, Value: testValidSID}) - - gw.logout() - assert.False(t, gw.isLoggedIn(), "logout should clear credentials") - assert.Empty(t, gw.client.Cookies, "logout should clear all resty cookies") + require.NoError(t, gw.Reboot(t.Context())) + assert.Equal(t, testValidSID, gotCookie, "reboot request must carry the session SID cookie") } -func TestNokiaGateway_Reboot_ReloginNoDuplicateCookie(t *testing.T) { - // After reboot (which calls logout), a subsequent Login should not - // accumulate a second sid cookie from the previous session. - callCount := 0 +func TestNokiaGateway_Reboot_ReloginHasFreshSID(t *testing.T) { + // After reboot (which calls logout), a subsequent Login must use fresh credentials, + // not carry over the old SID. ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) @@ -358,34 +370,22 @@ func TestNokiaGateway_Reboot_ReloginNoDuplicateCookie(t *testing.T) { case r.Method == http.MethodGet: _, _ = w.Write([]byte(testNonceBody)) case r.Method == http.MethodPost && r.URL.Path == loginWebAppCGI: - callCount++ _, _ = w.Write([]byte(testLoginRespBody)) - case r.Method == http.MethodPost && r.URL.Path == "/reboot_web_app.cgi": - // pass } }) gw := nokiaTestGw(ts, nokiaConfig(ts), testValidSID, testValidToken) - //nolint:gosec // Secure/HttpOnly/SameSite only apply to response cookies, not outgoing requests. - gw.client.SetCookie(&http.Cookie{Name: sidCookieName, Value: testValidSID}) require.NoError(t, gw.Reboot(t.Context())) require.NoError(t, gw.Login(t.Context())) - sidCookies := 0 - - for _, c := range gw.client.Cookies { - if c.Name == sidCookieName { - sidCookies++ - } - } - assert.Equal( t, - 1, - sidCookies, - "re-login after reboot must not accumulate duplicate sid cookies", + "testSid", + gw.credentials.SID, + "re-login after reboot must have fresh SID, not the old one", ) + assert.NotEqual(t, testValidSID, gw.credentials.SID) } func TestNewNokiaGateway(t *testing.T) { From 99a2b34348d947ea48e453ac0f2e9bc467205480 Mon Sep 17 00:00:00 2001 From: Hugo Haas Date: Tue, 30 Jun 2026 22:36:35 -0500 Subject: [PATCH 2/7] fix(gateway): check DryRun before Login in Reboot() Reboot() called Login() before checking config.DryRun, so dry-run mode still performed a real authentication round-trip against the gateway in both ArcadyanGateway and NokiaGateway. --- arcadyan.go | 8 ++++---- arcadyan_test.go | 15 +++++++++++++++ nokia.go | 8 ++++---- nokia_test.go | 13 +++++++++++++ 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/arcadyan.go b/arcadyan.go index beac9b2..6d72783 100644 --- a/arcadyan.go +++ b/arcadyan.go @@ -86,15 +86,15 @@ func (a *ArcadyanGateway) Login(ctx context.Context) error { // Reboot restarts the Arcadyan gateway. func (a *ArcadyanGateway) Reboot(ctx context.Context) error { + if a.config.DryRun { + return nil + } + err := a.Login(ctx) if err != nil { return fmt.Errorf("cannot reboot without successful login flow: %w", err) } - if a.config.DryRun { - return nil - } - rebootRequestPath := "/TMI/v1/gateway/reset?set=reboot" resp, err := a.client.R().SetContext(ctx).Post(rebootRequestPath) diff --git a/arcadyan_test.go b/arcadyan_test.go index 16522e9..c7fde3c 100644 --- a/arcadyan_test.go +++ b/arcadyan_test.go @@ -113,6 +113,21 @@ func TestArcadyanGateway_Reboot_DryRun(t *testing.T) { require.NoError(t, err) } +func TestArcadyanGateway_Reboot_DryRun_SkipsLogin(t *testing.T) { + // Not logged in: if DryRun didn't short-circuit before Login, this would + // hit the network and the handler below would fail the test. + ts := newTestServer(t, func(_ http.ResponseWriter, _ *http.Request) { + t.Fatal("unexpected HTTP call in dry run") + }) + + gw := newArcadyan(testCommon(ts), "", time.Time{}) + gw.config = testConfig(ts) + gw.config.DryRun = true + + err := gw.Reboot(t.Context()) + require.NoError(t, err) +} + func TestArcadyanGateway_Reboot_Success(t *testing.T) { ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodPost, r.Method) diff --git a/nokia.go b/nokia.go index 93ebfc2..728fe96 100644 --- a/nokia.go +++ b/nokia.go @@ -71,6 +71,10 @@ func (n *NokiaGateway) Login(ctx context.Context) error { // Reboot restarts the Nokia gateway. func (n *NokiaGateway) Reboot(ctx context.Context) error { + if n.config.DryRun { + return nil + } + if err := n.Login(ctx); err != nil { return fmt.Errorf("cannot reboot without successful login flow: %w", err) } @@ -79,10 +83,6 @@ func (n *NokiaGateway) Reboot(ctx context.Context) error { "csrf_token": n.credentials.csrfToken, } - if n.config.DryRun { - return nil - } - //nolint:gosec // Secure/HttpOnly/SameSite only apply to response cookies, not outgoing requests. resp, err := n.client.R(). SetContext(ctx). diff --git a/nokia_test.go b/nokia_test.go index f54bdfa..fa0b939 100644 --- a/nokia_test.go +++ b/nokia_test.go @@ -240,6 +240,19 @@ func TestNokiaGateway_Reboot_DryRun(t *testing.T) { assert.NoError(t, err) } +func TestNokiaGateway_Reboot_DryRun_SkipsLogin(t *testing.T) { + // Not logged in: if DryRun didn't short-circuit before Login, this would + // hit the network and the handler below would fail the test. + ts := newTestServer(t, func(_ http.ResponseWriter, _ *http.Request) { + t.Fatal("unexpected HTTP call in dry run") + }) + + gw := nokiaTestGw(ts, &GatewayConfig{DryRun: true}, "", "") + + err := gw.Reboot(t.Context()) + assert.NoError(t, err) +} + func TestNokiaGateway_Reboot_ErrorResponse(t *testing.T) { ts := newTestServer(t, textResponder(http.StatusInternalServerError, "reboot failed")) gw := nokiaTestGw(ts, &GatewayConfig{}, testValidSID, testValidToken) From 373a01ecc0b3d32642efa866b115c3a460cf2102 Mon Sep 17 00:00:00 2001 From: Hugo Haas Date: Tue, 30 Jun 2026 22:41:31 -0500 Subject: [PATCH 3/7] fix(arcadyan): use ErrStatusFailed sentinel in Status() Status() wrapped a failed registration-status GET in ErrSignalFailed, copy-pasted from Signal(). A caller checking errors.Is(err, ErrSignalFailed) to detect a real Signal() failure got a false positive from Status(). --- arcadyan.go | 2 +- arcadyan_test.go | 4 +++- errors.go | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/arcadyan.go b/arcadyan.go index 6d72783..b7298fc 100644 --- a/arcadyan.go +++ b/arcadyan.go @@ -178,7 +178,7 @@ func (a *ArcadyanGateway) Status(ctx context.Context) (*StatusResult, error) { "status", resp.StatusCode(), "failed to get registration status", - ErrSignalFailed, + ErrStatusFailed, ) } default: diff --git a/arcadyan_test.go b/arcadyan_test.go index c7fde3c..b572b36 100644 --- a/arcadyan_test.go +++ b/arcadyan_test.go @@ -315,7 +315,9 @@ func TestArcadyanGateway_Status_Error(t *testing.T) { result, err := gw.Status(t.Context()) require.NoError(t, err) assert.True(t, result.WebInterfaceUp) - assert.Error(t, result.Error) + require.Error(t, result.Error) + require.ErrorIs(t, result.Error, ErrStatusFailed) + assert.NotErrorIs(t, result.Error, ErrSignalFailed) } func TestArcadyanGateway_Request_ErrorStatus(t *testing.T) { diff --git a/errors.go b/errors.go index 7ca528e..3983d09 100644 --- a/errors.go +++ b/errors.go @@ -15,6 +15,8 @@ var ( ErrRebootFailed = errors.New("reboot failed") // ErrSignalFailed indicates a signal operation failed. ErrSignalFailed = errors.New("signal failed") + // ErrStatusFailed indicates a status operation failed. + ErrStatusFailed = errors.New("status failed") // ErrRequestFailed indicates an HTTP request returned an error status. ErrRequestFailed = errors.New("request failed") ) From 35487c59332113d72c993d7b7b141e333f99b962 Mon Sep 17 00:00:00 2001 From: Hugo Haas Date: Tue, 30 Jun 2026 22:42:32 -0500 Subject: [PATCH 4/7] fix(gateway): panic clearly on nil GatewayConfig NewGatewayCommon dereferenced cfg.Host with no nil check, so NewArcadyanGateway(nil)/NewNokiaGateway(nil) crashed with an opaque nil-pointer dereference instead of a clear, attributable error. --- gateway.go | 4 ++++ gateway_test.go | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/gateway.go b/gateway.go index fcd2403..0cd2396 100644 --- a/gateway.go +++ b/gateway.go @@ -33,6 +33,10 @@ type GatewayCommon struct { // NewGatewayCommon creates a new GatewayCommon with the given configuration. func NewGatewayCommon(cfg *GatewayConfig) *GatewayCommon { + if cfg == nil { + panic("tmhi: GatewayConfig must not be nil") + } + host := cfg.Host // Bare IPv6 literals must be bracketed in URLs. // Use ContainsRune(':') rather than To4()==nil so IPv4-mapped IPv6 diff --git a/gateway_test.go b/gateway_test.go index aa498a2..c7a3868 100644 --- a/gateway_test.go +++ b/gateway_test.go @@ -42,6 +42,12 @@ func TestNewGatewayCommon(t *testing.T) { assert.Equal(t, cfg, gc.config) } +func TestNewGatewayCommon_NilConfig(t *testing.T) { + assert.PanicsWithValue(t, "tmhi: GatewayConfig must not be nil", func() { + NewGatewayCommon(nil) + }) +} + func TestNewGatewayCommon_UserAgent(t *testing.T) { var gotUA string From ab131273b87176839e75633afdac9c6b28a7ab79 Mon Sep 17 00:00:00 2001 From: Hugo Haas Date: Tue, 30 Jun 2026 22:43:36 -0500 Subject: [PATCH 5/7] refactor(gateway): extract shared Reboot() flow across vendors ArcadyanGateway.Reboot() and NokiaGateway.Reboot() duplicated the entire login/dry-run/POST/401-403-logout/success-logout control flow verbatim, letting bugs (like the DryRun-after-Login issue) drift between the two copies. Extract a shared performReboot() on GatewayCommon, driven by a small authSession interface that both vendors' existing logout() methods already satisfy, and have each vendor supply only its login call and reboot request. Unifies the previously divergent "reboot request failed" vs. "error sending reboot request" error text between vendors. --- arcadyan.go | 35 +++++------------------------------ gateway.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ gateway_test.go | 7 +++++++ nokia.go | 45 ++++++++++++--------------------------------- nokia_test.go | 2 +- 5 files changed, 70 insertions(+), 64 deletions(-) diff --git a/arcadyan.go b/arcadyan.go index b7298fc..e54c593 100644 --- a/arcadyan.go +++ b/arcadyan.go @@ -5,9 +5,10 @@ import ( "context" "encoding/json" "fmt" - "net/http" "strings" "time" + + "resty.dev/v3" ) // infoURL is the endpoint for gateway information. @@ -86,35 +87,9 @@ func (a *ArcadyanGateway) Login(ctx context.Context) error { // Reboot restarts the Arcadyan gateway. func (a *ArcadyanGateway) Reboot(ctx context.Context) error { - if a.config.DryRun { - return nil - } - - err := a.Login(ctx) - if err != nil { - return fmt.Errorf("cannot reboot without successful login flow: %w", err) - } - - rebootRequestPath := "/TMI/v1/gateway/reset?set=reboot" - - resp, err := a.client.R().SetContext(ctx).Post(rebootRequestPath) - if err != nil { - return fmt.Errorf("reboot request failed: %w", err) - } - - if resp.IsStatusFailure() { - status := resp.StatusCode() - if status == http.StatusUnauthorized || status == http.StatusForbidden { - a.logout() - } - - return NewGatewayError("reboot", status, resp.String(), ErrRebootFailed) - } - - // A successful reboot invalidates the session on the gateway side. - a.logout() - - return nil + return a.performReboot(ctx, a, a.Login, func() (*resty.Response, error) { + return a.client.R().SetContext(ctx).Post("/TMI/v1/gateway/reset?set=reboot") + }) } // Info retrieves gateway information. diff --git a/gateway.go b/gateway.go index 0cd2396..3f88357 100644 --- a/gateway.go +++ b/gateway.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net" + "net/http" "strings" "resty.dev/v3" @@ -98,3 +99,47 @@ func (gc *GatewayCommon) CheckWebInterface(ctx context.Context) *StatusResult { return result } + +// authSession is implemented by gateway credential holders that must be +// invalidated when the gateway rejects a request as unauthenticated. +type authSession interface { + logout() +} + +// performReboot runs the shared reboot flow used by every gateway +// implementation: skip entirely in dry-run mode, otherwise authenticate, +// issue the vendor-supplied reboot request, and invalidate the session on +// success or on an auth rejection. +func (gc *GatewayCommon) performReboot( + ctx context.Context, + sess authSession, + login func(context.Context) error, + doRequest func() (*resty.Response, error), +) error { + if gc.config.DryRun { + return nil + } + + if err := login(ctx); err != nil { + return fmt.Errorf("cannot reboot without successful login flow: %w", err) + } + + resp, err := doRequest() + if err != nil { + return fmt.Errorf("reboot request failed: %w", err) + } + + if resp.IsStatusFailure() { + status := resp.StatusCode() + if status == http.StatusUnauthorized || status == http.StatusForbidden { + sess.logout() + } + + return NewGatewayError("reboot", status, resp.String(), ErrRebootFailed) + } + + // A successful reboot invalidates the session on the gateway side. + sess.logout() + + return nil +} diff --git a/gateway_test.go b/gateway_test.go index c7a3868..edd2df1 100644 --- a/gateway_test.go +++ b/gateway_test.go @@ -13,6 +13,13 @@ import ( const testServerErrMsg = "server error" +// Both vendor gateways must satisfy authSession so performReboot can +// invalidate their session on an auth rejection or a successful reboot. +var ( + _ authSession = (*ArcadyanGateway)(nil) + _ authSession = (*NokiaGateway)(nil) +) + func newTestServer(t *testing.T, handler http.HandlerFunc) *httptest.Server { t.Helper() diff --git a/nokia.go b/nokia.go index 728fe96..0e31008 100644 --- a/nokia.go +++ b/nokia.go @@ -5,6 +5,8 @@ import ( "fmt" "net/http" "strings" + + "resty.dev/v3" ) const ( @@ -71,41 +73,18 @@ func (n *NokiaGateway) Login(ctx context.Context) error { // Reboot restarts the Nokia gateway. func (n *NokiaGateway) Reboot(ctx context.Context) error { - if n.config.DryRun { - return nil - } - - if err := n.Login(ctx); err != nil { - return fmt.Errorf("cannot reboot without successful login flow: %w", err) - } - - formData := map[string]string{ - "csrf_token": n.credentials.csrfToken, - } - - //nolint:gosec // Secure/HttpOnly/SameSite only apply to response cookies, not outgoing requests. - resp, err := n.client.R(). - SetContext(ctx). - SetCookie(&http.Cookie{Name: sidCookieName, Value: n.credentials.SID}). - SetFormData(formData). - Post("/reboot_web_app.cgi") - if err != nil { - return fmt.Errorf("error sending reboot request: %w", err) - } - - if resp.IsStatusFailure() { - status := resp.StatusCode() - if status == http.StatusUnauthorized || status == http.StatusForbidden { - n.logout() + return n.performReboot(ctx, n, n.Login, func() (*resty.Response, error) { + formData := map[string]string{ + "csrf_token": n.credentials.csrfToken, } - return NewGatewayError("reboot", status, resp.String(), ErrRebootFailed) - } - - // A successful reboot invalidates the session on the gateway side. - n.logout() - - return nil + //nolint:gosec // Secure/HttpOnly/SameSite only apply to response cookies, not outgoing requests. + return n.client.R(). + SetContext(ctx). + SetCookie(&http.Cookie{Name: sidCookieName, Value: n.credentials.SID}). + SetFormData(formData). + Post("/reboot_web_app.cgi") + }) } // Request is not implemented for Nokia gateway. diff --git a/nokia_test.go b/nokia_test.go index fa0b939..815df3d 100644 --- a/nokia_test.go +++ b/nokia_test.go @@ -322,7 +322,7 @@ func TestNokiaGateway_Reboot_RequestError(t *testing.T) { err := gw.Reboot(t.Context()) require.Error(t, err) - assert.Contains(t, err.Error(), "error sending reboot request") + assert.Contains(t, err.Error(), "reboot request failed") } func TestNokiaGateway_NotImplemented(t *testing.T) { From 581149442a5df1010c8c9ecebd422f352657084d Mon Sep 17 00:00:00 2001 From: Hugo Haas Date: Tue, 30 Jun 2026 22:45:47 -0500 Subject: [PATCH 6/7] refactor(test): dedupe Nokia test helpers against shared gateway_test.go builders nokiaConfig(), nokiaTestGw(), and nokiaTestGwClosed() reimplemented testConfig()/testCommon() and the closed-server GatewayCommon builder already used by Arcadyan's tests. Extract newClosedServerCommon() in gateway_test.go and have both arcadyan_test.go and nokia_test.go call it, and have nokiaTestGw() reuse testCommon() instead of manually constructing a resty client. --- arcadyan_test.go | 15 ++----------- gateway_test.go | 15 +++++++++++++ nokia_test.go | 56 +++++++++++++++++------------------------------- 3 files changed, 37 insertions(+), 49 deletions(-) diff --git a/arcadyan_test.go b/arcadyan_test.go index b572b36..a510d24 100644 --- a/arcadyan_test.go +++ b/arcadyan_test.go @@ -39,19 +39,8 @@ func newArcadyanWithToken(t *testing.T, ts *httptest.Server) *ArcadyanGateway { func newClosedServerGateway(t *testing.T, token string, exp time.Time) *ArcadyanGateway { t.Helper() - ts := newTestServer(t, http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})) - cfg := testConfigNoCreds(ts) - baseURL := ts.URL - ts.Close() - - return newArcadyan( - &GatewayCommon{ - client: resty.NewWithClient(&http.Client{}).SetBaseURL(baseURL), - config: cfg, - }, - token, - exp, - ) + + return newArcadyan(newClosedServerCommon(t), token, exp) } func withHeadCheck(next http.HandlerFunc) http.HandlerFunc { diff --git a/gateway_test.go b/gateway_test.go index edd2df1..df14301 100644 --- a/gateway_test.go +++ b/gateway_test.go @@ -36,6 +36,21 @@ func testCommon(ts *httptest.Server) *GatewayCommon { } } +// newClosedServerCommon returns a GatewayCommon pointed at an already-closed +// server, used to simulate connection-refused errors. +func newClosedServerCommon(t *testing.T) *GatewayCommon { + t.Helper() + + ts := newTestServer(t, http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})) + baseURL := ts.URL + ts.Close() + + return &GatewayCommon{ + client: resty.NewWithClient(&http.Client{}).SetBaseURL(baseURL), + config: &GatewayConfig{}, + } +} + func TestNewGatewayCommon(t *testing.T) { cfg := &GatewayConfig{ Host: testIP, diff --git a/nokia_test.go b/nokia_test.go index 815df3d..761f1cc 100644 --- a/nokia_test.go +++ b/nokia_test.go @@ -3,12 +3,10 @@ package tmhi import ( "net/http" "net/http/httptest" - "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "resty.dev/v3" ) const ( @@ -30,31 +28,17 @@ func newNokia(gc *GatewayCommon, sid, token string) *NokiaGateway { return gw } -func nokiaConfig(ts *httptest.Server) *GatewayConfig { - return &GatewayConfig{ - Host: strings.TrimPrefix(ts.URL, "http://"), - Username: testUsername, - Password: testPassword, - } -} - func nokiaTestGw(ts *httptest.Server, cfg *GatewayConfig, sid, token string) *NokiaGateway { - return newNokia(&GatewayCommon{ - client: resty.NewWithClient(&http.Client{}).SetBaseURL(ts.URL), - config: cfg, - }, sid, token) + gc := testCommon(ts) + gc.config = cfg + + return newNokia(gc, sid, token) } -func nokiaTestGwClosed(cfg *GatewayConfig, sid, token string) *NokiaGateway { - ts := httptest.NewServer(http.HandlerFunc( - func(_ http.ResponseWriter, _ *http.Request) {}, - )) - ts.Close() +func nokiaTestGwClosed(t *testing.T, sid, token string) *NokiaGateway { + t.Helper() - return newNokia(&GatewayCommon{ - client: resty.NewWithClient(&http.Client{}).SetBaseURL(ts.URL), - config: cfg, - }, sid, token) + return newNokia(newClosedServerCommon(t), sid, token) } func Test_LoginSuccess(t *testing.T) { @@ -92,7 +76,7 @@ func TestNokiaGateway_getCredentials_ErrorResponse(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - gw := nokiaTestGw(tc.ts, nokiaConfig(tc.ts), "", "") + gw := nokiaTestGw(tc.ts, testConfig(tc.ts), "", "") _, err := gw.getCredentials(t.Context(), nokiaNonce{Nonce: "test"}) require.Error(t, err) @@ -108,7 +92,7 @@ func TestNokiaGateway_Reboot_Success(t *testing.T) { w.WriteHeader(http.StatusOK) }) - gw := nokiaTestGw(ts, nokiaConfig(ts), testValidSID, testValidToken) + gw := nokiaTestGw(ts, testConfig(ts), testValidSID, testValidToken) err := gw.Reboot(t.Context()) require.NoError(t, err) @@ -117,7 +101,7 @@ func TestNokiaGateway_Reboot_Success(t *testing.T) { func TestNokiaGateway_Reboot_StaleSession(t *testing.T) { ts := newTestServer(t, textResponder(http.StatusUnauthorized, "session expired")) - gw := nokiaTestGw(ts, nokiaConfig(ts), testValidSID, testValidToken) + gw := nokiaTestGw(ts, testConfig(ts), testValidSID, testValidToken) err := gw.Reboot(t.Context()) require.ErrorIs(t, err, ErrRebootFailed) @@ -139,7 +123,7 @@ func TestNokiaGateway_Status(t *testing.T) { } func TestNokiaGateway_getNonce_ErrorResponse(t *testing.T) { - gw := nokiaTestGwClosed(&GatewayConfig{}, "", "") + gw := nokiaTestGwClosed(t, "", "") _, err := gw.getNonce(t.Context()) require.Error(t, err) @@ -182,7 +166,7 @@ func TestNokiaGateway_getCredentials_NonceFormField(t *testing.T) { _, _ = w.Write([]byte(testLoginRespBody)) }) - gw := nokiaTestGw(ts, nokiaConfig(ts), "", "") + gw := nokiaTestGw(ts, testConfig(ts), "", "") _, err := gw.getCredentials(t.Context(), nokiaNonce{Nonce: rawNonce, RandomKey: "key"}) require.NoError(t, err) assert.Equal( @@ -195,7 +179,7 @@ func TestNokiaGateway_getCredentials_NonceFormField(t *testing.T) { func TestNokiaGateway_getCredentials_Success(t *testing.T) { ts := newTestServer(t, jsonResponder(http.StatusOK, testLoginRespBody)) - gw := nokiaTestGw(ts, nokiaConfig(ts), "", "") + gw := nokiaTestGw(ts, testConfig(ts), "", "") loginResp, err := gw.getCredentials( t.Context(), @@ -214,7 +198,7 @@ func TestNokiaGateway_Login_Alreadyauthenticated(t *testing.T) { } func TestNokiaGateway_Login_NonceError(t *testing.T) { - gw := nokiaTestGwClosed(&GatewayConfig{}, "", "") + gw := nokiaTestGwClosed(t, "", "") err := gw.Login(t.Context()) require.Error(t, err) @@ -223,7 +207,7 @@ func TestNokiaGateway_Login_NonceError(t *testing.T) { func TestNokiaGateway_Login_CredentialsError(t *testing.T) { ts := newTestServer(t, jsonResponder(http.StatusOK, testNonceBody)) - gw := nokiaTestGw(ts, nokiaConfig(ts), "", "") + gw := nokiaTestGw(ts, testConfig(ts), "", "") err := gw.Login(t.Context()) assert.Error(t, err) @@ -289,7 +273,7 @@ func TestNokiaGateway_Login_NonceSuccessCredentialsError(t *testing.T) { } }) - gw := nokiaTestGw(ts, nokiaConfig(ts), "", "") + gw := nokiaTestGw(ts, testConfig(ts), "", "") err := gw.Login(t.Context()) require.Error(t, err) @@ -309,7 +293,7 @@ func TestNokiaGateway_Login_Success(t *testing.T) { } }) - gw := nokiaTestGw(ts, nokiaConfig(ts), "", "") + gw := nokiaTestGw(ts, testConfig(ts), "", "") err := gw.Login(t.Context()) require.NoError(t, err) @@ -318,7 +302,7 @@ func TestNokiaGateway_Login_Success(t *testing.T) { } func TestNokiaGateway_Reboot_RequestError(t *testing.T) { - gw := nokiaTestGwClosed(&GatewayConfig{}, testValidSID, testValidToken) + gw := nokiaTestGwClosed(t, testValidSID, testValidToken) err := gw.Reboot(t.Context()) require.Error(t, err) @@ -366,7 +350,7 @@ func TestNokiaGateway_Reboot_SendsSIDCookie(t *testing.T) { } }) - gw := nokiaTestGw(ts, nokiaConfig(ts), testValidSID, testValidToken) + gw := nokiaTestGw(ts, testConfig(ts), testValidSID, testValidToken) require.NoError(t, gw.Reboot(t.Context())) assert.Equal(t, testValidSID, gotCookie, "reboot request must carry the session SID cookie") @@ -387,7 +371,7 @@ func TestNokiaGateway_Reboot_ReloginHasFreshSID(t *testing.T) { } }) - gw := nokiaTestGw(ts, nokiaConfig(ts), testValidSID, testValidToken) + gw := nokiaTestGw(ts, testConfig(ts), testValidSID, testValidToken) require.NoError(t, gw.Reboot(t.Context())) require.NoError(t, gw.Login(t.Context())) From 8a8956afcf4e9740e420c20b1ec5b8b860fb10f0 Mon Sep 17 00:00:00 2001 From: Hugo Haas Date: Wed, 1 Jul 2026 06:30:57 -0500 Subject: [PATCH 7/7] test: close coverage gaps found in test-suite audit Add cases for GatewayCommon.Close(), Arcadyan Status()'s network-error branch, and Request()'s invalid-JSON branch (all previously uncovered). Rename Test_LoginSuccess/Test_LoginFailure to TestNokiaLoginResp_HasCredentials as a table-driven test, since they exercised nokiaLoginResp.hasCredentials(), not a login flow. --- arcadyan_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ gateway_test.go | 9 +++++++++ nokia_test.go | 33 ++++++++++++++++++++------------- 3 files changed, 74 insertions(+), 13 deletions(-) diff --git a/arcadyan_test.go b/arcadyan_test.go index a510d24..6633c01 100644 --- a/arcadyan_test.go +++ b/arcadyan_test.go @@ -309,6 +309,51 @@ func TestArcadyanGateway_Status_Error(t *testing.T) { assert.NotErrorIs(t, result.Error, ErrSignalFailed) } +func TestArcadyanGateway_Status_NetworkError(t *testing.T) { + // HEAD succeeds so CheckWebInterface leaves webResult.Error nil; the GET + // for registration status hijacks and closes the connection to force a + // transport-level error distinct from an HTTP status failure. + ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodHead { + w.WriteHeader(http.StatusOK) + + return + } + + hj, ok := w.(http.Hijacker) + if !ok { + t.Fatal("ResponseWriter does not support hijacking") + } + + conn, _, err := hj.Hijack() + if err != nil { + t.Fatal(err) + } + + _ = conn.Close() + }) + + gw := newArcadyanWithToken(t, ts) + + result, err := gw.Status(t.Context()) + require.NoError(t, err) + assert.True(t, result.WebInterfaceUp) + require.Error(t, result.Error) + assert.Contains(t, result.Error.Error(), "failed to get registration status") +} + +func TestArcadyanGateway_Request_InvalidJSON(t *testing.T) { + ts := newTestServer(t, jsonResponder(http.StatusOK, "not valid json")) + + gw := newArcadyan(testCommon(ts), "valid-token", time.Now().Add(1*time.Hour)) + gw.config = testConfigNoCreds(ts) + + result, err := gw.Request(t.Context(), "GET", "/test") + require.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "json unmarshal failed") +} + func TestArcadyanGateway_Request_ErrorStatus(t *testing.T) { cases := []struct { name string diff --git a/gateway_test.go b/gateway_test.go index df14301..43ebc3e 100644 --- a/gateway_test.go +++ b/gateway_test.go @@ -51,6 +51,15 @@ func newClosedServerCommon(t *testing.T) *GatewayCommon { } } +func TestGatewayCommon_Close(t *testing.T) { + ts := newTestServer(t, func(_ http.ResponseWriter, _ *http.Request) {}) + gc := testCommon(ts) + + assert.NoError(t, gc.Close()) + // Close is safe to call more than once. + assert.NoError(t, gc.Close()) +} + func TestNewGatewayCommon(t *testing.T) { cfg := &GatewayConfig{ Host: testIP, diff --git a/nokia_test.go b/nokia_test.go index 761f1cc..9ad7b77 100644 --- a/nokia_test.go +++ b/nokia_test.go @@ -41,22 +41,29 @@ func nokiaTestGwClosed(t *testing.T, sid, token string) *NokiaGateway { return newNokia(newClosedServerCommon(t), sid, token) } -func Test_LoginSuccess(t *testing.T) { - valid := &nokiaLoginResp{ - Success: 0, - Reason: 0, - Sid: "foo", - CsrfToken: "bar", +func TestNokiaLoginResp_HasCredentials(t *testing.T) { + cases := []struct { + name string + resp *nokiaLoginResp + want bool + }{ + { + name: "success", + resp: &nokiaLoginResp{Success: 0, Reason: 0, Sid: "foo", CsrfToken: "bar"}, + want: true, + }, + { + name: "failure reason", + resp: &nokiaLoginResp{Success: 0, Reason: 600}, + want: false, + }, } - assert.True(t, valid.hasCredentials()) -} -func Test_LoginFailure(t *testing.T) { - invalid := &nokiaLoginResp{ - Success: 0, - Reason: 600, + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, tc.resp.hasCredentials()) + }) } - assert.False(t, invalid.hasCredentials()) } func TestNokiaGateway_getCredentials_ErrorResponse(t *testing.T) {