aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--hotline/client_conn.go55
-rw-r--r--hotline/server.go18
-rw-r--r--hotline/server_test.go6
-rw-r--r--internal/mobius/api.go4
-rw-r--r--internal/mobius/handlers_accounts.go10
-rw-r--r--internal/mobius/handlers_chat.go2
-rw-r--r--internal/mobius/handlers_session.go8
7 files changed, 76 insertions, 27 deletions
diff --git a/hotline/client_conn.go b/hotline/client_conn.go
index 3419e0a..fe4dc72 100644
--- a/hotline/client_conn.go
+++ b/hotline/client_conn.go
@@ -32,13 +32,13 @@ type ClientConn struct {
ID ClientID
Version []byte // TODO: make fixed size of 2
- Account *Account
- Server *Server // TODO: consider adding methods to interact with server
+ Server *Server // TODO: consider adding methods to interact with server
// The following fields hold mutable session state guarded by mu. They are read and written
// by multiple goroutines (the client's own transaction loop, other clients' handlers, and the
// server keepalive loop), so production code must use the accessor methods below. Direct
// field access is only safe before the connection is shared, e.g. in tests.
+ Account *Account
Flags UserFlags
UserName []byte
Icon []byte // TODO: make fixed size of 2
@@ -197,6 +197,45 @@ func (cc *ClientConn) GetAutoReply() []byte {
return cc.AutoReply
}
+// SetAccount attaches the authenticated account to the client.
+func (cc *ClientConn) SetAccount(account *Account) {
+ cc.mu.Lock()
+ defer cc.mu.Unlock()
+
+ cc.Account = account
+}
+
+// GetAccount returns the client's account, or nil before login completes. Every account field
+// except Access is immutable after login, so callers may read them from the returned struct;
+// Access must be read via Authorize or AccessBytes because SetAccountAccess mutates it.
+func (cc *ClientConn) GetAccount() *Account {
+ cc.mu.RLock()
+ defer cc.mu.RUnlock()
+
+ return cc.Account
+}
+
+// SetAccountAccess replaces the access bitmap on the client's account.
+func (cc *ClientConn) SetAccountAccess(access AccessBitmap) {
+ cc.mu.Lock()
+ defer cc.mu.Unlock()
+
+ cc.Account.Access = access
+}
+
+// AccessBytes returns a copy of the account's access bitmap, suitable for use as a transaction
+// field. It returns nil before login completes.
+func (cc *ClientConn) AccessBytes() []byte {
+ cc.mu.RLock()
+ defer cc.mu.RUnlock()
+
+ if cc.Account == nil {
+ return nil
+ }
+ access := cc.Account.Access
+ return access[:]
+}
+
// incrementIdleTime adds interval seconds to the client's idle time. It returns true if this
// crossed the idle threshold and marked the client as away, in which case the caller should
// notify other clients of the change.
@@ -227,8 +266,8 @@ func (cc *ClientConn) clearIdleAndAway() bool {
}
func (cc *ClientConn) FileRoot() string {
- if cc.Account.FileRoot != "" {
- return cc.Account.FileRoot
+ if account := cc.GetAccount(); account != nil && account.FileRoot != "" {
+ return account.FileRoot
}
return cc.Server.Config.FileRoot
}
@@ -331,6 +370,9 @@ func (cc *ClientConn) Authenticate(login string, password []byte) bool {
// Authorize checks if the user account has the specified permission
func (cc *ClientConn) Authorize(access int) bool {
+ cc.mu.RLock()
+ defer cc.mu.RUnlock()
+
if cc.Account == nil {
return false
}
@@ -424,11 +466,12 @@ func formatDownloadList(fts []FileTransfer) (s string) {
}
func (cc *ClientConn) String() string {
+ account := cc.GetAccount()
template := fmt.Sprintf(
userInfoTemplate,
cc.GetUserName(),
- cc.Account.Name,
- cc.Account.Login,
+ account.Name,
+ account.Login,
cc.RemoteAddr,
formatDownloadList(cc.ClientFileTransferMgr.Get(FileDownload)),
formatDownloadList(cc.ClientFileTransferMgr.Get(FolderDownload)),
diff --git a/hotline/server.go b/hotline/server.go
index 14cee04..700cf19 100644
--- a/hotline/server.go
+++ b/hotline/server.go
@@ -512,8 +512,6 @@ func (s *Server) NewClientConn(conn io.ReadWriteCloser, remoteAddr string) *Clie
ClientFileTransferMgr: NewClientFileTransferMgr(),
}
- s.ClientMgr.Add(clientConn)
-
return clientConn
}
@@ -627,16 +625,17 @@ func (s *Server) handleNewConnection(ctx context.Context, rwc io.ReadWriteCloser
c.SetIcon(clientLogin.GetField(FieldUserIconID).Data)
}
- c.Account = c.Server.AccountManager.Get(login)
- if c.Account == nil {
+ account := c.Server.AccountManager.Get(login)
+ if account == nil {
return nil
}
+ c.SetAccount(account)
if clientLogin.GetField(FieldUserName).Data != nil {
if c.Authorize(AccessAnyName) {
c.SetUserName(clientLogin.GetField(FieldUserName).Data)
} else {
- c.SetUserName([]byte(c.Account.Name))
+ c.SetUserName([]byte(account.Name))
}
}
@@ -644,6 +643,11 @@ func (s *Server) handleNewConnection(ctx context.Context, rwc io.ReadWriteCloser
c.SetFlag(UserFlagAdmin, 1)
}
+ // Publish the client to the manager only now that its session state (Account, Version,
+ // UserName, Flags) is fully initialized. Other goroutines iterate ClientMgr.List() and
+ // dereference Account, so a client must never be visible before login completes.
+ s.ClientMgr.Add(c)
+
c.Send(c.NewReply(&clientLogin,
NewField(FieldVersion, []byte{0x00, 0xbe}),
NewField(FieldCommunityBannerID, []byte{0, 0}),
@@ -651,7 +655,7 @@ func (s *Server) handleNewConnection(ctx context.Context, rwc io.ReadWriteCloser
))
// Send user access privs so client UI knows how to behave
- c.Send(NewTransaction(TranUserAccess, c.ID, NewField(FieldUserAccess, c.Account.Access[:])))
+ c.Send(NewTransaction(TranUserAccess, c.ID, NewField(FieldUserAccess, c.AccessBytes())))
// Accounts with AccessNoAgreement do not receive the server agreement on login. The behavior is different between
// client versions. For 1.2.3 client, we do not send TranShowAgreement. For other client versions, we send
@@ -746,7 +750,7 @@ func (s *Server) handleFileTransfer(ctx context.Context, rwc io.ReadWriter) erro
}
rLogger := s.Logger.With(
"remoteAddr", remoteAddr,
- "login", fileTransfer.ClientConn.Account.Login,
+ "login", fileTransfer.ClientConn.GetAccount().Login,
"Name", string(fileTransfer.ClientConn.GetUserName()),
)
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}
diff --git a/internal/mobius/api.go b/internal/mobius/api.go
index 3620cf4..c14c8bf 100644
--- a/internal/mobius/api.go
+++ b/internal/mobius/api.go
@@ -110,7 +110,7 @@ func (srv *APIServer) OnlineHandler(w http.ResponseWriter, r *http.Request) {
} else {
for _, c := range srv.hlServer.ClientMgr.List() {
users = append(users, map[string]string{
- "login": string(c.Account.Login),
+ "login": c.GetAccount().Login,
"nickname": string(c.GetUserName()),
"ip": c.RemoteAddr,
})
@@ -164,7 +164,7 @@ func (srv *APIServer) BanHandler(w http.ResponseWriter, r *http.Request) {
// Disconnect user if online
for _, c := range srv.hlServer.ClientMgr.List() {
- if (req.Username != "" && c.Account.Login == req.Username) ||
+ if (req.Username != "" && c.GetAccount().Login == req.Username) ||
(req.Nickname != "" && string(c.GetUserName()) == req.Nickname) ||
(req.IP != "" && c.IP() == req.IP) {
c.Disconnect()
diff --git a/internal/mobius/handlers_accounts.go b/internal/mobius/handlers_accounts.go
index 02b7c77..5014d84 100644
--- a/internal/mobius/handlers_accounts.go
+++ b/internal/mobius/handlers_accounts.go
@@ -55,18 +55,18 @@ func HandleSetUser(cc *hotline.ClientConn, t *hotline.Transaction) (res []hotlin
// Notify connected clients logged in as the user of the new access level
for _, c := range cc.Server.ClientMgr.List() {
- if c.Account.Login == login {
+ if c.GetAccount().Login == login {
newT := hotline.NewTransaction(hotline.TranUserAccess, c.ID, hotline.NewField(hotline.FieldUserAccess, newAccessLvl))
res = append(res, newT)
+ c.SetAccountAccess(account.Access)
+
if c.Authorize(hotline.AccessDisconUser) {
c.SetFlag(hotline.UserFlagAdmin, 1)
} else {
c.SetFlag(hotline.UserFlagAdmin, 0)
}
- c.Account.Access = account.Access
-
cc.SendAll(
hotline.TranNotifyChangeUser,
hotline.NewField(hotline.FieldUserID, c.ID[:]),
@@ -200,7 +200,7 @@ func HandleUpdateUser(cc *hotline.ClientConn, t *hotline.Transaction) (res []hot
}
for _, client := range cc.Server.ClientMgr.List() {
- if client.Account.Login == login {
+ if client.GetAccount().Login == login {
// "You are logged in with an account which was deleted."
res = append(res,
@@ -374,7 +374,7 @@ func HandleDeleteUser(cc *hotline.ClientConn, t *hotline.Transaction) (res []hot
}
for _, client := range cc.Server.ClientMgr.List() {
- if client.Account.Login == login {
+ if client.GetAccount().Login == login {
res = append(res,
hotline.NewTransaction(hotline.TranServerMsg, client.ID,
hotline.NewField(hotline.FieldData, []byte(ErrMsgAccountDeleted)),
diff --git a/internal/mobius/handlers_chat.go b/internal/mobius/handlers_chat.go
index b81cfa5..ec92a7c 100644
--- a/internal/mobius/handlers_chat.go
+++ b/internal/mobius/handlers_chat.go
@@ -62,7 +62,7 @@ func HandleChatSend(cc *hotline.ClientConn, t *hotline.Transaction) (res []hotli
//cc.Server.mux.Lock()
for _, c := range cc.Server.ClientMgr.List() {
- if c == nil || cc.Account == nil {
+ if c == nil || cc.GetAccount() == nil {
continue
}
// Skip clients that do not have the read chat permission.
diff --git a/internal/mobius/handlers_session.go b/internal/mobius/handlers_session.go
index 443fed0..836c608 100644
--- a/internal/mobius/handlers_session.go
+++ b/internal/mobius/handlers_session.go
@@ -102,11 +102,11 @@ func HandleTranAgreed(cc *hotline.ClientConn, t *hotline.Transaction) (res []hot
if cc.Authorize(hotline.AccessAnyName) {
cc.SetUserName(t.GetField(hotline.FieldUserName).Data)
} else {
- cc.SetUserName([]byte(cc.Account.Name))
+ cc.SetUserName([]byte(cc.GetAccount().Name))
}
}
- login := cc.Account.Login
+ login := cc.GetAccount().Login
ip := cc.IP()
if cc.Server.Presence != nil {
@@ -191,7 +191,7 @@ func HandleDisconnectUser(cc *hotline.ClientConn, t *hotline.Transaction) (res [
}
if clientConn.Authorize(hotline.AccessCannotBeDiscon) {
- return cc.NewErrReply(t, clientConn.Account.Login+" is not allowed to be disconnected.")
+ return cc.NewErrReply(t, clientConn.GetAccount().Login+" is not allowed to be disconnected.")
}
// If FieldOptions is set, then the client IP is banned in addition to disconnected.
@@ -266,7 +266,7 @@ func HandleSetClientUserInfo(cc *hotline.ClientConn, t *hotline.Transaction) (re
newNickname := string(t.GetField(hotline.FieldUserName).Data)
cc.SetUserName(t.GetField(hotline.FieldUserName).Data)
- login := cc.Account.Login
+ login := cc.GetAccount().Login
ip := cc.IP()
if cc.Server.Presence != nil {
generated by cgit (git 2.54.0) at 2026-10-01 23:30:36 +0000