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 Subject: bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict Date: Mon, 12 Dec 2022 06:35:00 -0800 Message-ID: <87pmcowuzv.fsf__6904.44567460276$1670855946$gmane$org@neverwas.me> References: <20221212000031.2f679e5e@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="19142"; 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 Mon Dec 12 15:38:58 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 1p4jxJ-0004m1-9u for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 12 Dec 2022 15:38:57 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1p4jvC-0004cb-86; Mon, 12 Dec 2022 09:36:46 -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 1p4juV-0004HX-BU for bug-gnu-emacs@gnu.org; Mon, 12 Dec 2022 09:36:09 -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 1p4juV-0003BV-0t for bug-gnu-emacs@gnu.org; Mon, 12 Dec 2022 09:36:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1p4juU-0007c6-Gc for bug-gnu-emacs@gnu.org; Mon, 12 Dec 2022 09:36: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: Mon, 12 Dec 2022 14:36: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.167085571629253 (code B ref 59976); Mon, 12 Dec 2022 14:36:02 +0000 Original-Received: (at 59976) by debbugs.gnu.org; 12 Dec 2022 14:35:16 +0000 Original-Received: from localhost ([127.0.0.1]:53360 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1p4jtj-0007bl-DL for submit@debbugs.gnu.org; Mon, 12 Dec 2022 09:35:16 -0500 Original-Received: from mail-108-mta18.mxroute.com ([136.175.108.18]:46511) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1p4jth-0007bb-1I for 59976@debbugs.gnu.org; Mon, 12 Dec 2022 09:35:14 -0500 Original-Received: from mail-111-mta2.mxroute.com ([136.175.111.2] filter006.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta18.mxroute.com (ZoneMTA) with ESMTPSA id 18506c2bbf00001d7e.001 for <59976@debbugs.gnu.org> (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256); Mon, 12 Dec 2022 14:35:03 +0000 X-Zone-Loop: c3dd63d4ae954da207bbc3c940ce120facd2478191bf 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=3zgm2vN+Dz63mr9thtd2zCHudqL/m01NnT5kf/3tWfs=; b=mmkBokIjA44r1CCfoGBjfHKkeQ JXcmtUtLGmb29/PWLhIiFl1GF+RYekdExBkN4DnRhWVA2wDONwCXgJBnO03OTa5itTyEqzliJvvVv 7yc/6dW7XHjyLVLzeHvhplgbRB7mDKHl93GESQn3HFsm0DyR+a9Ltx5raDuZlH2pz59o5evMzV2Ft +ivg3CIwCRlaaxnFl48sAM5DXyexP1mYD1zJ2Eq18BWXTmAqZAaCRERqRCcittfdLcD1KcrmFIZe6 mtmpZhtuI7uMBRfTCtRyjUp3Sj8lbnxHgP62lSrci6LAG01iF7zqcXfyUPfuFMMg0kkhzCKekzPQK pEz6g3jg==; In-Reply-To: <20221212000031.2f679e5e@malediction> (Mike Kazantsev's message of "Mon, 12 Dec 2022 00:00:31 +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:250706 Archived-At: --=-=-= Content-Type: text/plain Mike Kazantsev writes: > Hi. Hi Mike, Thanks for the detailed bug report. I mostly just have questions for now but am committed to finding a solution, so please bear with me. > I've needed to use case-sensitive channel names (with matrix2051 ad-hoc > ircd), wanted to test updated ERC client for that, and stumbled upon > what looks like a bug there: > > When IRC server is named "slack", and then it sends you a message from > user named "slack", ERC tries to open query-buffer "slack", ends up > running erc--open-target("slack") which does kill-all-local-variables() > when enabling erc-mode in the buffer. > > That removes erc-networks--id value there and from then on ERC keeps > spamming "error in process filter" about erc-networks--id being nil. > > > - What I'm trying to do: > > - (erc-tls ...) ;; cleates "slack" server buffer and some channels. > - Open "slack" buffer 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'"? > - Send "/ping myuser" IRC-command there > 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.) > - Expected result: > > Command actually results in PRIVMSG from "slack" user on this > specific ircd, so I expect to see that open a separate query-buffer > where that message is displayed. > > (message in question - though probably doesn't matter: > "msg: ^APING 1670781447.080567^A could not be sent channel_not_found") > > > - Actual result: > > - Emacs slows down while printing following errors to MiniBuffer and *Messages*: > > error in process filter: or: Wrong type argument: erc-networks--id, nil > error in process filter: Wrong type argument: erc-networks--id, nil > > - As far as I can tell that IRC connection becomes unusable, and > errors like above get signaled from then on randomly, likely on > some commands sent from ircd. > > > - Workaround: changing announced name of the server to something else > helps - "slack" query-buffer gets created and is separate from server buffer. You mean by changing `erc-server-announced-name'? > 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 = %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) 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. 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 some kind of disambiguation or conflict-detection for > query-channel names might be either missing or not working atm, > but is needed to avoid this happening unintentionally, or maybe > even on purpose (e.g. to annoy someone by cutting their IRC connection). Agreed. Upon detecting the collision, perhaps the query buffer needs to become "slack!~slack@example.com" (or "slack@slack/me") and/or the server buffer "slack/me". Additionally, we could have an option that says to always use full n!u@h's when naming query buffers instead of adapting dynamically. 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 :server "127.0.0.1" :port 2051 ... :id 'Slack 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. 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. > 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 unlikely. Unlikely, yes. But I sometimes worry about ERC's loaddefs when people manually shadow/replace instead of building alongside Emacs or installing from devel: (push '("devel" . "https://elpa.gnu.org/devel/") package-archives) (Not saying you need to do either, of course.) Thanks again for the very nice bug report. J.P. P.S. I've attached some patches for Emacs 29 that address issues discovered while briefly looking into this bug; the first is totally unrelated and the latter two only tangentially so (in case anyone wants to take a gander). --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-Be-carefuller-updating-browse-url-var-in-erc-compat.patch >From d7887fb9059ecd36d7209a98af2f4ff649e8642b Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 11 Dec 2022 19:16:07 -0800 Subject: [PATCH 1/3] ; Be carefuller 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 | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lisp/erc/erc-compat.el b/lisp/erc/erc-compat.el index bd93254758..884abaf824 100644 --- a/lisp/erc/erc-compat.el +++ b/lisp/erc/erc-compat.el @@ -389,10 +389,17 @@ erc-compat--29-browse-url-irc url-irc-function))) (url-irc url))) +(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)) + browse-url-default-handlers + :key #'car + :test (lambda (_ b) + (and (stringp b) + (string-match-p b "irc://localhost"))))) ((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 c6423e8834e7ed7101e800931bc461a960083aa9 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 11 Dec 2022 19:41:43 -0800 Subject: [PATCH 2/3] 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. --- 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 28eb435ca3756ecde6791162e7ef8de1018113b2 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 11 Dec 2022 19:41:43 -0800 Subject: [PATCH 3/3] 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. --- 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..e30452b1cf 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=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 "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 "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 "Bob[m]`") "bob{m}`")) (should (equal (erc-downcase "Tilde~") "tilde~" )) (should (equal (erc-downcase "\\O/") "|o/" ))))) --=20 2.38.1 --=-=-=--