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.bugs,gmane.emacs.erc.general Subject: bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict Date: Tue, 13 Dec 2022 07:13:14 -0800 Message-ID: <87o7s7qqut.fsf@neverwas.me> References: <20221212000031.2f679e5e@malediction> <87pmcowuzv.fsf@neverwas.me> <20221212203508.3cd44bb6@malediction> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="40256"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 59976@debbugs.gnu.org, emacs-erc@gnu.org To: Mike Kazantsev Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Dec 13 16:14:12 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1p56yx-000AD9-2U for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 13 Dec 2022 16:14:11 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1p56yr-0000yb-4h; Tue, 13 Dec 2022 10:14:05 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p56yp-0000w6-Da for bug-gnu-emacs@gnu.org; Tue, 13 Dec 2022 10:14:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1p56yp-000308-5O for bug-gnu-emacs@gnu.org; Tue, 13 Dec 2022 10:14:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1p56yo-00054C-HX for bug-gnu-emacs@gnu.org; Tue, 13 Dec 2022 10:14:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: "J.P." Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 13 Dec 2022 15:14:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 59976 X-GNU-PR-Package: emacs Original-Received: via spool by 59976-submit@debbugs.gnu.org id=B59976.167094441119456 (code B ref 59976); Tue, 13 Dec 2022 15:14:02 +0000 Original-Received: (at 59976) by debbugs.gnu.org; 13 Dec 2022 15:13:31 +0000 Original-Received: from localhost ([127.0.0.1]:32768 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1p56yH-00053j-EX for submit@debbugs.gnu.org; Tue, 13 Dec 2022 10:13:31 -0500 Original-Received: from mail-108-mta126.mxroute.com ([136.175.108.126]:44585) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1p56yE-00053X-1G for 59976@debbugs.gnu.org; Tue, 13 Dec 2022 10:13:28 -0500 Original-Received: from mail-111-mta2.mxroute.com ([136.175.111.2] filter006.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta126.mxroute.com (ZoneMTA) with ESMTPSA id 1850c0c1eff0001d7e.001 for <59976@debbugs.gnu.org> (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256); Tue, 13 Dec 2022 15:13:19 +0000 X-Zone-Loop: 2031df4aa751b3225969a3205a4336c0f82736f46b96 X-Originating-IP: [136.175.111.2] DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=neverwas.me ; s=x; h=Content-Type:MIME-Version:Message-ID:Date:References:In-Reply-To: Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=UTNBxZVTX1QiPoOlMJL0Yl+1DtXoGYXxJOPpguDfq3c=; b=VABfiNQJM0xPYaEmTi5Evcaav6 2i9IHhYn+Ag/Xyczu3/Hxsspues8nIq0oRgNU9tDh7Zo8E5s+/qBmN9XD1W1P+nB8d8NUgtyh3WXd UuswOW1aNbOtqJKUDl9wfk++qvofsdJ30WGZgSSVbm4h3R+ZkjyBopVv9v5HwLSPJ9O1VPXnEoIMm C4Y2lMOXI48dbgjZf1ComfGJ9Id4M7UMUGrO58wkUjGkcF5WjmEy4mlmc8SqRStHd6CbFlQqKZo9p bex7QrHupYPGKlre+hDa2K5kdmX9YyfGnt7jsQ6IbVAW7TUYfaA9ZbbfEZTegku368+RR3DucN/Zr qt1D4mHg==; In-Reply-To: <20221212203508.3cd44bb6@malediction> (Mike Kazantsev's message of "Mon, 12 Dec 2022 20:35:08 +0500") X-Authenticated-Id: masked@neverwas.me X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:250848 gmane.emacs.erc.general:2031 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Mike Kazantsev writes: >> > - What I'm trying to do: >> > >> > - (erc-tls ...) ;; cleates "slack" server buffer and some channels. >> > - Open "slack" buffer=20=20 >>=20 >> You mean opened manually, like by issuing a /query slack, right? So this >> is in addition to the observation above where "it sends you a message >> from user named 'slack'"? > > Sorry, no, I meant as in (switch-to-buffer "slack"), to type command into. > > No manual /query command is implied anywhere, but maybe it can also be > used to reproduce the issue, given how it should do roughly same thing. OK, gotcha. Thanks for clarifying. >> > - Send "/ping myuser" IRC-command there >>=20 >> You also mention "case-sensitive channel names." It'd be nice to know >> what those look like when they first arrive off the wire. (BTW, if >> you're expecting ERC to interoperate with a case-sensitive network, like >> one for which "#FOO" and "#foo" are distinct channels, then that may >> need a separate bug report.) > > It's not related to this issue at all, and I probably shouldn't have > mentioned it, to avoid adding extra confusion, sorry. > I.e. "slack" buffer in case of this issue is all-lowercase everywhere. I see, so "slack" (the nick) is already lowercase when it arrives from the server. > But as a separate unrelated thing, here is specific problem that I was > thinking to try solving with updated ERC, as it seem to have some new > code added, related to casemapping: > > (to stress this - it's a separate issue from this one, only here as a > curiosity, should be ignored entirely in context of this bug report) > > Detailed description: > https://github.com/progval/matrix2051/issues/41 > > I thought at first that issue might be CASEMAPPING=3Drfc3454 used > there, which I was not familiar with, and assumed that it doesn't > behave like ascii/rfc1459 casemapping. > > So I was thinking to try adding it to new erc-downcase func locally, > and fix the issue with case-sensitive channels that way. > > But after looking at RFC 3454 I've found that it includes casemapping > for ASCII letters and pretty much just extends casemapping to larger > unicode character set, and should be compatible with ERC, at least > for the case of simple ASCII letters. > > So it seem to be a bug in progval/matrix2051 ad-hoc ircd > implementation, which I've reported there at the URL above, with a > workaround I've used in ERC mentioned in the first comment there, > just in case someone might find it via google when bumping into same > issue. > > So, again, it's not really related to this issue, and not an ERC issue > at all, and I probably shouldn't have mentioned it here. > I've only run across 3454 in passing but always thought of it more as a standardizing framework through which abiding protocols could arrive at identical notions of a character from disparate input representations. However, in chapter 3, they do mention that their provided mapping tables "MUST be used by implementations of this specification," so I'm just confused, basically. But thanks for the lowdown regardless. I'll try diving into it properly at some point. >> > - Workaround: changing announced name of the server to something else >> > helps - "slack" query-buffer gets created and is separate from serve= r buffer.=20=20 >>=20 >> You mean by changing `erc-server-announced-name'? > > Yes... sort of. > > As I understand it, this name ends up being converted via > erc-networks-alist from erc-server-announced-name to a network symbol > (e.g. 'OFTC or 'slack) in erc-network value. Right, exactly. It would seem that since most IRC networks specify the NETWORK ISUPPORT parameter these days, `erc-networks-alist' now serves more as a means of specifying overrides. > So guess my description above is incorrect or somewhat incomplete - > what you'd actually want is have symbol in that alist not conflict with > query-channel name, not just whatever erc-server-announced-name ends up > being. > > Only slightly related to description above, and should likely be > ignored in context of this issue: > > To set erc-network to 'slack value I ended up replacing that alist > lookup with a much simplier local solution: > https://github.com/mk-fg/emacs-setup/blob/4dd532b/core/fg_erc.el#L72-L95 > (current version uses '=F0=9F=96=A7-slack symbol with that unicode pref= ix > exactly to avoid any name conflicts) That's a very clean looking ERC setup! Let's hope you won't experience too much churn with these upcoming "fixes." > It's partly needed because ad-hoc protocol-bridge ircd's don't > announce network names, and partly because just seemed simplier > to derive them this way, but I didn't think about :id solution that > you've mentioned below, which might work even better here. > > But pretty sure if one did use the alist-to-symbol lookup, they'd=20 > end up with same result, e.g. 'slack in erc-network, just like it > works with 'OFTC in there. Right, same result. It seems the bridge doesn't send a NETWORK ISUPPORT parameter at all. So, at present, I believe you have no choice but to add an entry to that alist or advise/shadow as you've done in your config. However, the `:id' stuff was always meant to work for these situations. That it doesn't is definitely a bug, which I've tried to partially remedy in the latest iteration of the growing patch set for this thread. >> > Was able to understand what seem to be the issue here by enabling >> > logging for erc-networks--id changes to *Messages* like this: >> > >> > (defun debug-watch-log (sym newval op where) >> > (message "Variable update: %s =3D %S -> %S [%S %S]" >> > sym (symbol-value sym) newval op where) >> > (backtrace)) >> > (add-variable-watcher 'erc-networks--id #'debug-watch-log) >> > >> > (run M-x cancel-debug-on-variable-change afterwards to disable this)= =20=20 >>=20 >> Thanks for the detective work. In this case, I'm pretty sure the root >> cause is not directly related to that variable, despite all appearances. Actually, I now see how `erc-networks--id' fails to be set (along with `erc-server-connected'). So, it's all really closely related. The real problem is that the connection is allowed to proceed in a degraded state when it should just be destroyed immediately when network detection fails. >> BTW, the most helpful insights are usually to be found in the contents >> of the *erc-protocol* buffer, which can be generated by calling >> `erc-toggle-debug-irc-protocol' before connecting for the first time in >> an Emacs session. If you're not comfortable sharing such info on the >> tracker, please consider doing so out of band. > > I think issue should be easily reproducible without needing the > protocol logs, by setting erc-network to your own nickname and then > sending /msg to yourself, or maybe even e.g. "/q OFTC". > > Will try it out a bit later and report on whether such trivial ways to > do it work. No worries, I ended up just creating a minimal setup locally. I can see that, in addition to not supplying a NETWORK, the bridge also doesn't furnish a 004, which ERC uses to populate `erc-server-announced-name'. However, at least on my instance, it sets the :source (prefix) to "server.", and ERC just ends up using that as a fallback. >> That said, at the moment, we only support unambiguous "network IDs" via >> the ":id" keyword parameter of `erc' and `erc-tls'. IOW, calling these >> with args like >>=20 >> :server "127.0.0.1" >> :port 2051 >> ... >> :id 'Slack >>=20 >> is supposed to always work. But, I can definitely see how that doesn't >> in some cases (e.g., a lowercase `:id'). As you say, you may have no >> control over who sends you a query, which is still pertinent in the case >> of an explicit `:id' so long as it consists entirely of valid nick >> characters. > > That looks like it might be a good solution to my other issue with > deriving names for these ad-hoc protocol-bridge networks mentioned > above, thanks for mentioning it. Well, it's supposed to be a good solution, but it's bugged in a few places, which you've probably noticed by now (all my bad). I've attempted to address the most glaring aspects in the patches below. >> 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/issue= s. 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. >> > I'm running ERC 5.4.1 from current 0e5d059 git master on Emacs 28.2 >> > (replacing "erc" dir in /usr/share/emacs), so it is also possible that >> > maybe some change in Emacs 29+ prevents this behavior, but it seems un= likely.=20=20 >>=20 >> Unlikely, yes. But I sometimes worry about ERC's loaddefs when people >> manually shadow/replace instead of building alongside Emacs or >> installing from devel: >>=20 >> (push '("devel" . "https://elpa.gnu.org/devel/") package-archives) > > I didn't consider that newer ERC can be installed this way, given that > it seem to be part of the emacs tree, but good to know, thanks, will > probably use this instead. > > (although tbf it seems more complicated than replacing a dir, but if > it's cleaner then yeah, probably best to do it via package-archives) (One thing to note that I've learned recently is that the package snapshot is only updated when a commit touches the "main" file, as in lisp/erc/erc.el.) >> (Not saying you need to do either, of course.) Thanks again for the very >> nice bug report. > > Thank you for looking into this and work on ERC in general. > > As someone who spends most of the day looking at emacs, it's by far the > most convenient and configurable client for me, that I end up using for > pretty much all modern IM protocols because of that. Thanks, that's really encouraging to hear. Hopefully, with the help of bug reports like this, we can make the experience more consistent and flexible for adventurous folks, like yourself, rather than simply cater to the lowest common denominator. --=-=-= Content-Type: text/x-patch; charset=utf-8 Content-Disposition: attachment; filename=0000-v1-v2.diff Content-Transfer-Encoding: quoted-printable >From 81e2668d2d20025c38f780b9238e722f5d4e0317 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Tue, 13 Dec 2022 00:23:58 -0800 Subject: [PATCH 0/5] *** NOT A PATCH *** *** BLURB HERE *** F. Jason Park (5): ; Be nicer when updating browse-url var in erc-compat Actually accept non-symbols as IDs in erc-open Limit casemapping to appropriate ranges in ERC Set erc-network to a "given" ID instead of failing Add escape hatch for picky servers in erc-sasl lisp/erc/erc-common.el | 16 +++----- lisp/erc/erc-compat.el | 7 +++- lisp/erc/erc-networks.el | 34 +++++++++++------ lisp/erc/erc-sasl.el | 33 ++++++++++++++--- lisp/erc/erc.el | 37 +++++++++++++------ test/lisp/erc/erc-networks-tests.el | 2 + .../erc/erc-scenarios-base-netid-samenet.el | 3 ++ test/lisp/erc/erc-tests.el | 3 ++ .../resources/base/local-modules/fourth.eld | 2 +- .../erc/resources/erc-scenarios-common.el | 2 +- 10 files changed, 95 insertions(+), 44 deletions(-) Interdiff: diff --git a/lisp/erc/erc-compat.el b/lisp/erc/erc-compat.el index 884abaf824..77625398ab 100644 --- a/lisp/erc/erc-compat.el +++ b/lisp/erc/erc-compat.el @@ -389,17 +389,13 @@ erc-compat--29-browse-url-irc url-irc-function))) (url-irc url))) =20 -(declare-function cl-adjoin "cl-lib" (arg1 arg2 &rest rest)) - (cond ((fboundp 'browse-url-irc)) ; 29 ((boundp 'browse-url-default-handlers) ; 28 - (require 'cl-lib) - (cl-pushnew '("\\`irc6?s?://" . erc-compat--29-browse-url-irc) - browse-url-default-handlers - :key #'car - :test (lambda (_ b) - (and (stringp b) - (string-match-p b "irc://localhost"))))) + (setf (alist-get "\\`irc6?s?://" browse-url-default-handlers + nil nil (lambda (a _) + (and (stringp a) + (string-match-p a "irc://localhost"= )))) + #'erc-compat--29-browse-url-irc)) ((boundp 'browse-url-browser-function) ; 27 (require 'browse-url) (let ((existing browse-url-browser-function)) diff --git a/lisp/erc/erc-networks.el b/lisp/erc/erc-networks.el index 19a7ab8643..51ba54f45c 100644 --- a/lisp/erc/erc-networks.el +++ b/lisp/erc/erc-networks.el @@ -60,6 +60,7 @@ erc-session-server (declare-function erc-buffer-filter "erc" (predicate &optional proc)) (declare-function erc-current-nick "erc" nil) (declare-function erc-display-error-notice "erc" (parsed string)) +(declare-function erc-display-message "erc" (parsed type buffer msg &rest = args)) (declare-function erc-error "erc" (&rest args)) (declare-function erc-get-buffer "erc" (target &optional proc)) (declare-function erc-server-buffer "erc" nil) @@ -1260,21 +1261,31 @@ erc-networks--determine return name) (and-let* ((vanity (erc--get-isupport-entry 'NETWORK 'single)) ((intern vanity)))) + (erc-networks--id-given erc-networks--id) erc-networks--name-missing-sentinel)) =20 -(defun erc-networks--set-name (_proc parsed) +(defun erc-networks--set-name (proc parsed) "Set `erc-network' to the value returned by `erc-networks--determine'. -Signal an error when the network cannot be determined." +Signal an error when the network cannot be determined after first +shutting down the connection." ;; Always update (possibly clobber) current value, if any. (let ((name (erc-networks--determine))) + (when (eq name (erc-networks--id-given erc-networks--id)) + (let ((m (format "Couldn't determine network. Using given ID `%s'." + name))) + (erc-display-message parsed 'notice nil m))) (when (eq name erc-networks--name-missing-sentinel) - ;; This can happen theoretically, e.g., if you're editing some - ;; settings interactively on a proxy service that impersonates IRC - ;; but aren't being proxied through to a real network. The + ;; This can happen theoretically, e.g., when adjusting settings + ;; on a proxy service that partially impersonates IRC but isn't + ;; currently conveying anything through to a real network. The ;; service may send a 422 but no NETWORK param (or *any* 005s). - (let ((m (concat "Failed to determine network. Please set entry for " - erc-server-announced-name " in `erc-networks-alist'= ."))) + (let ((m (concat "Failed to determine network. Please set entry for= \"" + erc-server-announced-name "\" in `erc-networks-alis= t'" + " or consider calling `erc-tls' with the keyword `:= id'." + " See Info:\"(erc) Network Identifier\" for more."= ))) (erc-display-error-notice parsed m) + (run-hook-with-args 'erc-quit-hook proc) + (delete-process proc) (erc-error "Failed to determine network"))) ; beep (setq erc-network name)) nil) @@ -1287,11 +1298,12 @@ erc-networks--ensure-announced Copy source (prefix) from MOTD-ish message as a last resort." ;; The 004 handler never ran; see 2004-03-10 Diane Murray in change log (unless erc-server-announced-name - (erc-display-error-notice parsed "Failed to determine server name.") + (setq erc-server-announced-name (erc-response.sender parsed)) (erc-display-error-notice - parsed (concat "If this was unexpected, consider reporting it via " - (substitute-command-keys "\\[erc-bug]") ".")) - (setq erc-server-announced-name (erc-response.sender parsed))) + parsed (concat "Failed to determine server name. Using \"" + erc-server-announced-name "\" instead." + " If this was unexpected, consider reporting it via " + (substitute-command-keys "\\[erc-bug]") "."))) nil) =20 (defun erc-unset-network-name (_nick _ip _reason) diff --git a/lisp/erc/erc-sasl.el b/lisp/erc/erc-sasl.el index 5b2c93988a..9486250529 100644 --- a/lisp/erc/erc-sasl.el +++ b/lisp/erc/erc-sasl.el @@ -414,17 +414,38 @@ erc-sasl--destroy " ")) (erc-sasl--destroy proc)) =20 +;; The rationale for not enabling this by default is twofold: +;; +;; - It more strongly implies that ERC supports client capability +;; negotiation, which is therefore more disingenuous. +;; +;; - We'd still be "faking it" by firing and forgetting, and more +;; balls in the air makes things less predictable. + +(defvar erc-sasl--send-cap-ls nil + "Whether to send an opening \"CAP LS\" command. +This is an escape hatch for picky servers expecting this command. +If you need this, please let us know via \\[erc-bug], so we can +offer a user option instead.") + (cl-defmethod erc--register-connection (&context (erc-sasl-mode (eql t))) - "Send speculative/pipelined CAP and AUTHENTICATE and hope for the best." + "Send speculative CAP and pipelined AUTHENTICATE and hope for the best." (if-let* ((c (erc-sasl--state-client erc-sasl--state)) (m (sasl-mechanism-name (sasl-client-mechanism c)))) (progn - (erc-server-send "CAP REQ :sasl") - (if (and erc-session-password - (eq :password (alist-get 'password erc-sasl--options))) - (let (erc-session-password) - (erc-login)) + (erc-server-send (if erc-sasl--send-cap-ls "CAP LS" "CAP REQ :sasl= ")) + (let ((erc-session-password + (and erc-session-password + (not (eq :password (alist-get 'password erc-sasl--opti= ons))) + erc-session-password)) + (erc-session-username + ;; The username may contain a colon or a space + (if (eq :user (alist-get 'user erc-sasl--options)) + (erc-current-nick) + erc-session-username))) (erc-login)) + (when erc-sasl--send-cap-ls + (erc-server-send "CAP REQ :sasl")) (erc-server-send (format "AUTHENTICATE %s" m))) (erc-sasl--destroy erc-server-process))) =20 diff --git a/test/lisp/erc/erc-networks-tests.el b/test/lisp/erc/erc-networ= ks-tests.el index fc12bf7ce3..02fc57d4e6 100644 --- a/test/lisp/erc/erc-networks-tests.el +++ b/test/lisp/erc/erc-networks-tests.el @@ -1171,6 +1171,8 @@ erc-networks--set-name (let (erc-server-announced-name (erc--isupport-params (make-hash-table)) erc-network + erc-quit-hook + (erc-server-process (erc-networks-tests--create-live-proc)) calls) (erc-mode) =20 diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el index e30452b1cf..51c562f525 100644 --- a/test/lisp/erc/erc-tests.el +++ b/test/lisp/erc/erc-tests.el @@ -428,21 +428,21 @@ erc-downcase =20 (ert-info ("ascii") (puthash 'CASEMAPPING '("ascii") erc--isupport-params) - (should (equal (erc-downcase "ABC 123 =CE=A3=CE=9B=CE=A3=CE=9D=CE=9F= =CE=A3") "abc 123 =CE=A3=CE=9B=CE=A3=CE=9D=CE=9F=CE=A3")) + (should (equal (erc-downcase "ABC 123 =CE=94=CE=9E=CE=A9=CE=A3") "ab= c 123 =CE=94=CE=9E=CE=A9=CE=A3")) (should (equal (erc-downcase "Bob[m]`") "bob[m]`")) (should (equal (erc-downcase "Tilde~") "tilde~" )) (should (equal (erc-downcase "\\O/") "\\o/" ))) =20 (ert-info ("rfc1459") (puthash 'CASEMAPPING '("rfc1459") erc--isupport-params) - (should (equal (erc-downcase "ABC 123 =CE=A3=CE=9B=CE=A3=CE=9D=CE=9F= =CE=A3") "abc 123 =CE=A3=CE=9B=CE=A3=CE=9D=CE=9F=CE=A3")) + (should (equal (erc-downcase "ABC 123 =CE=94=CE=9E=CE=A9=CE=A3") "ab= c 123 =CE=94=CE=9E=CE=A9=CE=A3")) (should (equal (erc-downcase "Bob[m]`") "bob{m}`" )) (should (equal (erc-downcase "Tilde~") "tilde^" )) (should (equal (erc-downcase "\\O/") "|o/" ))) =20 (ert-info ("rfc1459-strict") (puthash 'CASEMAPPING '("rfc1459-strict") erc--isupport-params) - (should (equal (erc-downcase "ABC 123 =CE=A3=CE=9B=CE=A3=CE=9D=CE=9F= =CE=A3") "abc 123 =CE=A3=CE=9B=CE=A3=CE=9D=CE=9F=CE=A3")) + (should (equal (erc-downcase "ABC 123 =CE=94=CE=9E=CE=A9=CE=A3") "ab= c 123 =CE=94=CE=9E=CE=A9=CE=A3")) (should (equal (erc-downcase "Bob[m]`") "bob{m}`")) (should (equal (erc-downcase "Tilde~") "tilde~" )) (should (equal (erc-downcase "\\O/") "|o/" ))))) diff --git a/test/lisp/erc/resources/base/local-modules/fourth.eld b/test/l= isp/erc/resources/base/local-modules/fourth.eld index fd6d62b6cc..4ac5dcbd38 100644 --- a/test/lisp/erc/resources/base/local-modules/fourth.eld +++ b/test/lisp/erc/resources/base/local-modules/fourth.eld @@ -1,7 +1,7 @@ ;; -*- mode: lisp-data; -*- ((cap 10 "CAP REQ :sasl")) ((nick 10 "NICK tester`")) -((user 10 "USER tester 0 * :tester")) +((user 10 "USER tester` 0 * :tester")) =20 ((authenticate 10 "AUTHENTICATE PLAIN") (0.0 ":irc.foonet.org CAP * ACK sasl") --=20 2.38.1 --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-Be-nicer-when-updating-browse-url-var-in-erc-compat.patch >From 7f832bce68a327d1930eac71f2d937c88e34055e Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 11 Dec 2022 19:16:07 -0800 Subject: [PATCH 1/5] ; Be nicer when updating browse-url var in erc-compat * lisp/erc/erc-compat.el: Be more cautious about modifying `browse-url-default-handlers' when loading erc-compat on Emacs 28. A user may have already added an entry for irc:// URLs before loading `erc-compat'. --- lisp/erc/erc-compat.el | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lisp/erc/erc-compat.el b/lisp/erc/erc-compat.el index bd93254758..77625398ab 100644 --- a/lisp/erc/erc-compat.el +++ b/lisp/erc/erc-compat.el @@ -391,8 +391,11 @@ erc-compat--29-browse-url-irc (cond ((fboundp 'browse-url-irc)) ; 29 ((boundp 'browse-url-default-handlers) ; 28 - (cl-pushnew '("\\`irc6?s?://" . erc-compat--29-browse-url-irc) - browse-url-default-handlers)) + (setf (alist-get "\\`irc6?s?://" browse-url-default-handlers + nil nil (lambda (a _) + (and (stringp a) + (string-match-p a "irc://localhost")))) + #'erc-compat--29-browse-url-irc)) ((boundp 'browse-url-browser-function) ; 27 (require 'browse-url) (let ((existing browse-url-browser-function)) -- 2.38.1 --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0002-Actually-accept-non-symbols-as-IDs-in-erc-open.patch >From 1b40fcdbe664ba4b364b7665370c66de82998038 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 11 Dec 2022 19:41:43 -0800 Subject: [PATCH 2/5] Actually accept non-symbols as IDs in erc-open * lisp/erc/erc.el (erc-generate-new-buffer-name): Despite what it says in the documentation, only symbols were being accepted as valid :id entry point arguments. This ensures that the `princ' form is used instead. * test/lisp/erc/erc-scenarios-base-netid-samenet.el (erc-scenarios-common--base-network-id-same-network): Randomly specify a string for the ID param instead of a non-nil symbol when opening a new connection. * test/lisp/erc/resources/erc-scenarios-common.el (erc-scenarios-common-assert-initial-buf-name): Adjust helper to allow for non-symbol IDs. (Bug#59976.) --- lisp/erc/erc.el | 9 +++++---- test/lisp/erc/erc-scenarios-base-netid-samenet.el | 3 +++ test/lisp/erc/resources/erc-scenarios-common.el | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el index 6bb2e013c3..a5ba45d9b3 100644 --- a/lisp/erc/erc.el +++ b/lisp/erc/erc.el @@ -1607,11 +1607,12 @@ erc-generate-new-buffer-name (if (and (with-suppressed-warnings ((obsolete erc-reuse-buffers)) erc-reuse-buffers) id) - (progn - (when-let* ((buf (get-buffer (symbol-name id))) + (let ((string (symbol-name (erc-networks--id-symbol + (erc-networks--id-create id))))) + (when-let* ((buf (get-buffer string)) ((erc-server-process-alive buf))) - (user-error "Session with ID %S already exists" id)) - (symbol-name id)) + (user-error "Session with ID %S already exists" string)) + string) (generate-new-buffer-name (if (and server port) (if (with-suppressed-warnings ((obsolete erc-reuse-buffers)) diff --git a/test/lisp/erc/erc-scenarios-base-netid-samenet.el b/test/lisp/erc/erc-scenarios-base-netid-samenet.el index 3cd8b7dfa1..1436712251 100644 --- a/test/lisp/erc/erc-scenarios-base-netid-samenet.el +++ b/test/lisp/erc/erc-scenarios-base-netid-samenet.el @@ -40,6 +40,9 @@ erc-scenarios-common--base-network-id-same-network (erc-server-flood-margin 30) erc-serv-buf-a erc-serv-buf-b) + (when (and id-a (zerop (random 2))) (setq id-a (symbol-name id-a))) + (when (and id-b (zerop (random 2))) (setq id-b (symbol-name id-b))) + (ert-info ("Connect to foonet with nick tester") (with-current-buffer (setq erc-serv-buf-a (erc :server "127.0.0.1" diff --git a/test/lisp/erc/resources/erc-scenarios-common.el b/test/lisp/erc/resources/erc-scenarios-common.el index d77b32984b..57d4658e75 100644 --- a/test/lisp/erc/resources/erc-scenarios-common.el +++ b/test/lisp/erc/resources/erc-scenarios-common.el @@ -185,7 +185,7 @@ erc-scenarios-common-with-cleanup (defun erc-scenarios-common-assert-initial-buf-name (id port) ;; Assert no limbo period when explicit ID given (should (string= (if id - (symbol-name id) + (format "%s" id) (format "127.0.0.1:%d" port)) (buffer-name)))) -- 2.38.1 --=-=-= Content-Type: text/x-patch; charset=utf-8 Content-Disposition: attachment; filename=0003-Limit-casemapping-to-appropriate-ranges-in-ERC.patch Content-Transfer-Encoding: quoted-printable >From f37e2fdf8a5958dd6122c38eefb7261f9983ac52 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 11 Dec 2022 19:41:43 -0800 Subject: [PATCH 3/5] Limit casemapping to appropriate ranges in ERC * lisp/erc/erc-common.el (erc-downcase): Use case table for `erc-downcase' so that case conversions are limited to the ASCII interval. * lisp/erc/erc.el (erc-casemapping--rfc1459-strict, erc--casemapping-rfc1459): Make these case tables instead of translation tables. The functions in case-table.el modify the standard syntax table, but that doesn't seem to make sense here, right? * test/lisp/erc/erc-tests.el (erc-downcase): Add cases showing mappings outside of the ASCII range. (Bug#59976.) --- lisp/erc/erc-common.el | 16 +++++----------- lisp/erc/erc.el | 28 ++++++++++++++++++++-------- test/lisp/erc/erc-tests.el | 3 +++ 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el index a4046ba9b3..e662c06daa 100644 --- a/lisp/erc/erc-common.el +++ b/lisp/erc/erc-common.el @@ -301,17 +301,11 @@ erc-log (defun erc-downcase (string) "Return a downcased copy of STRING with properties. Use the CASEMAPPING ISUPPORT parameter to determine the style." - (let* ((mapping (erc--get-isupport-entry 'CASEMAPPING 'single)) - (inhibit-read-only t)) - (if (equal mapping "ascii") - (downcase string) - (with-temp-buffer - (insert string) - (translate-region (point-min) (point-max) - (if (equal mapping "rfc1459-strict") - erc--casemapping-rfc1459-strict - erc--casemapping-rfc1459)) - (buffer-string))))) + (with-case-table (pcase (erc--get-isupport-entry 'CASEMAPPING 'single) + ("ascii" ascii-case-table) + ("rfc1459-strict" erc--casemapping-rfc1459-strict) + (_ erc--casemapping-rfc1459)) + (downcase string))) =20 (define-inline erc-get-channel-user (nick) "Find NICK in the current buffer's `erc-channel-users' hash table." diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el index a5ba45d9b3..195fb4c730 100644 --- a/lisp/erc/erc.el +++ b/lisp/erc/erc.el @@ -407,15 +407,27 @@ erc-server-users "Hash table of users on the current server. It associates nicknames with `erc-server-user' struct instances.") =20 -(defconst erc--casemapping-rfc1459 - (make-translation-table - '((?\[ . ?\{) (?\] . ?\}) (?\\ . ?\|) (?~ . ?^)) - (mapcar (lambda (c) (cons c (+ c 32))) "ABCDEFGHIJKLMNOPQRSTUVWXYZ"))) - (defconst erc--casemapping-rfc1459-strict - (make-translation-table - '((?\[ . ?\{) (?\] . ?\}) (?\\ . ?\|)) - (mapcar (lambda (c) (cons c (+ c 32))) "ABCDEFGHIJKLMNOPQRSTUVWXYZ"))) + (let ((tbl (copy-sequence ascii-case-table)) + (cup (copy-sequence (char-table-extra-slot ascii-case-table 0)))) + (set-char-table-extra-slot tbl 0 cup) + (set-char-table-extra-slot tbl 1 nil) + (set-char-table-extra-slot tbl 2 nil) + (pcase-dolist (`(,uc . ,lc) '((?\[ . ?\{) (?\] . ?\}) (?\\ . ?\|))) + (aset tbl uc lc) + (aset tbl lc lc) + (aset cup uc uc)) + tbl)) + +(defconst erc--casemapping-rfc1459 + (let ((tbl (copy-sequence erc--casemapping-rfc1459-strict)) + (cup (copy-sequence (char-table-extra-slot + erc--casemapping-rfc1459-strict 0)))) + (set-char-table-extra-slot tbl 0 cup) + (aset tbl ?~ ?^) + (aset tbl ?^ ?^) + (aset cup ?~ ?~) + tbl)) =20 (defun erc-add-server-user (nick user) "This function is for internal use only. diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el index 4d0d69cd7b..51c562f525 100644 --- a/test/lisp/erc/erc-tests.el +++ b/test/lisp/erc/erc-tests.el @@ -428,18 +428,21 @@ erc-downcase =20 (ert-info ("ascii") (puthash 'CASEMAPPING '("ascii") erc--isupport-params) + (should (equal (erc-downcase "ABC 123 =CE=94=CE=9E=CE=A9=CE=A3") "ab= c 123 =CE=94=CE=9E=CE=A9=CE=A3")) (should (equal (erc-downcase "Bob[m]`") "bob[m]`")) (should (equal (erc-downcase "Tilde~") "tilde~" )) (should (equal (erc-downcase "\\O/") "\\o/" ))) =20 (ert-info ("rfc1459") (puthash 'CASEMAPPING '("rfc1459") erc--isupport-params) + (should (equal (erc-downcase "ABC 123 =CE=94=CE=9E=CE=A9=CE=A3") "ab= c 123 =CE=94=CE=9E=CE=A9=CE=A3")) (should (equal (erc-downcase "Bob[m]`") "bob{m}`" )) (should (equal (erc-downcase "Tilde~") "tilde^" )) (should (equal (erc-downcase "\\O/") "|o/" ))) =20 (ert-info ("rfc1459-strict") (puthash 'CASEMAPPING '("rfc1459-strict") erc--isupport-params) + (should (equal (erc-downcase "ABC 123 =CE=94=CE=9E=CE=A9=CE=A3") "ab= c 123 =CE=94=CE=9E=CE=A9=CE=A3")) (should (equal (erc-downcase "Bob[m]`") "bob{m}`")) (should (equal (erc-downcase "Tilde~") "tilde~" )) (should (equal (erc-downcase "\\O/") "|o/" ))))) --=20 2.38.1 --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0004-Set-erc-network-to-a-given-ID-instead-of-failing.patch >From 5478db49a2949d2835b79ea460eac11853b190c3 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Mon, 12 Dec 2022 23:58:03 -0800 Subject: [PATCH 4/5] Set erc-network to a "given" ID instead of failing * lisp/erc/erc-networks.el (erc-networks--determine): Return the so-called "given" ID from a non-nil `:id' keyword arg passed to `erc' or `erc-tls'. (erc-networks--set-name): Tell the user about falling back to a given ID when the network can't be determined. Destroy the connection when network cannot be determined. (Bug#59976.) (erc-networks--ensure-announced): Include the fallback announced name in the error message. * test/lisp/erc/erc-networks-tests.el (erc-networks--set-name): Add dummy server process to test. --- lisp/erc/erc-networks.el | 34 +++++++++++++++++++---------- test/lisp/erc/erc-networks-tests.el | 2 ++ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/lisp/erc/erc-networks.el b/lisp/erc/erc-networks.el index 19a7ab8643..51ba54f45c 100644 --- a/lisp/erc/erc-networks.el +++ b/lisp/erc/erc-networks.el @@ -60,6 +60,7 @@ erc-session-server (declare-function erc-buffer-filter "erc" (predicate &optional proc)) (declare-function erc-current-nick "erc" nil) (declare-function erc-display-error-notice "erc" (parsed string)) +(declare-function erc-display-message "erc" (parsed type buffer msg &rest args)) (declare-function erc-error "erc" (&rest args)) (declare-function erc-get-buffer "erc" (target &optional proc)) (declare-function erc-server-buffer "erc" nil) @@ -1260,21 +1261,31 @@ erc-networks--determine return name) (and-let* ((vanity (erc--get-isupport-entry 'NETWORK 'single)) ((intern vanity)))) + (erc-networks--id-given erc-networks--id) erc-networks--name-missing-sentinel)) -(defun erc-networks--set-name (_proc parsed) +(defun erc-networks--set-name (proc parsed) "Set `erc-network' to the value returned by `erc-networks--determine'. -Signal an error when the network cannot be determined." +Signal an error when the network cannot be determined after first +shutting down the connection." ;; Always update (possibly clobber) current value, if any. (let ((name (erc-networks--determine))) + (when (eq name (erc-networks--id-given erc-networks--id)) + (let ((m (format "Couldn't determine network. Using given ID `%s'." + name))) + (erc-display-message parsed 'notice nil m))) (when (eq name erc-networks--name-missing-sentinel) - ;; This can happen theoretically, e.g., if you're editing some - ;; settings interactively on a proxy service that impersonates IRC - ;; but aren't being proxied through to a real network. The + ;; This can happen theoretically, e.g., when adjusting settings + ;; on a proxy service that partially impersonates IRC but isn't + ;; currently conveying anything through to a real network. The ;; service may send a 422 but no NETWORK param (or *any* 005s). - (let ((m (concat "Failed to determine network. Please set entry for " - erc-server-announced-name " in `erc-networks-alist'."))) + (let ((m (concat "Failed to determine network. Please set entry for \"" + erc-server-announced-name "\" in `erc-networks-alist'" + " or consider calling `erc-tls' with the keyword `:id'." + " See Info:\"(erc) Network Identifier\" for more."))) (erc-display-error-notice parsed m) + (run-hook-with-args 'erc-quit-hook proc) + (delete-process proc) (erc-error "Failed to determine network"))) ; beep (setq erc-network name)) nil) @@ -1287,11 +1298,12 @@ erc-networks--ensure-announced Copy source (prefix) from MOTD-ish message as a last resort." ;; The 004 handler never ran; see 2004-03-10 Diane Murray in change log (unless erc-server-announced-name - (erc-display-error-notice parsed "Failed to determine server name.") + (setq erc-server-announced-name (erc-response.sender parsed)) (erc-display-error-notice - parsed (concat "If this was unexpected, consider reporting it via " - (substitute-command-keys "\\[erc-bug]") ".")) - (setq erc-server-announced-name (erc-response.sender parsed))) + parsed (concat "Failed to determine server name. Using \"" + erc-server-announced-name "\" instead." + " If this was unexpected, consider reporting it via " + (substitute-command-keys "\\[erc-bug]") "."))) nil) (defun erc-unset-network-name (_nick _ip _reason) diff --git a/test/lisp/erc/erc-networks-tests.el b/test/lisp/erc/erc-networks-tests.el index fc12bf7ce3..02fc57d4e6 100644 --- a/test/lisp/erc/erc-networks-tests.el +++ b/test/lisp/erc/erc-networks-tests.el @@ -1171,6 +1171,8 @@ erc-networks--set-name (let (erc-server-announced-name (erc--isupport-params (make-hash-table)) erc-network + erc-quit-hook + (erc-server-process (erc-networks-tests--create-live-proc)) calls) (erc-mode) -- 2.38.1 --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0005-Add-escape-hatch-for-picky-servers-in-erc-sasl.patch >From 81e2668d2d20025c38f780b9238e722f5d4e0317 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Mon, 12 Dec 2022 23:58:03 -0800 Subject: [PATCH 5/5] Add escape hatch for picky servers in erc-sasl * lisp/erc/erc-sasl.el (erc-sasl--send-cap-ls): Add internal switch to toggle feigning better support for IRCv3 capability negotiation by sending an opening "CAP LS" and holding off on the "CAP REQ" until after "USER". (erc--register-connection): Possibly send a "CAP LS" before anything. Also, don't attempt to send `erc-session-username' when it holds an SASL username because the latter may contain protocol-defying characters. * test/lisp/erc/resources/base/local-modules/fourth.eld: change user parameter of "USER" command to reflect nick when `erc-sasl-user' is set to `:user'. (Bug#59976.) --- lisp/erc/erc-sasl.el | 33 +++++++++++++++---- .../resources/base/local-modules/fourth.eld | 2 +- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/lisp/erc/erc-sasl.el b/lisp/erc/erc-sasl.el index 5b2c93988a..9486250529 100644 --- a/lisp/erc/erc-sasl.el +++ b/lisp/erc/erc-sasl.el @@ -414,17 +414,38 @@ erc-sasl--destroy " ")) (erc-sasl--destroy proc)) +;; The rationale for not enabling this by default is twofold: +;; +;; - It more strongly implies that ERC supports client capability +;; negotiation, which is therefore more disingenuous. +;; +;; - We'd still be "faking it" by firing and forgetting, and more +;; balls in the air makes things less predictable. + +(defvar erc-sasl--send-cap-ls nil + "Whether to send an opening \"CAP LS\" command. +This is an escape hatch for picky servers expecting this command. +If you need this, please let us know via \\[erc-bug], so we can +offer a user option instead.") + (cl-defmethod erc--register-connection (&context (erc-sasl-mode (eql t))) - "Send speculative/pipelined CAP and AUTHENTICATE and hope for the best." + "Send speculative CAP and pipelined AUTHENTICATE and hope for the best." (if-let* ((c (erc-sasl--state-client erc-sasl--state)) (m (sasl-mechanism-name (sasl-client-mechanism c)))) (progn - (erc-server-send "CAP REQ :sasl") - (if (and erc-session-password - (eq :password (alist-get 'password erc-sasl--options))) - (let (erc-session-password) - (erc-login)) + (erc-server-send (if erc-sasl--send-cap-ls "CAP LS" "CAP REQ :sasl")) + (let ((erc-session-password + (and erc-session-password + (not (eq :password (alist-get 'password erc-sasl--options))) + erc-session-password)) + (erc-session-username + ;; The username may contain a colon or a space + (if (eq :user (alist-get 'user erc-sasl--options)) + (erc-current-nick) + erc-session-username))) (erc-login)) + (when erc-sasl--send-cap-ls + (erc-server-send "CAP REQ :sasl")) (erc-server-send (format "AUTHENTICATE %s" m))) (erc-sasl--destroy erc-server-process))) diff --git a/test/lisp/erc/resources/base/local-modules/fourth.eld b/test/lisp/erc/resources/base/local-modules/fourth.eld index fd6d62b6cc..4ac5dcbd38 100644 --- a/test/lisp/erc/resources/base/local-modules/fourth.eld +++ b/test/lisp/erc/resources/base/local-modules/fourth.eld @@ -1,7 +1,7 @@ ;; -*- mode: lisp-data; -*- ((cap 10 "CAP REQ :sasl")) ((nick 10 "NICK tester`")) -((user 10 "USER tester 0 * :tester")) +((user 10 "USER tester` 0 * :tester")) ((authenticate 10 "AUTHENTICATE PLAIN") (0.0 ":irc.foonet.org CAP * ACK sasl") -- 2.38.1 --=-=-=--