fix connection handlers, group ops, and various HTTP handler bugs

2026-04-04 19:56:20 +02:00
parent e30a3077b1
commit 8f5e405532
4 changed files with 254 additions and 8 deletions
@@ -0,0 +1,35 @@
+# Bug List
+
+## Open
+
+### Bug 1 — `WsSendToUser`: sends message to sender, not recipient
+**File:** `wsServer.go:86`
+`to *User` parameter is never used — message is sent back to `from` instead.
+```go
+sendMessageCloseIfTimeout(from, &msg)  // should be `to`
+```
+
+### Bug 2 — `ServeWsConnection`: `ignoreCache` captured by value in defer
+**File:** `wsServer.go:30`
+`ignoreCache` is `false` at defer registration; setting it to `true` later has no effect.
+The user is always evicted from cache on disconnect.
+```go
+defer closeConnection(&user, ignoreCache)        // wrong
+defer func() { closeConnection(&user, ignoreCache) }()  // correct
+```
+
+### Bug 3 — Deadlock in `sendToAllMessageCloseIfTimeout`
+**File:** `wsServer.go:72`, `cache.go:52`
+`sendToAllMessageCloseIfTimeout` holds `mu.RLock()`, calls `sendMessageCloseIfTimeout`,
+which on timeout calls `closeConnection` → `CacheDeleteUser` → `mu.Lock()`.
+Upgrading RLock to write Lock deadlocks.
+
+### Bug 4 — `closeConnection`: nil `WsConn` not guarded
+**File:** `wsServer.go:179`
+`user.WsConn.CloseNow()` panics if `WsConn` is nil (e.g. unauthenticated user).
+`sendMessageCloseIfTimeout` guards against this but `closeConnection` does not.
+
+### Bug 5 — `HttpHandleUserAcceptConnection`: potential nil dereference
+**File:** `http.go:410`
+`target.Connections[user.Id].IsAccepted = true` panics if the entry is missing
+(DB inconsistency). The sender side is guarded but the recipient side is not.
@@ -38,9 +38,13 @@ func DbInit(ctx context.Context) {
 			requestor_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE,
 			recipient_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE,
 			is_accepted BOOLEAN NOT NULL DEFAULT FALSE,
-			created_at TIMESTAMP NOT NULL DEFAULT NOW()
+			created_at TIMESTAMP NOT NULL DEFAULT NOW(),
+			PRIMARY KEY (requestor_id, recipient_id)
 		)
 	`)
+	if err != nil {
+		panic(err)
+	}

 	_, err = dbConn.Exec(ctx, `
 		CREATE TABLE IF NOT EXISTS chat_groups (
@@ -184,6 +188,27 @@ func DbGroupSetColor(ctx context.Context, group *Group) error {
 	return err
 }

+func DbConnectionSave(ctx context.Context, creationDate time.Time, requestorId uint32, recipientId uint32, isAccepted bool) error {
+	_, err := dbConn.Exec(ctx, `
+		INSERT INTO user_connections (created_at, requestor_id, recipient_id, is_accepted) VALUES ($1, $2, $3, $4)
+	`, creationDate, requestorId, recipientId, isAccepted)
+	return err
+}
+
+func DbConnectionAccept(ctx context.Context, requestorId uint32, recipientId uint32) error {
+	_, err := dbConn.Exec(ctx, `
+		UPDATE user_connections SET is_accepted = true WHERE requestor_id = $1 AND recipient_id = $2
+	`, requestorId, recipientId)
+	return err
+}
+
+func DbConnectionDelete(ctx context.Context, requestorId uint32, recipientId uint32) error {
+	_, err := dbConn.Exec(ctx, `
+		DELETE FROM user_connections WHERE requestor_id = $1 AND recipient_id = $2
+	`, requestorId, recipientId)
+	return err
+}
+
 func DbGroupSave(ctx context.Context, group *Group) error {
 	err := dbConn.QueryRow(ctx, `
 		INSERT INTO chat_groups (name, creator_id, owner_id, enable_client_colors, color_red, color_green, color_blue, created_at)
@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	json2 "encoding/json"
+	"errors"
 	"fmt"
 	"maps"
 	"net/http"
@@ -89,7 +90,7 @@ func getIfOwnerUserAndGroup(ctx context.Context, response *http.ResponseWriter,

 	if !isOwner(user, group) {
 		http.Error(*response, "no such group", http.StatusUnauthorized)
-		return nil, nil, err
+		return nil, nil, errors.New("not an owner")
 	}
 	return user, group, nil
 }
@@ -153,6 +154,7 @@ func HttpHandleUserDelete(response http.ResponseWriter, request *http.Request) {
 	err = DbUserDelete(ctx, userId)
 	if err != nil {
 		http.Error(response, "internal server error", http.StatusInternalServerError)
+		return
 	}

 	CacheDeleteUser(userId)
@@ -192,7 +194,7 @@ func HttpHandleUserModifyAbout(response http.ResponseWriter, request *http.Reque
 	}

 	pronouns := request.FormValue("pronouns")
-	if len(pronouns) > 25 && len(pronouns) < 2 {
+	if len(pronouns) > 25 || len(pronouns) < 2 {
 		http.Error(response, "invalid pronouns", http.StatusBadRequest)
 		return
 	}
@@ -226,10 +228,16 @@ func HttpHandleUserMessage(response http.ResponseWriter, request *http.Request)
 		err = DbUserGetById(ctx, target)
 		if err != nil {
 			http.Error(response, "invalid recipient id", http.StatusBadRequest)
+			return
+		}
+		err = DbUserGetConnections(ctx, target)
+		if err != nil {
+			http.Error(response, "invalid recipient id", http.StatusBadRequest)
+			return
 		}
 	}

-	if user.Connections[target.Id] == nil {
+	if user.Connections[target.Id] == nil || !user.Connections[targetId].IsAccepted {
 		http.Error(response, "invalid recipient id", http.StatusBadRequest)
 		return
 	}
@@ -243,7 +251,176 @@ func HttpHandleUserMessage(response http.ResponseWriter, request *http.Request)
 	WsSendToUser(user, target, message)
 }

-func HttpHandleNewToken(response http.ResponseWriter, request *http.Request) {
+func HttpHandleUserNewConnection(response http.ResponseWriter, request *http.Request) {
+	if !isMethodAllowed(&response, request) {
+		return
+	}
+
+	ctx := request.Context()
+
+	user, err := getUser(ctx, request.FormValue("token"))
+	if err != nil {
+		http.Error(response, "invalid token", http.StatusUnauthorized)
+		return
+	}
+
+	targetId, err := ConvertStringUint32(request.FormValue("recipientid"))
+	if err != nil {
+		http.Error(response, "invalid recipient id", http.StatusBadRequest)
+		return
+	}
+
+	if user.Id == targetId {
+		http.Error(response, "invalid recipient id", http.StatusBadRequest)
+		return
+	}
+
+	target, err := CacheGetUserById(targetId)
+	if err != nil {
+		target = &User{Id: targetId}
+		err = DbUserGetById(ctx, target)
+		if err != nil {
+			http.Error(response, "invalid recipient id", http.StatusBadRequest)
+			return
+		}
+		err = DbUserGetConnections(ctx, target)
+		if err != nil {
+			http.Error(response, "invalid recipient id", http.StatusBadRequest)
+			return
+		}
+	}
+	if user.Connections[target.Id] != nil {
+		http.Error(response, "already sent/connected", http.StatusConflict)
+		return
+	}
+
+	timeNow := time.Now()
+
+	err = DbConnectionSave(ctx, timeNow, user.Id, targetId, false)
+	if err != nil {
+		http.Error(response, "internal server error", http.StatusInternalServerError)
+		return
+	}
+
+	user.Connections[target.Id] = &Connection{
+		CreatedAt:  timeNow,
+		With:       targetId,
+		IsFromUser: true,
+		IsAccepted: false,
+	}
+	if target.Connections == nil {
+		target.Connections = make(map[uint32]*Connection)
+	}
+	target.Connections[user.Id] = &Connection{
+		CreatedAt:  timeNow,
+		With:       user.Id,
+		IsFromUser: false,
+		IsAccepted: false,
+	}
+
+	response.WriteHeader(http.StatusCreated)
+}
+
+func HttpHandleUserDeleteConnection(response http.ResponseWriter, request *http.Request) {
+	ctx := request.Context()
+
+	user, err := getUser(ctx, request.FormValue("token"))
+	if err != nil {
+		http.Error(response, "invalid token", http.StatusUnauthorized)
+		return
+	}
+
+	targetId, err := ConvertStringUint32(request.FormValue("connectedid"))
+	if err != nil {
+		http.Error(response, "invalid recipient id", http.StatusBadRequest)
+		return
+	}
+
+	target, err := CacheGetUserById(targetId)
+	if err != nil {
+		target = &User{Id: targetId}
+		err = DbUserGetById(ctx, target)
+		if err != nil {
+			http.Error(response, "invalid recipient id", http.StatusBadRequest)
+			return
+		}
+		err = DbUserGetConnections(ctx, target)
+		if err != nil {
+			http.Error(response, "invalid recipient id", http.StatusBadRequest)
+			return
+		}
+	}
+	if user.Connections[targetId] == nil {
+		http.Error(response, "invalid recipient id", http.StatusBadRequest)
+		return
+	}
+
+	if user.Connections[targetId].IsFromUser {
+		err = DbConnectionDelete(ctx, user.Id, targetId)
+	} else {
+		err = DbConnectionDelete(ctx, targetId, user.Id)
+	}
+	if err != nil {
+		http.Error(response, "internal server error", http.StatusInternalServerError)
+		return
+	}
+
+	delete(user.Connections, targetId)
+	delete(target.Connections, user.Id)
+
+	response.WriteHeader(http.StatusAccepted)
+}
+
+func HttpHandleUserAcceptConnection(response http.ResponseWriter, request *http.Request) {
+	ctx := request.Context()
+
+	user, err := getUser(ctx, request.FormValue("token"))
+	if err != nil {
+		http.Error(response, "invalid token", http.StatusUnauthorized)
+		return
+	}
+
+	targetId, err := ConvertStringUint32(request.FormValue("connectedid"))
+	if err != nil {
+		http.Error(response, "invalid recipient id", http.StatusBadRequest)
+		return
+	}
+
+	target, err := CacheGetUserById(targetId)
+	if err != nil {
+		target = &User{Id: targetId}
+		err = DbUserGetById(ctx, target)
+		if err != nil {
+			http.Error(response, "invalid recipient id", http.StatusBadRequest)
+			return
+		}
+		err = DbUserGetConnections(ctx, target)
+		if err != nil {
+			http.Error(response, "invalid recipient id", http.StatusBadRequest)
+			return
+		}
+	}
+	if user.Connections[targetId] == nil {
+		http.Error(response, "invalid recipient id", http.StatusBadRequest)
+		return
+	}
+	if user.Connections[targetId].IsFromUser {
+		http.Error(response, "cant accept own request", http.StatusConflict)
+		return
+	}
+
+	user.Connections[targetId].IsAccepted = true
+	target.Connections[user.Id].IsAccepted = true
+
+	err = DbConnectionAccept(ctx, targetId, user.Id)
+	if err != nil {
+		http.Error(response, "internal server error", http.StatusInternalServerError)
+		return
+	}
+	response.WriteHeader(http.StatusAccepted)
+}
+
+func HttpHandleTokenNew(response http.ResponseWriter, request *http.Request) {
 	if !isMethodAllowed(&response, request) {
 		return
 	}
@@ -321,6 +498,10 @@ func HttpHandeGroupCreate(response http.ResponseWriter, request *http.Request) {

 	colorString := request.FormValue("color")
 	color, err := ConvertStringToRgb(colorString)
+	if err != nil {
+		http.Error(response, "invalid color", http.StatusBadRequest)
+		return
+	}

 	group := Group{
 		Name:      name,
@@ -503,7 +684,7 @@ func HttpHandleGroupChangeOwner(response http.ResponseWriter, request *http.Requ
 	}

 	ctx := request.Context()
-	user, group, err := getIfOwnerUserAndGroup(ctx, &response, request)
+	_, group, err := getIfOwnerUserAndGroup(ctx, &response, request)
 	if err != nil {
 		return
 	}
@@ -519,7 +700,7 @@ func HttpHandleGroupChangeOwner(response http.ResponseWriter, request *http.Requ
 			return
 		}

-		CacheSaveUser(user)
+		CacheSaveUser(newOwner)
 	}

 	_, ok := group.Users[newOwner.Id]
@@ -558,6 +739,10 @@ func HttpHandleGroupMessage(response http.ResponseWriter, request *http.Request)
 	}

 	group, err := getGroup(ctx, groupId)
+	if err != nil {
+		http.Error(response, "no such group", http.StatusUnauthorized)
+		return
+	}

 	content := request.FormValue("content")
 	if content == "" {
@@ -568,6 +753,7 @@ func HttpHandleGroupMessage(response http.ResponseWriter, request *http.Request)
 	_, ok := group.Users[user.Id]
 	if !ok {
 		http.Error(response, "no such group", http.StatusUnauthorized)
+		return
 	}

 	err = WsSendToGroup(group, user, content)
@@ -18,7 +18,7 @@ func main() {
 	DbInit(ctx)

 	http.HandleFunc("/new/user", withCORS(HttpHandleUserNew))
-	http.HandleFunc("/new/token", withCORS(HttpHandleNewToken))
+	http.HandleFunc("/new/token", withCORS(HttpHandleTokenNew))
 	http.HandleFunc("/new/group", withCORS(HttpHandeGroupCreate))

 	http.HandleFunc("/mod/user/appearence", withCORS(HttpHandleUserModifyAppearance))