From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: "J.P." Newsgroups: gmane.emacs.erc.general,gmane.emacs.bugs Subject: 28.0.50; buffer-naming collisions involving bouncers in ERC Date: Sat, 22 May 2021 18:22:41 -0700 Message-ID: <875yzakzvi.fsf@neverwas.me> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="10539"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: emacs-erc@gnu.org To: bug-gnu-emacs@gnu.org Original-X-From: emacs-erc-bounces+sf-erc-help=m.gmane-mx.org@gnu.org Sun May 23 03:22:57 2021 Return-path: Envelope-to: sf-erc-help@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lkcpV-0002YH-8Y for sf-erc-help@m.gmane-mx.org; Sun, 23 May 2021 03:22:57 +0200 Original-Received: from localhost ([::1]:40566 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lkcpU-0001Dl-7n for sf-erc-help@m.gmane-mx.org; Sat, 22 May 2021 21:22:56 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:47254) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lkcpQ-0001Dc-4R for emacs-erc@gnu.org; Sat, 22 May 2021 21:22:52 -0400 Original-Received: from dal1relay157.mxroute.com ([199.181.239.157]:41955) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lkcpM-0004Cc-CW for emacs-erc@gnu.org; Sat, 22 May 2021 21:22:51 -0400 Original-Received: from filter004.mxroute.com ([149.28.56.236] filter004.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by dal1relay157.mxroute.com (ZoneMTA) with ESMTPSA id 17996d138730001a68.002 for (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256); Sun, 23 May 2021 01:22:44 +0000 X-Zone-Loop: 5caa10dcad9d246d6d35d8bd5071f1e6df1073d9da7d X-Originating-IP: [149.28.56.236] X-AuthUser: masked@neverwas.me Received-SPF: pass client-ip=199.181.239.157; envelope-from=jp@neverwas.me; helo=dal1relay157.mxroute.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-erc@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: General discussion about ERC List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-erc-bounces+sf-erc-help=m.gmane-mx.org@gnu.org Original-Sender: "emacs-erc" Xref: news.gmane.io gmane.emacs.erc.general:1527 gmane.emacs.bugs:207072 Archived-At: 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: # # # #> 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=C2=A0#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: # # #> #> Whereas now only the second one survives: # # # #> Note that when `erc-reuse-buffers' is *enabled*, two buffers are indeed created, as before. Except now, the name of the first is # 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=3Dnetwork" - 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 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=3D40121 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D30639 [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/downlo= ad/?job=3Dtest: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.2= 9, 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=3Dyes,glyphs --build=3Dx86_64-redhat-linux-gnu --host=3Dx86_64-redhat-linux-gnu --program-prefix=3D --prefix=3D/usr --exec-prefix=3D/usr --bindir=3D/usr/b= in --sbindir=3D/usr/sbin --sysconfdir=3D/etc --datadir=3D/usr/share --includedir=3D/usr/include --libdir=3D/usr/lib64 --libexecdir=3D/usr/libe= xec --localstatedir=3D/var --sharedstatedir=3D/var/lib --mandir=3D/usr/share/m= an --infodir=3D/usr/share/info --with-dbus --with-gif --with-jpeg --with-png --with-rsvg --with-tiff --with-xft --with-xpm --with-x-toolkit=3Dgtk3 --with-gpm=3Dno --with-xwidgets --with-modules --with-harfbuzz --with-cairo --with-json build_alias=3Dx86_64-redhat-linux-gnu host_alias=3Dx86_64-redhat-linux-gnu CC=3Dgcc 'CFLAGS=3D-O0 -g3' LDFLAGS=3D-Wl,-z,relro PKG_CONFIG_PATH=3D:/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=3Dibus 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))