aboutsummaryrefslogtreecommitdiff
path: root/hotline/client_conn_test.go
diff options
context:
space:
mode:
authorJeff Halter <868228+jhalter@users.noreply.github.com>2026-06-12 08:47:54 -0700
committerJeff Halter <868228+jhalter@users.noreply.github.com>2026-06-12 08:47:54 -0700
commitc15f8510fbd5ccf9a122f88d6e975d1ef69e00b3 (patch)
tree1ba7e392610637ed792d0003568cf9edb6adc526 /hotline/client_conn_test.go
parentb2c462a3a1353f0653a5964b3a6924538ce83523 (diff)
Fix data races on ClientConn state, banner reload, and rate limiter growth
ClientConn's mutable session state (Flags, UserName, Icon, IdleTime, AutoReply) was guarded inconsistently: two mutexes (FlagsMU and mu) covered some paths while others mutated or read the fields with no locking at all, including HandleSetClientUserInfo, HandleUpdateUser (which writes other clients' admin flag), the login flow, the HTTP API handlers, and the keepalive loop. Consolidate on a single mutex with accessor methods (SetFlag/IsFlagSet/FlagBytes, SetUserName/GetUserName, and so on) used by all production code; direct field access remains for test construction. The idle/away logic moves into incrementIdleTime and clearIdleAndAway helpers that report whether a notification is needed, so SendAll is no longer called while holding the lock. HandleRejectChatInvite also no longer appends to the username slice, which could write past its length into the backing buffer. The server banner is now behind Banner/SetBanner with an RWMutex: the SIGHUP reload previously reassigned the field while banner download goroutines read it, and nilled it when the file read failed. Reload now keeps the previous banner on failure. Per-IP rate limiter entries now record a last-seen time, and the keepalive ticker evicts entries idle for over seven days, so the map no longer grows unboundedly with each unique client IP.
Diffstat (limited to 'hotline/client_conn_test.go')
-rw-r--r--hotline/client_conn_test.go57
1 files changed, 57 insertions, 0 deletions
diff --git a/hotline/client_conn_test.go b/hotline/client_conn_test.go
index 5c5463d..3312954 100644
--- a/hotline/client_conn_test.go
+++ b/hotline/client_conn_test.go
@@ -660,3 +660,60 @@ func TestClientConn_SendDisconnectRace(t *testing.T) {
wg.Wait()
}
+
+func TestClientConn_incrementIdleTime(t *testing.T) {
+ cc := &ClientConn{}
+
+ // Increment until just below the idle threshold: not yet away.
+ for i := 0; i < userIdleSeconds/idleCheckInterval; i++ {
+ assert.False(t, cc.incrementIdleTime(idleCheckInterval))
+ }
+ assert.False(t, cc.IsFlagSet(UserFlagAway))
+
+ // The increment that crosses the threshold marks the client away exactly once.
+ assert.True(t, cc.incrementIdleTime(idleCheckInterval))
+ assert.True(t, cc.IsFlagSet(UserFlagAway))
+ assert.False(t, cc.incrementIdleTime(idleCheckInterval), "already-away client should not be marked away again")
+}
+
+func TestClientConn_clearIdleAndAway(t *testing.T) {
+ cc := &ClientConn{IdleTime: 500}
+
+ // Not away: idle timer resets, no notification needed.
+ assert.False(t, cc.clearIdleAndAway())
+ assert.Equal(t, 0, cc.IdleTime)
+
+ // Away: flag clears and the caller is told to notify.
+ cc.SetFlag(UserFlagAway, 1)
+ assert.True(t, cc.clearIdleAndAway())
+ assert.False(t, cc.IsFlagSet(UserFlagAway))
+ assert.False(t, cc.clearIdleAndAway(), "second clear should report no change")
+}
+
+// TestClientConn_sessionStateRace exercises concurrent access to the mutable session state through
+// the accessor methods. Run with -race.
+func TestClientConn_sessionStateRace(t *testing.T) {
+ cc := &ClientConn{}
+
+ var wg sync.WaitGroup
+ for range 4 {
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+ for i := range 100 {
+ cc.SetUserName(fmt.Appendf(nil, "user-%d", i))
+ _ = cc.GetUserName()
+ cc.SetIcon([]byte{0, byte(i)})
+ _ = cc.GetIcon()
+ cc.SetAutoReply([]byte("brb"))
+ _ = cc.GetAutoReply()
+ cc.SetFlag(UserFlagRefusePM, uint(i%2))
+ _ = cc.IsFlagSet(UserFlagRefusePM)
+ _ = cc.FlagBytes()
+ _ = cc.incrementIdleTime(idleCheckInterval)
+ _ = cc.clearIdleAndAway()
+ }
+ }()
+ }
+ wg.Wait()
+}