28.0.50; buffer-naming collisions involving bouncers in ERC

[J.P.]

Tags: patch

This follows directly from #40121 "27.0.90; ERC incorrectly reuses
single buffer for channels of same name," which may be related to #30639
"25.1; ERC buffer name not unique, broken on reconnect." [1]

In the first section below, you'll find a handful of summaries
describing user experiences related to this email's subject line. While
these descriptions overlap quite a bit, it's their differences that
reveal the real culprit, which is ERC's use of servers rather than
networks for connection identities.

Additional materials, like logs and screenshots, accompany these
descriptions. They can be found here [2] along with a few other related
scenarios not involving bouncers. Each scenario includes a corresponding
test case that reproduces its problematic behavior. More on testing
further below.

Beware that the summaries themselves are rather matter-of-fact (boring)
and are mostly included for posterity. Unless you're directly impacted
by this bug and looking to make common cause, please just skip to the
main discussion further below. Thanks.


Bug descriptions
~~~~~~~~~~~~~~~~

Nickname: clash-of-chans/bouncer-history
Subject: Intermingled output in channel buffer on connect
Playback: true

Connect to two IRC networks via the same bouncer. Do this by issuing two
separate `erc' invocations. This will create two connections with the
same remote address. The bouncer should send "playback" (logs) for an
identically named channel on both upstream networks. It does this
because it's currently "joined" to (maintaining a presence in) both
channels. A single ERC channel buffer is created showing the log output
from both channels. Live, real-time output continues below the playback.

This is *almost* the same bug as #40121 "ERC incorrectly reuses
single buffer for channels of same name."  The difference here is that
the well-known TCP listening address is the same for both connections.

Note that this issue existed prior to Emacs commit
88567ca8ecb505a59157af6338ebe355a304182b "Fix erc-reuse-buffers
behavior." Before that commit, the buffer/process situation looked like
this:

  #<buffer 127.0.0.1:16668>           #<process erc-127.0.0.1-16668>
  #<buffer 127.0.0.1:16668/127.0.0.1> #<process erc-127.0.0.1-16668<1>>

With that commit, the second server's *buffer* name gained a <2>
suffix, but its process name remains unchanged.


Nickname: clash-of-chans/auto-join
Subject: Autojoin module joins wrong channels when using bouncer
Playback: false

Ensure the module `autojoin' is loaded. Connect to an IRC network named
foonet via a bouncer. Join a channel named #chan, and then quit. Connect
to a different network, barnet, via the same bouncer. The same server
buffer is reused. The existing #chan buffer (with foonet's output) is
also reused.

Now reconnect to foonet via the same bouncer. The bouncer sends playback
for #chan@foonet, which is displayed in the same unified #chan buffer.
After that, output from both networks is printed in an alternating
fashion.


Nickname: clash-of-chans/rename-buffers
Subject: Channel buffers still collide despite erc-rename-buffers
Playback: false

Ensure the option `erc-rename-buffers' is non-nil. Connect to an IRC
network named foonet via a bouncer. The server buffer is correctly
renamed to match the network "foonet." Join a channel named #chan.
Connect to a different network, barnet, via the same bouncer. This
server buffer is also renamed correctly. Join a channel of the same
name, #chan, on this network. Another buffer is not created. The
existing buffer is not renamed. Output in #chan is intermingled,
displaying content from both networks.

This scenario is identical to "clash-of-chans/bouncer-history," except
for the lack of playback and the `erc-rename-buffers' option.


Nickname: clash-of-chans/uniquify-fail
Subject: Unpredictable intermingling of proxied channels
Playback: true

Remove the `autojoin' module, which is enabled by default. Set the
variable `erc-reuse-buffers' to nil. Connect to two networks via the
same bouncer.

The bouncer sends "playback" for two channels, one from each network.
The channels share the same name, and the bouncer is still subscribed to
both. A single channel buffer displaying intermingled output is created.
The process returned by `get-buffer-process' toggles back and forth
between client processes. This happens whenever a speaker from the "out
of phase" network speaks.

Now, emitting a PART in this unified buffer only leaves one network's
channel (call it "foonet's" channel). And a new channel buffer appears
exclusive to the *surviving* network's channel ("barnet's" channel). The
other, previously intermingled buffer is left inactive. However,
attempting a /join #chan from its input prompt succeeds in reviving
it, meaning the same buffer is reused here (for new output exclusive to
foonet) despite `erc-resuse-buffers' being nil.

Revisiting the other #chan buffer and emitting a PART succeeds in
leaving the channel. However, a subsequent /join #chan in the same
buffer does not create a new buffer. Instead, it causes intermingling to
resume in the *other* channel buffer, the one originally intermingled
and then foonet-only.

This bug is a mashup of two other "clash" scenarios, "buffer-history"
and "uniquify-litter," but it exhibits slightly different `reuse'
behavior.


Nickname: clash-of-chans/uniquify-litter
Subject: Buffer creation unpredictable with identically named channels
Playback: true

Ensure the `autojoin' module is disabled (it's enabled by default). Set
the variable `erc-reuse-buffers' to nil (it's normally t). Connect to
two networks via the same bouncer.

The bouncer sends playback for two channels, one from each network. The
channels share the same name. A single channel buffer is created
displaying intermingled output.

Continuing, emit a PART in the combined buffer, timing it to affect the
first network (call that "foonet"). After a bit, another channel buffer
appears exclusive to the *second* network (call that "barnet"). The
original, unified buffer no longer has an active process.

Visit barnet's #chan buffer (the one automatically created earlier) and
emit a PART and (after a sec) a subsequent JOIN. This should create
another buffer, which is consistent with `erc-reuse-buffers' being off.

But switching back to the now inactive first buffer (the intermingled
one) and sending a /join #chan will *not* create a new buffer. New
output exclusive to foonet will just start appearing below the old,
intermingled stuff.


Nickname: rebuffed/gapless
Subject: Back-to-back bouncer connections, but only the second survives
Playback: true

Make two back-to-back connections to the same bouncer. This bouncer
should be sustaining two upstream IRC-network connections with one
channel joined on each. The channels should have distinct names.

When the second connection completes, two new buffers have been created.
The first is a server buffer whose ~erc-server-process~ belongs to the
second network. The second is a channel buffer whose channel belongs to
the second network. No channel buffers exist for the first network. No
notices or welcome messages from the first network appear anywhere in
the one server buffer.

This issue existed prior to commit 88567ca8 "Fix erc-reuse-buffers
behavior."


Nickname: rebuffed/reuseless
Subject: Bouncer-related server-buffer naming regression
Playback: false

Connect twice to the same bouncer endpoint, once for each of two proxied
upstream networks. No channels are currently joined. When
`erc-reuse-buffers' is disabled, the buffer created for the second
connection clobbers the first, and both connections remain alive.

This issue arose with commit 88567ca8ecb505a59157af6338ebe355a304182b
"Fix erc-reuse-buffers behavior." Before that commit, two buffers would
be created:

  #<buffer 127.0.0.1:6667/127.0.0.1>      #<process erc-127.0.0.1-6667>
  #<buffer 127.0.0.1:6667/127.0.0.1<2>>   #<process erc-127.0.0.1-6667<1>>

Whereas now only the second one survives:

  #<buffer 127.0.0.1:6667/127.0.0.1>      #<process erc-127.0.0.1-6667>
  #<buffer 127.0.0.1:6667/127.0.0.1>      #<process erc-127.0.0.1-6667<1>>

Note that when `erc-reuse-buffers' is *enabled*, two buffers are indeed
created, as before. Except now, the name of the first is
#<buffer 127.0.0.1:6667> without the slash-host suffix.

What's surprising is that `erc-reuse-buffers' has any effect at all
given that, according to the doc string, the option ought only concern
channel and query buffers. (And this bug is only about server buffers.)


Discussion
~~~~~~~~~~

Thus far, a definitive solution to all or most of the above eludes me.
As such, I can't yet lay claim to a fix despite the patches on offer
below. So if anyone has a smarter/simpler proposal, please don't
hesitate to share, and I'll gladly make way. In fact, I seem to recall
ERC's own maintainer investigating related matters a while back. And so
a comparable, if not superior solution may already be in the works! (Or
perhaps that can be arranged with a little nudging.) :)

Anyhow, for the time being, please consider the attached series of WIP
patches a mere jumping-off point/placeholder to get the ball rolling.
These all depend on a collection of tests and related helpers that I'm
of course willing to rework as needed. A broad rationale for their
inclusion appears below [A].


Some background

Unlike most clients, ERC (to my knowledge) doesn't offer configurable
connection identities, by which I mean persistent, user-adjustable
settings that tie TCP endpoints to "announced" server names and their
networks [3]. This isn't in itself a bad thing, but it invites some
complications where proxies are concerned (not to suggest that such
problems are unique to ERC).

One method for disambiguation used by some clients adopts a pet
convention introduced by popular bouncers, like ZNC. In this scheme, a
unique network identifier and sometimes other details useful to a
bouncer are smuggled into the payload of an early client-initiated
command, like PASS, NICK, or USER. But this practice has not been
standardized. And it's entirely plausible a client won't know which
network it's connecting to until after introductions have been made. I
mention this because I don't think it's worth considering as a primary
solution.

Right now, different parts of the library vary in how they identify
connections. Some rely on the variable `erc-session-server', whose value
originates from the argument passed as the :server parameter of the
entry-point `erc' command. This is an IP address or a host name.
Elsewhere, the network's "announced" name is preferred. This is an
FQDN-like identifier scraped from early banner numerics, like RPL_MYINFO
or RPL_YOURHOST, and stored in the variable `erc-server-announced-name'.

These two server names tend to be used as fallbacks for each other,
though the motivation for favoring one over the other is often a mystery
(to me). Occasionally, the last couple domain components (labels) of an
announced name are used to divine an identity [4]. For example,
foo.gnu.org and bar.gnu.org might be pegged as individual servers on the
same network.

Currently, various places in the library appeal to ERC's `networks'
module (erc-networks.el) for the network identifier. This is often used
for less important purposes such as mode-line info. Under normal
circumstances, the module's API returns a name taken directly from the
NETWORK parameter of the RPL_ISUPPORT numeric. This is a stylized name
potentially containing spaces and other characters [5]. But it's useful,
authoritative info nonetheless.


The current approach

  "The only way to do it is connection=network" - Irssi's maintainer [6]

I'd like to believe ERC's authors basically agreed with this, at least
in spirit. And while their whole ad hoc/dynamic way of assigning
connection identities is a bit different, I don't think there's any
reason to abandon it just yet, especially if we strive to place more
emphasis on understanding and applying the evolving standard going
forward.

Here are a couple of assumptions that had better hold if my present
angle of attack is to get us off the ground:

1. There is at most one connection from a client to a network at any
   given moment [7]

2. Buffer->network associations cannot change once determined, i.e.,
   networks and ERC buffers mate for life, even when disconnected

So, until its network is known, a connection is considered unique but
orphaned. And the moment its network is discovered, any conflicts must
be resolved immediately and propagated throughout the rest of the
program.

With channels and queries, this is relatively straightforward: retain
the behavior exhibited (or at least aspired to) when the option
`erc-rename-buffers' is in effect. Only now, expand its berth to cover
server buffers. Also, make it more precise, and make it the default.
Moreover, mandate that the `networks' module always be loaded because it
provides essential network resolution/normalization functionality upon
which everything else relies. (This is likely already the case anyway,
effectively speaking.)

Of course, at least a few exceptions and what-ifs concerning all of the
above come to mind (or will, most certainly). Here's just one, as an
example, in the form of a Q&A:

Q: Say you connect to a bouncer to do some maintenance or check some
logs without being patched through to an upstream network. In this case,
a network name may never be announced. If another connection is
attempted to the same endpoint, perhaps with the aim of being proxied to
an upstream IRC network, should it be rejected as a dupe or allowed to
proceed? And what should its buffer name be?

A: The second connection should be allowed to complete and should be
considered wholly distinct and unrelated to the first. Its server buffer
should be named after the dialed TCP address and uniquified with an
incremented <n> suffix [8]. But it should adopt the name of the network
once that's discovered. If there's an existing connection for that
network and it's alive, the new one should be dropped immediately and an
error signaled. If buffers from an earlier, now dead connection to that
same network exist, the new connection should inherit those channel and
query buffers along with all relevant data from the server buffer (which
should then be killed off or renamed).

Plenty of other concerns and questions exist, many unresolved. The
accompanying WIP patch set is peppered with comments labeled "ERASE ME"
that pose some of these in context. (Apologies if that's annoying.) The
patches themselves I've refrained from attaching because their combined
size is around 200K (mostly tests). Instead, they're available here [9].

Items still left to address generally involve query buffers, but other
things like &local channels remain unexplored. There are still plenty of
unit tests to be written, and a few loose ends regarding IRC standards
may require additional investigative legwork to nail down completely.

Thanks again,
J.P.


Notes
~~~~~

[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=40121

     https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30639

[2]: https://jpneverwas.gitlab.io/erc-tools/

[3]: Not that the ingredients don't already exist for this sort of
     setup. See the variable `net' in the command `erc-server-select'.

[4]: See option `erc-common-server-suffixes' used by functions
     `erc-shorten-server-name' and `erc-canonicalize-server-name'.

     Indeed, leveraging domain name hierarchies for determining
     connection identities a fine idea, and erc-networks.el does it well
     enough when performing lookups in a predefined alist. See [5].

     With erc-join.el, however, the same basic idea leads to unfortunate
     results. See option `erc-autojoin-domain-only'. In the scenario
     named clash-of-chans/auto-join, this truncation operation produces
     "0.1" because an IP address is used where a host name is expected.

[5]: Characters disallowed in domain names are actually fine for our
     purposes. The standard (according to ircdocs, based on the Hardy
     draft) states that NETWORK's value should only be used for
     informational purposes. Whether you take that to mean it shouldn't
     be used for identification purposes is up for debate, but some
     influential voices certainly take that view.

     I'm unsure what the best course of action is here, but the
     `networks' module actually includes another method of identifying
     networks, which is currently used as a fallback. And that's to
     consult a user option for known networks (it basically associates
     domain names with canonical identifiers). Because it offers a means
     of escape and because it would only run once per connection, I
     don't think there's any harm in elevating its role to pole position
     and using the vanity name as a fallback.

[6]: Ailin Nemui (Nei) in conversation with me on #ircdocs, 05/21/2021.

[7]: Legitimate exceptions exist, for example connecting as two
     different users to a network through a bouncer to debug a bot.

[8]: We can't use anything derived from the physical connection, such as
     the ephemeral local port, because none of that's known at buffer
     creation time. Also, this scenario demonstrates why interpreting
     the source/prefix from server-originating messages as a network
     name is never seen (e.g., :irc.znc.in).

[9]: Web UI (javascript):

     https://gitlab.com/jpneverwas/erc-tools/-/tree/master/resources/trunk/wip

     Direct download (zip file):

     
https://gitlab.com/jpneverwas/erc-tools/-/jobs/artifacts/master/download/?job=test:trunk-only-wip

     This one ^ redirects to whatever the latest incarnation may be. If
     you require diffs, use the web UI or demand that I furnish them.
     The ones marked WIP are *not* ready for prime time. If they ever
     are, I intend on squashing them down into just one or two.

[A]: Test server and related helpers (a rationale)

     The first few patches in this set are meant to lay some groundwork
     for a larger undertaking to retrofit ERC with the flexibility
     needed to accommodate modern features like IRCv3 capability
     negotiation and SASL authentication, which have recently become
     fixtures of the living IRC standard. For example, Libera (like
     Freenode before it), has begun requiring SASL authentication from
     certain IP ranges, which was something formerly demanded only of
     TOR users.

     The current health of the library is probably on par with at least
     a few others of its age in the sense of being "open for extension,
     closed for modification." However, many of the newer changes on the
     horizon require overhauling vast swaths of the foundation. To my
     knowledge, accomplishing that in situ is best handled by fortifying
     the library with functional tests. The alternative is finding a
     comparable replacement (or creating a greenfield one) and making a
     swap at some point.

     Not that anyone should care, but I don't in principle subscribe to
     the view that blanketing a library with functional tests is always
     the best way forward, at least for an IRC client. If the code were
     written in a more modern style, with more easily digestible
     bite-sized pieces and more controllable side effects, we could be
     reasonably certain (with the help of unit tests) that when wired
     up, everything would behave as advertised. Unfortunately, that's
     not the case with ERC in its present condition.

     BTW, these tests will also afford us the breathing room to
     temporarily ignore certain issues with core functionality, namely,
     things being only half implemented or otherwise not up to spec.
     Examples include RPL_ISUPPORT handling, casemapping, and v3 message
     tags.



In GNU Emacs 28.0.50 (build 1, x86_64-redhat-linux-gnu, GTK+ Version 3.24.29, 
cairo version 1.17.4)
 of 2021-05-18 built on pc
Repository revision: 1276ba75eb0d308b76df34c522bb0d6e059c146e
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Fedora 34 (Workstation Edition)

Configured using:
 'configure --enable-check-lisp-object-type --enable-checking=yes,glyphs
 --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu
 --program-prefix= --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin
 --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share
 --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec
 --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man
 --infodir=/usr/share/info --with-dbus --with-gif --with-jpeg --with-png
 --with-rsvg --with-tiff --with-xft --with-xpm --with-x-toolkit=gtk3
 --with-gpm=no --with-xwidgets --with-modules --with-harfbuzz
 --with-cairo --with-json build_alias=x86_64-redhat-linux-gnu
 host_alias=x86_64-redhat-linux-gnu CC=gcc 'CFLAGS=-O0 -g3'
 LDFLAGS=-Wl,-z,relro
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
X11 XDBE XIM XPM XWIDGETS GTK3 ZLIB

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

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-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:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
iso-transl 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 easymenu timer select scroll-bar mouse jit-lock
font-lock syntax 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 button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads
xwidget-internal dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting cairo move-toolbar gtk x-toolkit
x multi-tty make-network-process emacs)

Memory information:
((conses 16 51721 6314)
 (symbols 48 6601 1)
 (strings 32 18223 1958)
 (string-bytes 1 611836)
 (vectors 16 13547)
 (vector-slots 8 178617 7780)
 (floats 8 21 45)
 (intervals 56 322 0)
 (buffers 992 11))