aboutsummaryrefslogtreecommitdiff
path: root/hotline/server_test.go
diff options
context:
space:
mode:
authorJeff Halter <868228+jhalter@users.noreply.github.com>2026-07-09 18:28:58 -0700
committerJeff Halter <868228+jhalter@users.noreply.github.com>2026-07-09 18:28:58 -0700
commitfff87ed2d3791d6853529457cc37689e8869b270 (patch)
tree2a32d47fd192ddccb9029ca6cd13605325daaa1b /hotline/server_test.go
parent631b7f1f99c2ff6295152210640a2f9b2c4f5a88 (diff)
Publish ClientConn only after login and guard Account with the state mutex
NewClientConn added the connection to the ClientManager before Account and Version were assigned, so any goroutine iterating ClientMgr.List() during the login handshake could dereference a nil Account (e.g. HandleSetUser reading c.Account.Login) and panic. Account was also read and written across goroutines with no synchronization: HandleSetUser wrote c.Account.Access on another client's connection while that client's own transaction loop read it in Authorize. The connection is now added to the manager in handleNewConnection only once Account, Version, UserName, and Flags are initialized, so a published client is always fully formed. Failed logins never publish the connection at all; Disconnect's manager delete is a no-op for them. Account joins the mutex-guarded session state with accessors in the established style: SetAccount/GetAccount, SetAccountAccess for the one post-login mutation, AccessBytes for building transaction fields, and Authorize now takes the read lock. All cross-goroutine call sites go through the accessors. HandleSetUser previously recomputed the admin flag from the client's stale access level and only converged on the following edit; the access update now happens before the recompute.
Diffstat (limited to 'hotline/server_test.go')
-rw-r--r--hotline/server_test.go6
1 files changed, 4 insertions, 2 deletions
diff --git a/hotline/server_test.go b/hotline/server_test.go
index ebcd073..a9842c8 100644
--- a/hotline/server_test.go
+++ b/hotline/server_test.go
@@ -15,7 +15,6 @@ import (
"time"
"github.com/stretchr/testify/assert"
- "github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"golang.org/x/text/encoding"
"golang.org/x/text/encoding/charmap"
@@ -831,8 +830,11 @@ type nopCloserRWC struct {
func (n *nopCloserRWC) Close() error { return nil }
func TestServer_NewClientConn(t *testing.T) {
+ // The mock has no Add expectation on purpose: NewClientConn must NOT publish the connection
+ // to the ClientManager. Publication happens later in handleNewConnection, once the session
+ // state (Account, Version, UserName, Flags) is fully initialized, so that other goroutines
+ // never observe a partially initialized client.
mockMgr := &MockClientMgr{}
- mockMgr.On("Add", mock.AnythingOfType("*hotline.ClientConn")).Return()
srv := &Server{ClientMgr: mockMgr}