From b1353d904b8f9de89ae93365ea9006b792ec8ac7 Mon Sep 17 00:00:00 2001
From: m1ngsama <contact@m1ng.space>
Date: Sun, 17 May 2026 13:44:25 +0800
Subject: [PATCH] commands: reject :nick collisions with active clients (UX-3)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

:nick used to swap client->username unconditionally, so two clients
could both end up named "alice" — and the subsequent :msg / :w
disambiguation would just hit whichever came first in g_room->clients.

Now the rename happens under wrlock with an explicit scan: if any
*other* client already owns that username, the change is refused with

    Nickname 'alice' is already taken

If the requested name equals the current one, return a short
"Nickname unchanged" instead of broadcasting a no-op system message.

The room-lock-held scan is O(n) over current clients (n ≤ 64 by
default) and folds naturally into the existing wrlock, so there's no
new lock acquisition.
---
 src/commands.c | 44 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/src/commands.c b/src/commands.c
index 86d32e0..1fe3726 100644
--- a/src/commands.c
+++ b/src/commands.c
@@ -164,21 +164,47 @@ void commands_dispatch(client_t *client) {
                 utf8_truncate(validated_name, 20);
             }
 
+            /* Reject collisions with active room members.  Held under
+             * wrlock so the username swap below races neither read nor
+             * concurrent :nick from another client. */
             char old_name[MAX_USERNAME_LEN];
+            bool taken = false;
             pthread_rwlock_wrlock(&g_room->lock);
             snprintf(old_name, sizeof(old_name), "%s", client->username);
-            snprintf(client->username, MAX_USERNAME_LEN, "%s", validated_name);
+            if (strcmp(validated_name, old_name) != 0) {
+                for (int i = 0; i < g_room->client_count; i++) {
+                    if (g_room->clients[i] == client) continue;
+                    if (strcmp(g_room->clients[i]->username,
+                               validated_name) == 0) {
+                        taken = true;
+                        break;
+                    }
+                }
+            }
+            if (!taken) {
+                snprintf(client->username, MAX_USERNAME_LEN, "%s", validated_name);
+            }
             pthread_rwlock_unlock(&g_room->lock);
 
-            message_t nick_msg = { .timestamp = time(NULL) };
-            snprintf(nick_msg.username, MAX_USERNAME_LEN, "系统");
-            snprintf(nick_msg.content, MAX_MESSAGE_LEN,
-                     "%s 更名为 %s", old_name, client->username);
-            room_broadcast(g_room, &nick_msg);
-            message_save(&nick_msg);
+            if (taken) {
+                buffer_appendf(output, sizeof(output), &pos,
+                               "Nickname '%s' is already taken\n",
+                               validated_name);
+            } else if (strcmp(validated_name, old_name) == 0) {
+                buffer_appendf(output, sizeof(output), &pos,
+                               "Nickname unchanged\n");
+            } else {
+                message_t nick_msg = { .timestamp = time(NULL) };
+                snprintf(nick_msg.username, MAX_USERNAME_LEN, "系统");
+                snprintf(nick_msg.content, MAX_MESSAGE_LEN,
+                         "%s 更名为 %s", old_name, client->username);
+                room_broadcast(g_room, &nick_msg);
+                message_save(&nick_msg);
 
-            buffer_appendf(output, sizeof(output), &pos,
-                           "Nickname changed: %s -> %s\n", old_name, client->username);
+                buffer_appendf(output, sizeof(output), &pos,
+                               "Nickname changed: %s -> %s\n",
+                               old_name, client->username);
+            }
         }
 
     } else if (strncmp(cmd, "last", 4) == 0 && (cmd[4] == ' ' || cmd[4] == '\0')) {