bug#45872: 27.1; rcirc nick tracking

bug#45872: 27.1; rcirc nick tracking

[Ken Raeburn]

One of my IRC contacts uses frequent nick changes to indicate away
status, e.g., "johnsmith" when available, "johnsmith|away",
"johnsmith|vacation", whatever. I've noticed that sometimes rcirc will
fail to rename the buffer used for private messages between us, and so
I'll wind up with a buffer "johnsmith|away" showing something along the
lines of:

  ... *** johnsmith NICK johnsmith|away
  ... *** johnsmith|away NICK johnsmith

but the buffer won't have been renamed back to "johnsmith@<server>", and
if I try sending him a message, it'll try sending to johnsmith|away and
will fail. I can use "/msg johnsmith...", or he can send messages to me,
and a new buffer will be created.

If, after this new buffer has been created, he issues a NICK command,
rcirc will attempt a rename, and may error out if it conflicts with the
existing johnsmith|away buffer.

The issue seems to be that rcirc-handler-NICK looks up the chat buffer
using rcirc-get-buffer, which uses rcirc-buffer-alist, but doesn't
update that alist, so a second invocation of rcirc-get-buffer using the
new nick won't find that buffer.

But fixing the assoc list won't address buffer renaming conflicts caused
by nick changes happening while I'm offline; if rcirc renames a buffer
to "johnsmith|away@server" and then I get disconnected, and when I
reconnect I see "johnsmith" active and start exchanging messages (in a
new buffer), and later I receive a NICK message causing another rename
to "johnsmith|away@server", it'll conflict with the existing one. If the
server isn't authenticating, we can't guarantee that the old
"johnsmith|away" and the new "johnsmith" really are the same user, so
I'm not sure if we should blithely try combining them or make them
unique; erroring out during a rename is an ugly failure mode though.




In GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw scroll bars)
 of 2020-11-10 built on crash
Windowing system distributor 'Fedora Project', version 11.0.12006000
System Description: Fedora 31 (Workstation Edition)

Recent messages:
Mark set [2 times]
Quit
Mark set
Quit
Mark set [2 times]
Saving file /home/raeburn/.private/notes.org...
Wrote /home/raeburn/.private/notes.org
GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw scroll bars) of 2020-11-10

Configured using:
 'configure --prefix=/home/raeburn/dev/emacs/Install-20201110T0556
 --with-x-toolkit=lucid'

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND GPM DBUS GSETTINGS GLIB NOTIFY INOTIFY
LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ XFT ZLIB TOOLKIT_SCROLL_BARS
LUCID X11 XDBE XIM MODULES THREADS LIBSYSTEMD JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: IELM

Minor modes in effect:
  async-bytecomp-package-mode: t
  shell-dirtrack-mode: t
  rcirc-track-minor-mode: t
  display-time-mode: t
  desktop-save-mode: t
  global-edit-server-edit-mode: t
  which-function-mode: t
  icomplete-mode: t
  global-hi-lock-mode: t
  hi-lock-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tab-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/raeburn/elisp/with-editor hides /home/raeburn/.emacs.d/elpa/with-editor-20181113.1845/with-editor

Features:
(shadow emacsbug flyspell gnus-icalendar icalendar diary-lib
diary-loaddefs novice systemtap-mode cc-awk apropos descr-text dired-aux
mailalias smtpmail sendmail thai-util thai-word ispell pcmpl-unix
pcmpl-gnu cc-langs completion gnus-eform etags fileloop xref project
tramp-cmds follow flow-fill edmacro kmacro rect ibuf-ext ibuffer
ibuffer-loaddefs term/xterm xterm cl-print ielm warnings nroff-mode
eieio-opt speedbar sb-image ezimage dframe bookmark tabify timezone
org-protocol pp help-fns radix-tree cus-edit org-capture grep url-http
url-gw url-auth url-cache shr-color color mm-archive gnus-topic sort
smiley gnus-cite mail-extr gnus-async gnus-bcklg qp gnus-ml mule-util
cal-move with-editor async-bytecomp async ruby-mode misearch
multi-isearch nndraft nnmh nnfolder utf-7 gnutls gnus-agent gnus-srvr
gnus-score score-mode nnvirtual gnus-msg nntp gnus-cache tramp-cache
tramp-sh vagrant-tramp dash tramp tramp-loaddefs trampver
tramp-integration files-x tramp-compat ls-lisp network-stream nsm
add-log face-remap conf-mode adaptive-wrap make-mode yaml-mode sh-script
smie executable eww mm-url thingatpt url-queue cperl-mode smerge-mode
diff perl-mode rst compile objdump vc-git diff-mode cc-mode cc-fonts
cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs
cl-extra help-mode org-element avl-tree generator ol-eww ol-rmail ol-mhe
ol-irc ol-info ol-gnus nnir ol-docview doc-view jka-compr image-mode
exif ol-bibtex bibtex ol-bbdb ol-w3m org org-macro org-footnote
org-pcomplete org-list org-faces org-entities noutline outline
org-version ob-shell shell pcomplete ob ob-tangle org-src ob-ref ob-lob
ob-table ob-exp ob-comint ob-emacs-lisp ob-core ob-eval org-table ol
org-keys org-compat org-macs org-loaddefs find-func cal-menu calendar
cal-loaddefs rcirc gnus-art mm-uu mml2015 mm-view mml-smime smime dig
gnus-sum url url-proxy url-privacy url-expand url-methods url-history
mailcap shr url-cookie url-domsuf url-util svg xml dom gnus-group
gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source utf7 netrc
nnoo parse-time iso8601 gnus-spec gnus-int gnus-range message rmc puny
dired dired-loaddefs format-spec rfc822 mml mml-sec epa derived epg
epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader gnus-win gnus nnheader gnus-util rmail
rmail-loaddefs rfc2047 rfc2045 ietf-drums text-property-search time-date
mail-utils mm-util mail-prsvr wid-edit time desktop frameset cus-start
cus-load kr-init edit-server iso-transl advice smart-quotes easy-mmode
which-func imenu icomplete server term disp-table comint ansi-color
ehelp ring hi-lock finder-inf info package easymenu browse-url
url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json subr-x map url-vars seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 40581826 2812970)
 (symbols 48 52910 12)
 (strings 32 1258102 286070)
 (string-bytes 1 37999133)
 (vectors 16 91413)
 (vector-slots 8 2960172 1274366)
 (floats 8 689 867)
 (intervals 56 4370790 62376)
 (buffers 1000 514))

bug#45872: 27.1; rcirc nick tracking

[Lars Ingebrigtsen]

Ken Raeburn <raeburn@redhat.com> writes:

> One of my IRC contacts uses frequent nick changes to indicate away
> status, e.g., "johnsmith" when available, "johnsmith|away",
> "johnsmith|vacation", whatever. I've noticed that sometimes rcirc will
> fail to rename the buffer used for private messages between us, and so
> I'll wind up with a buffer "johnsmith|away" showing something along the
> lines of:
>
>   ... *** johnsmith NICK johnsmith|away
>   ... *** johnsmith|away NICK johnsmith
>
> but the buffer won't have been renamed back to "johnsmith@<server>", and
> if I try sending him a message, it'll try sending to johnsmith|away and
> will fail. I can use "/msg johnsmith...", or he can send messages to me,
> and a new buffer will be created.

rcirc has gotten a lot of fixes in Emacs 28, but I'm not sure whether
this is one of the things that have been fixed.  I've added Philip to
the CCs; he'll probably know.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45872: 27.1; rcirc nick tracking

[Philip Kaludercic]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Ken Raeburn <raeburn@redhat.com> writes:
>
>> One of my IRC contacts uses frequent nick changes to indicate away
>> status, e.g., "johnsmith" when available, "johnsmith|away",
>> "johnsmith|vacation", whatever. I've noticed that sometimes rcirc will
>> fail to rename the buffer used for private messages between us, and so
>> I'll wind up with a buffer "johnsmith|away" showing something along the
>> lines of:
>>
>>   ... *** johnsmith NICK johnsmith|away
>>   ... *** johnsmith|away NICK johnsmith
>>
>> but the buffer won't have been renamed back to "johnsmith@<server>",

Do you have some idea in what cases the buffer is not renamed?  In
principle, the NICK handler (rcirc-handler-NICK) should handle this
case, but there might be issues if you disconnect and reconnect.

>> and if I try sending him a message, it'll try sending to
>> johnsmith|away and will fail.  I can use "/msg johnsmith...", or he
>> can send messages to me, and a new buffer will be created.

This hasn't been fixed yet, but I am intending to do so.

> rcirc has gotten a lot of fixes in Emacs 28, but I'm not sure whether
> this is one of the things that have been fixed.  I've added Philip to
> the CCs; he'll probably know.  :-)

-- 
	Philip Kaludercic

bug#45872: 27.1; rcirc nick tracking

[Ken Raeburn]

On 7/23/21 8:02 AM, Philip Kaludercic wrote:
> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> Ken Raeburn <raeburn@redhat.com> writes:
>>
>>> One of my IRC contacts uses frequent nick changes to indicate away
>>> status, e.g., "johnsmith" when available, "johnsmith|away",
>>> "johnsmith|vacation", whatever. I've noticed that sometimes rcirc will
>>> fail to rename the buffer used for private messages between us, and so
>>> I'll wind up with a buffer "johnsmith|away" showing something along the
>>> lines of:
>>>
>>>    ... *** johnsmith NICK johnsmith|away
>>>    ... *** johnsmith|away NICK johnsmith
>>>
>>> but the buffer won't have been renamed back to "johnsmith@<server>",
> Do you have some idea in what cases the buffer is not renamed?  In
> principle, the NICK handler (rcirc-handler-NICK) should handle this
> case, but there might be issues if you disconnect and reconnect.

I forgot to update this ticket... I found that rcirc-buffer-alist 
included a nick that had text properties set, and scanning the list 
didn't find a match. I used advice-add to postprocess the list after 
rcirc-handler-NICK using string-equal to work around it, and that seems 
to do the job (as long as I can stay connected).

I haven't checked in a while to see if it's been fixed. If not, a better 
fix might be stripping out the text properties before putting a nick 
into the list.

Ken

bug#45872: 27.1; rcirc nick tracking

[Ken Raeburn]

Oh, but no, I'm not sure exactly what usage pattern cause this situation 
to arise, or why it wouldn't just show up for everyone.

Ken

bug#45872: 27.1; rcirc nick tracking

[Philip Kaludercic]

Ken Raeburn <raeburn@redhat.com> writes:

> On 7/23/21 8:02 AM, Philip Kaludercic wrote:
>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>>
>>> Ken Raeburn <raeburn@redhat.com> writes:
>>>
>>>> One of my IRC contacts uses frequent nick changes to indicate away
>>>> status, e.g., "johnsmith" when available, "johnsmith|away",
>>>> "johnsmith|vacation", whatever. I've noticed that sometimes rcirc will
>>>> fail to rename the buffer used for private messages between us, and so
>>>> I'll wind up with a buffer "johnsmith|away" showing something along the
>>>> lines of:
>>>>
>>>>    ... *** johnsmith NICK johnsmith|away
>>>>    ... *** johnsmith|away NICK johnsmith
>>>>
>>>> but the buffer won't have been renamed back to "johnsmith@<server>",
>> Do you have some idea in what cases the buffer is not renamed?  In
>> principle, the NICK handler (rcirc-handler-NICK) should handle this
>> case, but there might be issues if you disconnect and reconnect.
>
> I forgot to update this ticket... I found that rcirc-buffer-alist
> included a nick that had text properties set, and scanning the list
> didn't find a match. I used advice-add to postprocess the list after
> rcirc-handler-NICK using string-equal to work around it, and that
> seems to do the job (as long as I can stay connected).
>
> I haven't checked in a while to see if it's been fixed. If not, a
> better fix might be stripping out the text properties before putting a
> nick into the list.

That shouldn't be an issue, but I wonder where the text properties come
from. Could you find out what text properties these were that were
confusing rcirc?

> Ken
>

-- 
	Philip Kaludercic

bug#45872: 27.1; rcirc nick tracking

[Ken Raeburn]

On 7/24/21 10:56 AM, Philip Kaludercic wrote:

> Ken Raeburn <raeburn@redhat.com> writes:

> I forgot to update this ticket... I found that rcirc-buffer-alist
> included a nick that had text properties set, and scanning the list
> didn't find a match. I used advice-add to postprocess the list after
> rcirc-handler-NICK using string-equal to work around it, and that
> seems to do the job (as long as I can stay connected).
>
> I haven't checked in a while to see if it's been fixed. If not, a
> better fix might be stripping out the text properties before putting a
> nick into the list.
> That shouldn't be an issue, but I wonder where the text properties come
> from. Could you find out what text properties these were that were
> confusing rcirc?

It's setting font-lock-face to rcirc-other-nick. Oh... but I'm mixing 
this up with some other issue, I think. My apologies... the text 
properties are stored, but they're just a distraction. The access 
methods like assoc-string do ignore them.

Looking back at the 27.1 code I'm still running, I don't think there's 
anything even trying to update rcirc-buffer-alist in response to NICK. 
Rename the buffer, yes, but not change the key it's listed under. If a 
buffer johnsmith@irc.server is initially stored in the alist under the 
key "johnsmith" (or #("johnsmith" 0 9 (font-lock-face 
(rcirc-other-nick)))) then it'll still be stored under that key even if 
the buffer is renamed to johnsmith|away@irc.server.

So one failure to rename the buffer is those cases where the key in 
rcirc-buffer-alist doesn't match, because a previous rename didn't 
update the key, and the handle hasn't been renamed back to its original 
value as stored in as a key in the alist.

If the buffer rename back to remove the "|away" is missed, then I can't 
use the johnsmith|away@irc.server buffer to talk to johnsmith@irc.server 
any more, as I described in my original report. The handle info stored 
in the buffer is out of date. I can use "/msg johnsmith" and it'll 
create a new johnsmith@irc.server buffer, but another NICK message might 
try to rename that to johnsmith|away@irc.server again and would fail.

Ken

bug#45872: 27.1; rcirc nick tracking

[Philip Kaludercic]

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

Ken Raeburn <raeburn@redhat.com> writes:

> On 7/24/21 10:56 AM, Philip Kaludercic wrote:
>
>> Ken Raeburn <raeburn@redhat.com> writes:
>
>> I forgot to update this ticket... I found that rcirc-buffer-alist
>> included a nick that had text properties set, and scanning the list
>> didn't find a match. I used advice-add to postprocess the list after
>> rcirc-handler-NICK using string-equal to work around it, and that
>> seems to do the job (as long as I can stay connected).
>>
>> I haven't checked in a while to see if it's been fixed. If not, a
>> better fix might be stripping out the text properties before putting a
>> nick into the list.
>> That shouldn't be an issue, but I wonder where the text properties come
>> from. Could you find out what text properties these were that were
>> confusing rcirc?
>
> It's setting font-lock-face to rcirc-other-nick. Oh... but I'm mixing
> this up with some other issue, I think. My apologies... the text
> properties are stored, but they're just a distraction. The access
> methods like assoc-string do ignore them.
>
> Looking back at the 27.1 code I'm still running, I don't think there's
> anything even trying to update rcirc-buffer-alist in response to
> NICK. Rename the buffer, yes, but not change the key it's listed
> under. If a buffer johnsmith@irc.server is initially stored in the
> alist under the key "johnsmith" (or #("johnsmith" 0 9 (font-lock-face
> (rcirc-other-nick)))) then it'll still be stored under that key even
> if the buffer is renamed to johnsmith|away@irc.server.

That is true, the following should fix that:


[-- Attachment #2: Type: text/plain, Size: 779 bytes --]

diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index 60751c14e2..e5663d3fe6 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -3149,7 +3149,11 @@ rcirc-handler-NICK
 	(with-current-buffer chat-buffer
 	  (rcirc-print process sender "NICK" old-nick new-nick)
 	  (setq rcirc-target new-nick)
-	  (rename-buffer (rcirc-generate-new-buffer-name process new-nick)))))
+	  (rename-buffer (rcirc-generate-new-buffer-name process new-nick))))
+      (setf rcirc-buffer-alist
+            (cons (cons new-nick chat-buffer)
+                  (delq (assoc-string old-nick rcirc-buffer-alist t)
+                        rcirc-buffer-alist))))
     ;; remove old nick and add new one
     (with-rcirc-process-buffer process
       (let ((v (gethash old-nick rcirc-nick-table)))

[-- Attachment #3: Type: text/plain, Size: 845 bytes --]


Since the handler hasn't been changed since 2005, you should be able to
apply the change and see if it works.

> So one failure to rename the buffer is those cases where the key in
> rcirc-buffer-alist doesn't match, because a previous rename didn't
> update the key, and the handle hasn't been renamed back to its
> original value as stored in as a key in the alist.
>
> If the buffer rename back to remove the "|away" is missed, then I
> can't use the johnsmith|away@irc.server buffer to talk to
> johnsmith@irc.server any more, as I described in my original
> report. The handle info stored in the buffer is out of date. I can use
> "/msg johnsmith" and it'll create a new johnsmith@irc.server buffer,
> but another NICK message might try to rename that to
> johnsmith|away@irc.server again and would fail.
>
> Ken
>

-- 
	Philip Kaludercic

bug#45872: 27.1; rcirc nick tracking

[Ken Raeburn]

Philip Kaludercic <philipk@posteo.net> writes:
> That is true, the following should fix that:

Sorry for the delays in getting back to this. I see that 28.0.91
incorporates the change you included, but I’m still seeing messages
logged in the server buffer like this:

2022-03-16T08:24:29 !!! ":someuser|away!~someuser@some-internal-host.redhat.com NICK :someuser" (error Buffer name ‘someuser@some-irc-host.redhat.com’ is in use)

I think this is coming from cases where I’ve been offline for a nick
renaming, so when I come online, “someuser|away” is online but I have a
buffer “someuser”. Perhaps the UNIQUE argument to rename-buffer should
be set?

Ken

bug#45872: 27.1; rcirc nick tracking

[Lars Ingebrigtsen]

Ken Raeburn <raeburn@redhat.com> writes:

> 2022-03-16T08:24:29 !!!
> ":someuser|away!~someuser@some-internal-host.redhat.com NICK
> :someuser" (error Buffer name ‘someuser@some-irc-host.redhat.com’ is
> in use)
>
> I think this is coming from cases where I’ve been offline for a nick
> renaming, so when I come online, “someuser|away” is online but I have a
> buffer “someuser”. Perhaps the UNIQUE argument to rename-buffer should
> be set?

So the suggested change is:

diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index 0d30b34922..815dfef50f 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -3298,7 +3298,7 @@ rcirc-handler-NICK
       (with-current-buffer chat-buffer
 	(rcirc-print process sender "NICK" old-nick new-nick)
 	(setq rcirc-target new-nick)
-	(rename-buffer (rcirc-generate-new-buffer-name process new-nick)))
+	(rename-buffer (rcirc-generate-new-buffer-name process new-nick) t))
       (setf rcirc-buffer-alist
             (cons (cons new-nick chat-buffer)
                   (delq (assoc-string old-nick rcirc-buffer-alist t)

Skimming the code, it seems like this should do the trick (but I haven't
tested it).  Ken, Philip, does this look OK to you?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45872: 27.1; rcirc nick tracking

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Skimming the code, it seems like this should do the trick (but I haven't
> tested it).  Ken, Philip, does this look OK to you?

There were no comments in a month, so I've now pushed the change to
Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no