bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict

["J.P." <jp@neverwas.me>]

[-- Attachment #1: Type: text/plain, Size: 1827 bytes --]

"J.P." <jp@neverwas.me> writes:

>>> Unfortunately, the best we can do for ERC 5.5 (Emacs 29) is to mention
>>> somewhere, like in (info "(erc) Network Identifier"), that users really
>>> worried about this issue should choose an `:id' containing characters
>>> disallowed in nicks by their network (or just something improbable and
>>> unlikely to be guessed). But, hopefully, we can address this in a more
>>> DWIM-like fashion in an upcoming ELPA release, such as ERC 5.6.
>>
>> Hopefully won't be an issue for most people, although it might be not
>> that uncommon for this kind of service/bot/proxy ircd's to also use
>> their name for messaging user with any kind of service-related info/issues.
>
> Hm, right. That's good to know. I think the key for now is to make
> people aware that they should assign an ID or modify
> `erc-networks-alist' if they find themselves in a similar boat. Most
> folks just connecting to a public network should be safe because those
> NETWORKs are mostly titlecase and/or contain a dot. But, in the long
> run, I'd definitely like the default behavior to account for this
> possibility.

Actually, an egregious oversight has come to light that makes this a
more general (and more pressing) matter relevant to Emacs 29. Currently,
renaming queries fails if *any* buffer, even a non-ERC buffer, exists
whose name matches that of the target in question. And, AFAICT, no
amount of :id or options twiddling can serve as a workaround. (If this
is what you've been getting at this whole time, then apologies: I'm
rather thick headed, if you haven't noticed.)

Anyhow, I have attempted to address this in the attached patch. If you
or anyone out there is willing, please install it locally atop the
current lisp/erc subtree on the emacs-29 branch and see if you can break
it. Thanks in advance!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-some-naming-issues-involving-query-buffers-in-ER.patch --]
[-- Type: text/x-patch, Size: 15824 bytes --]

From 22a90166a40f6f333621c05839e1b7a779ed3813 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 12 Dec 2022 07:38:44 -0800
Subject: [PATCH] Fix some naming issues involving query buffers in ERC

* lisp/erc/erc-networks.el
(erc-networks-rename-surviving-target-buffer): Don't kill a surviving
query buffer when another non-target buffer, possibly not even
belonging to ERC, already exists with the target name.
(erc-networks--reconcile-buffer-names): Always append a network-ID
suffix to a target buffer's name if another buffer of that name
already exists.  (Bug#59976.)
* test/lisp/erc/erc-networks-tests.el:
(erc-networks-rename-surviving-target-buffer--query-non-target): Add
new test.
* test/lisp/erc/erc-scenarios-base-association-query.el: New file.
* test/lisp/erc/resources/base/assoc/queries/netnick.eld: New file.
* test/lisp/erc/resources/base/assoc/queries/non-erc.eld: New file.
---
 lisp/erc/erc-networks.el                      |  24 ++--
 test/lisp/erc/erc-networks-tests.el           |  30 +++++
 .../erc-scenarios-base-association-query.el   | 107 ++++++++++++++++++
 .../resources/base/assoc/queries/netnick.eld  |  43 +++++++
 .../resources/base/assoc/queries/non-erc.eld  |  34 ++++++
 5 files changed, 227 insertions(+), 11 deletions(-)
 create mode 100644 test/lisp/erc/erc-scenarios-base-association-query.el
 create mode 100644 test/lisp/erc/resources/base/assoc/queries/netnick.eld
 create mode 100644 test/lisp/erc/resources/base/assoc/queries/non-erc.eld

diff --git a/lisp/erc/erc-networks.el b/lisp/erc/erc-networks.el
index fd8bed470a..30a72a5c6a 100644
--- a/lisp/erc/erc-networks.el
+++ b/lisp/erc/erc-networks.el
@@ -1097,7 +1097,8 @@ erc-networks-rename-surviving-target-buffer
                                   (erc--target-symbol erc--target))))))))
        ((not (cdr others))))
     (with-current-buffer (car others)
-      (rename-buffer (erc--target-string target)))))
+      (unless (get-buffer (erc--target-string target))
+        (rename-buffer (erc--target-string target))))))
 
 (defun erc-networks-shrink-ids-and-buffer-names ()
   "Recompute network IDs and buffer names, ignoring the current buffer.
@@ -1188,19 +1189,20 @@ erc-networks--reconcile-buffer-names
                  (erc--target-string target)))
          placeholder)
     ;; If we don't exist, claim name temporarily while renaming others
-    (when-let* (namesakes
-                (ex (get-buffer name))
-                ((not (memq ex existing)))
-                (temp-name (generate-new-buffer-name (format "*%s*" name))))
-      (setq existing (remq ex existing))
-      (with-current-buffer ex
-        (rename-buffer temp-name)
-        (setq placeholder (get-buffer-create name))
-        (rename-buffer name 'unique)))
+    (when-let* ((ex (get-buffer name))
+                ((not (memq ex existing))))
+      (if namesakes ; if namesakes is nonempty, it contains ex
+          (with-current-buffer ex
+            (let ((temp-name (generate-new-buffer-name (format "*%s*" name))))
+              (rename-buffer temp-name)
+              (setq placeholder (get-buffer-create name))
+              (rename-buffer name 'unique)))
+        ;; This must be a server buffer or a non-ERC buffer
+        (setq name (erc-networks--construct-target-buffer-name target))))
     (unless (with-suppressed-warnings ((obsolete erc-reuse-buffers))
               erc-reuse-buffers)
       (when (string-suffix-p ">" name)
-        (setq name (substring name 0 -3))))
+        (setq name (string-trim-right name (rx "<" (+ digit) ">")))))
     (dolist (ex (erc-networks--id-sort-buffers existing))
       (with-current-buffer ex
         (rename-buffer name 'unique)))
diff --git a/test/lisp/erc/erc-networks-tests.el b/test/lisp/erc/erc-networks-tests.el
index e883174e28..0a8b5935df 100644
--- a/test/lisp/erc/erc-networks-tests.el
+++ b/test/lisp/erc/erc-networks-tests.el
@@ -197,6 +197,36 @@ erc-networks-rename-surviving-target-buffer--query
 
   (erc-networks-tests--clean-bufs))
 
+;; A non-ERC buffer exists named "bob", and we're killing one of two
+;; ERC target buffers named "bob@<netid>".  The surviving buffer
+;; retains its suffix.
+
+(ert-deftest erc-networks-rename-surviving-target-buffer--query-non-target ()
+  (should (memq #'erc-networks-rename-surviving-target-buffer
+                erc-kill-buffer-hook))
+
+  (let ((existing (get-buffer-create "bob"))
+        (bob-foonet (get-buffer-create "bob@foonet")))
+
+    (with-current-buffer bob-foonet
+      (erc-mode)
+      (setq erc-networks--id (make-erc-networks--id-qualifying
+                              :parts [foonet "bob"] :len 1)
+            erc--target (erc--target-from-string "bob")))
+
+    (with-current-buffer (get-buffer-create "bob@barnet")
+      (erc-mode)
+      (setq erc-networks--id (make-erc-networks--id-qualifying
+                              :parts [barnet "bob"] :len 1)
+            erc--target (erc--target-from-string "bob")))
+
+    (kill-buffer "bob@barnet")
+    (should (buffer-live-p existing))
+    (should (buffer-live-p bob-foonet))
+    (kill-buffer existing))
+
+  (erc-networks-tests--clean-bufs))
+
 (ert-deftest erc-networks-rename-surviving-target-buffer--multi ()
 
   (ert-info ("Multiple leftover channels untouched")
diff --git a/test/lisp/erc/erc-scenarios-base-association-query.el b/test/lisp/erc/erc-scenarios-base-association-query.el
new file mode 100644
index 0000000000..78b75a530c
--- /dev/null
+++ b/test/lisp/erc/erc-scenarios-base-association-query.el
@@ -0,0 +1,107 @@
+;;; erc-scenarios-base-association-query.el --- assoc query scenarios -*- lexical-binding: t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert-x)
+(eval-and-compile
+  (let ((load-path (cons (ert-resource-directory) load-path)))
+    (require 'erc-scenarios-common)))
+
+(eval-when-compile (require 'erc-join))
+
+
+;; Non-ERC buffers exist whose names match the nicknames of query
+;; targets, both newly arriving and outgoing.  No target buffers yet
+;; exist for these, so new ones are created that feature a net-ID
+;; @suffix.
+
+(ert-deftest erc-scenarios-base-association-existing-non-erc-buffer ()
+  :tags '(:expensive-test)
+  (erc-scenarios-common-with-cleanup
+      ((erc-scenarios-common-dialog "base/assoc/queries")
+       (dumb-server (erc-d-run "localhost" t 'non-erc))
+       (port (process-contact dumb-server :service))
+       (expect (erc-d-t-make-expecter))
+       (nitwit (with-current-buffer (get-buffer-create "nitwit")
+                 (prin1 (ert-test-name (ert-running-test)) (current-buffer))
+                 (current-buffer))) ; these are killed on completion by macro
+       (dummy (with-current-buffer (get-buffer-create "dummy")
+                (prin1 (ert-test-name (ert-running-test)) (current-buffer))
+                (current-buffer)))
+       (erc-server-flood-penalty 0.1))
+
+    (ert-info ("Connect to foonet")
+      (with-current-buffer (erc :server "127.0.0.1"
+                                :port port
+                                :nick "tester"
+                                :user "tester"
+                                :full-name "tester")
+        (erc-scenarios-common-assert-initial-buf-name nil port)
+        (erc-d-t-wait-for 5 (eq erc-network 'foonet))
+        (funcall expect 15 "debug mode")))
+
+    (ert-info ("Nick dummy queries us")
+      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "dummy@foonet"))
+        (should (erc-query-buffer-p))
+        (funcall expect 5 "hi")
+
+        (ert-info ("We query nick nitwit")
+          (with-current-buffer (erc-cmd-QUERY "nitwit")
+            (should (equal (buffer-name) "nitwit@foonet"))
+            (erc-scenarios-common-say "hola")
+            (funcall expect 5 "ciao")))
+
+        (erc-scenarios-common-say "howdy")
+        (funcall expect 5 "bye")
+        (erc-cmd-QUIT "")))))
+
+;; Someone sending you a PM has the same name as the network (bug#59976)
+
+(ert-deftest erc-scenarios-base-association-some-nick-is-network ()
+  :tags '(:expensive-test)
+  (erc-scenarios-common-with-cleanup
+      ((erc-scenarios-common-dialog "base/assoc/queries")
+       (dumb-server (erc-d-run "localhost" t 'netnick))
+       (port (process-contact dumb-server :service))
+       (expect (erc-d-t-make-expecter))
+       (erc-server-flood-penalty 0.5))
+
+    (ert-info ("Connect to foonet")
+      (with-current-buffer (erc :server "127.0.0.1"
+                                :port port
+                                :nick "tester"
+                                :user "tester"
+                                :full-name "tester")
+        (erc-scenarios-common-assert-initial-buf-name nil port)
+        (erc-d-t-wait-for 5 (eq erc-network 'foonet))))
+
+    (ert-info ("Join common channel as nick foonet")
+      (with-current-buffer "foonet"
+        (funcall expect 15 "debug mode")
+        (erc-cmd-JOIN "#chan"))
+      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
+        (funcall expect 5 "welcome")))
+
+    (ert-info ("Nick foonet PMs us")
+      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "foonet@foonet"))
+        (should (erc-query-buffer-p))
+        (funcall expect 5 "hi")))))
+
+;;; erc-scenarios-base-association-query.el ends here
diff --git a/test/lisp/erc/resources/base/assoc/queries/netnick.eld b/test/lisp/erc/resources/base/assoc/queries/netnick.eld
new file mode 100644
index 0000000000..9493a7c057
--- /dev/null
+++ b/test/lisp/erc/resources/base/assoc/queries/netnick.eld
@@ -0,0 +1,43 @@
+;; -*- mode: lisp-data; -*-
+((nick 10 "NICK tester"))
+((user 1 "USER tester 0 * :tester")
+ (0.00 ":irc.foonet.org 001 tester :Welcome to the foonet IRC Network tester")
+ (0.00 ":irc.foonet.org 002 tester :Your host is irc.foonet.org, running version ergo-v2.8.0")
+ (0.00 ":irc.foonet.org 003 tester :This server was created Mon, 12 Dec 2022 01:25:38 UTC")
+ (0.00 ":irc.foonet.org 004 tester irc.foonet.org ergo-v2.8.0 BERTZios CEIMRUabefhiklmnoqstuv Iabefhkloqv")
+ (0.00 ":irc.foonet.org 005 tester AWAYLEN=390 BOT=B CASEMAPPING=ascii CHANLIMIT=#:100 CHANMODES=Ibe,k,fl,CEMRUimnstu CHANNELLEN=64 CHANTYPES=# ELIST=U EXCEPTS EXTBAN=,m FORWARD=f INVEX KICKLEN=390 :are supported by this server")
+ (0.00 ":irc.foonet.org 005 tester MAXLIST=beI:60 MAXTARGETS=4 MODES MONITOR=100 NETWORK=foonet NICKLEN=32 PREFIX=(qaohv)~&@%+ STATUSMSG=~&@%+ TARGMAX=NAMES:1,LIST:1,KICK:,WHOIS:1,USERHOST:10,PRIVMSG:4,TAGMSG:4,NOTICE:4,MONITOR:100 TOPICLEN=390 UTF8MAPPING=rfc8265 UTF8ONLY WHOX :are supported by this server")
+ (0.01 ":irc.foonet.org 005 tester draft/CHATHISTORY=100 :are supported by this server")
+ (0.00 ":irc.foonet.org 251 tester :There are 0 users and 4 invisible on 1 server(s)")
+ (0.00 ":irc.foonet.org 252 tester 0 :IRC Operators online")
+ (0.00 ":irc.foonet.org 253 tester 0 :unregistered connections")
+ (0.00 ":irc.foonet.org 254 tester 1 :channels formed")
+ (0.00 ":irc.foonet.org 255 tester :I have 4 clients and 0 servers")
+ (0.00 ":irc.foonet.org 265 tester 4 4 :Current local users 4, max 4")
+ (0.01 ":irc.foonet.org 266 tester 4 4 :Current global users 4, max 4")
+ (0.00 ":irc.foonet.org 422 tester :MOTD File is missing"))
+
+((mode 10 "MODE tester +i")
+ (0.00 ":irc.foonet.org 221 tester +i")
+ (0.00 ":irc.foonet.org NOTICE tester :This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect.")
+ (0.02 ":irc.foonet.org 221 tester +i"))
+
+((join 10 "JOIN #chan")
+ (0.03 ":tester!~u@z5d6jyn8pwxge.irc JOIN #chan"))
+
+((mode-1 10 "MODE #chan")
+ (0.01 ":irc.foonet.org 353 tester = #chan :@alice bob foonet tester")
+ (0.00 ":irc.foonet.org 366 tester #chan :End of NAMES list")
+ (0.03 ":irc.foonet.org 324 tester #chan +nt")
+ (0.00 ":irc.foonet.org 329 tester #chan 1670808354")
+ (0.00 ":bob!~u@d6ftaiqzk8x2k.irc PRIVMSG #chan :tester, welcome!")
+ (0.00 ":alice!~u@d6ftaiqzk8x2k.irc PRIVMSG #chan :tester, welcome!")
+
+ (0.00 ":foonet!~u@z5d6jyn8pwxge.irc PRIVMSG tester :hi")
+
+ (0.03 ":bob!~u@d6ftaiqzk8x2k.irc PRIVMSG #chan :alice: Forbear it therefore; give your cause to heaven.")
+ (0.01 ":alice!~u@d6ftaiqzk8x2k.irc PRIVMSG #chan :bob: Even at thy teat thou hadst thy tyranny.")
+ (0.00 ":foonet!~u@z5d6jyn8pwxge.irc QUIT :connection closed"))
+
+((quit 10 "QUIT :\2ERC\2")
+ (0.03 ":tester!~u@z5d6jyn8pwxge.irc QUIT :Quit: \2ERC\2"))
diff --git a/test/lisp/erc/resources/base/assoc/queries/non-erc.eld b/test/lisp/erc/resources/base/assoc/queries/non-erc.eld
new file mode 100644
index 0000000000..69b9ac8f69
--- /dev/null
+++ b/test/lisp/erc/resources/base/assoc/queries/non-erc.eld
@@ -0,0 +1,34 @@
+;; -*- mode: lisp-data; -*-
+((nick 10 "NICK tester"))
+((user 1 "USER tester 0 * :tester")
+ (0.00 ":irc.foonet.org 001 tester :Welcome to the foonet IRC Network tester")
+ (0.00 ":irc.foonet.org 002 tester :Your host is irc.foonet.org, running version ergo-v2.8.0")
+ (0.00 ":irc.foonet.org 003 tester :This server was created Mon, 12 Dec 2022 01:25:38 UTC")
+ (0.00 ":irc.foonet.org 004 tester irc.foonet.org ergo-v2.8.0 BERTZios CEIMRUabefhiklmnoqstuv Iabefhkloqv")
+ (0.00 ":irc.foonet.org 005 tester AWAYLEN=390 BOT=B CASEMAPPING=ascii CHANLIMIT=#:100 CHANMODES=Ibe,k,fl,CEMRUimnstu CHANNELLEN=64 CHANTYPES=# ELIST=U EXCEPTS EXTBAN=,m FORWARD=f INVEX KICKLEN=390 :are supported by this server")
+ (0.00 ":irc.foonet.org 005 tester MAXLIST=beI:60 MAXTARGETS=4 MODES MONITOR=100 NETWORK=foonet NICKLEN=32 PREFIX=(qaohv)~&@%+ STATUSMSG=~&@%+ TARGMAX=NAMES:1,LIST:1,KICK:,WHOIS:1,USERHOST:10,PRIVMSG:4,TAGMSG:4,NOTICE:4,MONITOR:100 TOPICLEN=390 UTF8MAPPING=rfc8265 UTF8ONLY WHOX :are supported by this server")
+ (0.01 ":irc.foonet.org 005 tester draft/CHATHISTORY=100 :are supported by this server")
+ (0.00 ":irc.foonet.org 251 tester :There are 0 users and 4 invisible on 1 server(s)")
+ (0.00 ":irc.foonet.org 252 tester 0 :IRC Operators online")
+ (0.00 ":irc.foonet.org 253 tester 0 :unregistered connections")
+ (0.00 ":irc.foonet.org 254 tester 1 :channels formed")
+ (0.00 ":irc.foonet.org 255 tester :I have 4 clients and 0 servers")
+ (0.00 ":irc.foonet.org 265 tester 4 4 :Current local users 4, max 4")
+ (0.01 ":irc.foonet.org 266 tester 4 4 :Current global users 4, max 4")
+ (0.00 ":irc.foonet.org 422 tester :MOTD File is missing"))
+
+((mode 10 "MODE tester +i")
+ (0.00 ":irc.foonet.org 221 tester +i")
+ (0.00 ":irc.foonet.org NOTICE tester :This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect.")
+ (0.02 ":irc.foonet.org 221 tester +i")
+ (0.00 ":dummy!~u@z5d6jyn8pwxge.irc PRIVMSG tester :hi"))
+
+((~privmsg-open 10 "PRIVMSG nitwit :hola")
+ (0.00 ":nitwit!~u@m5q6wla8cjktr.irc PRIVMSG tester :ciao"))
+
+((privmsg 10 "PRIVMSG dummy :howdy")
+ (0.00 ":dummy!~u@z5d6jyn8pwxge.irc PRIVMSG tester :bye")
+ (0.01 ":dummy!~u@z5d6jyn8pwxge.irc QUIT :connection closed"))
+
+((quit 10 "QUIT :\2ERC\2")
+ (0.03 ":tester!~u@z5d6jyn8pwxge.irc QUIT :Quit: \2ERC\2"))
-- 
2.38.1