* bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict
@ 2022-12-11 19:00 Mike Kazantsev
2022-12-12 14:35 ` J.P.
[not found] ` <87pmcowuzv.fsf@neverwas.me>
0 siblings, 2 replies; 8+ messages in thread
From: Mike Kazantsev @ 2022-12-11 19:00 UTC (permalink / raw)
To: 59976; +Cc: emacs-erc
Hi.
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
- Send "/ping myuser" IRC-command there
- 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.
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)
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).
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.
Thanks.
--
Mike Kazantsev // fraggod.net
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict
2022-12-11 19:00 bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict Mike Kazantsev
@ 2022-12-12 14:35 ` J.P.
[not found] ` <87pmcowuzv.fsf@neverwas.me>
1 sibling, 0 replies; 8+ messages in thread
From: J.P. @ 2022-12-12 14:35 UTC (permalink / raw)
To: Mike Kazantsev; +Cc: 59976, emacs-erc
[-- Attachment #1: Type: text/plain, Size: 5627 bytes --]
Mike Kazantsev <mk.fraggod@gmail.com> 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).
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Be-carefuller-updating-browse-url-var-in-erc-compat.patch --]
[-- Type: text/x-patch, Size: 1476 bytes --]
From d7887fb9059ecd36d7209a98af2f4ff649e8642b Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Actually-accept-non-symbols-as-IDs-in-erc-open.patch --]
[-- Type: text/x-patch, Size: 3351 bytes --]
From c6423e8834e7ed7101e800931bc461a960083aa9 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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
[-- Attachment #4: 0003-Limit-casemapping-to-appropriate-ranges-in-ERC.patch --]
[-- Type: text/x-patch, Size: 5004 bytes --]
From 28eb435ca3756ecde6791162e7ef8de1018113b2 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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)))
(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.")
-(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))
(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
(ert-info ("ascii")
(puthash 'CASEMAPPING '("ascii") erc--isupport-params)
+ (should (equal (erc-downcase "ABC 123 ΣΛΣΝΟΣ") "abc 123 ΣΛΣΝΟΣ"))
(should (equal (erc-downcase "Bob[m]`") "bob[m]`"))
(should (equal (erc-downcase "Tilde~") "tilde~" ))
(should (equal (erc-downcase "\\O/") "\\o/" )))
(ert-info ("rfc1459")
(puthash 'CASEMAPPING '("rfc1459") erc--isupport-params)
+ (should (equal (erc-downcase "ABC 123 ΣΛΣΝΟΣ") "abc 123 ΣΛΣΝΟΣ"))
(should (equal (erc-downcase "Bob[m]`") "bob{m}`" ))
(should (equal (erc-downcase "Tilde~") "tilde^" ))
(should (equal (erc-downcase "\\O/") "|o/" )))
(ert-info ("rfc1459-strict")
(puthash 'CASEMAPPING '("rfc1459-strict") erc--isupport-params)
+ (should (equal (erc-downcase "ABC 123 ΣΛΣΝΟΣ") "abc 123 ΣΛΣΝΟΣ"))
(should (equal (erc-downcase "Bob[m]`") "bob{m}`"))
(should (equal (erc-downcase "Tilde~") "tilde~" ))
(should (equal (erc-downcase "\\O/") "|o/" )))))
--
2.38.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict
[not found] ` <87pmcowuzv.fsf@neverwas.me>
@ 2022-12-12 15:35 ` Mike Kazantsev
[not found] ` <20221212203508.3cd44bb6@malediction>
1 sibling, 0 replies; 8+ messages in thread
From: Mike Kazantsev @ 2022-12-12 15:35 UTC (permalink / raw)
To: J.P.; +Cc: 59976, emacs-erc
On Mon, 12 Dec 2022 06:35:00 -0800
"J.P." <jp@neverwas.me> wrote:
> Mike Kazantsev <mk.fraggod@gmail.com> writes:
> > 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'"?
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.
> > - 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.)
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.
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=rfc3454 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.
> > - 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'?
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.
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 '🖧-slack symbol with that unicode prefix
exactly to avoid any name conflicts)
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
end up with same result, e.g. 'slack in erc-network, just like it
works with 'OFTC in there.
> > 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 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.
> > 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.
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.
> 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/issues.
> > 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)
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)
> (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.
>
> 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).
--
Mike Kazantsev // fraggod.net
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict
[not found] ` <20221212203508.3cd44bb6@malediction>
@ 2022-12-13 15:13 ` J.P.
2022-12-15 14:16 ` J.P.
[not found] ` <87sfhgn45z.fsf@neverwas.me>
0 siblings, 2 replies; 8+ messages in thread
From: J.P. @ 2022-12-13 15:13 UTC (permalink / raw)
To: Mike Kazantsev; +Cc: 59976, emacs-erc
[-- Attachment #1: Type: text/plain, Size: 11101 bytes --]
Mike Kazantsev <mk.fraggod@gmail.com> writes:
>> > - 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'"?
>
> 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
>>
>> 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=rfc3454 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 server buffer.
>>
>> 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 '🖧-slack symbol with that unicode prefix
> 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
> 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 = %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.
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
>>
>> :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.
>
> 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/issues.
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 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)
>
> 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.
[-- Attachment #2: 0000-v1-v2.diff --]
[-- Type: text/x-patch, Size: 11045 bytes --]
From 81e2668d2d20025c38f780b9238e722f5d4e0317 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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)))
-(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))
-(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/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/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)
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
(ert-info ("ascii")
(puthash 'CASEMAPPING '("ascii") erc--isupport-params)
- (should (equal (erc-downcase "ABC 123 ΣΛΣΝΟΣ") "abc 123 ΣΛΣΝΟΣ"))
+ (should (equal (erc-downcase "ABC 123 ΔΞΩΣ") "abc 123 ΔΞΩΣ"))
(should (equal (erc-downcase "Bob[m]`") "bob[m]`"))
(should (equal (erc-downcase "Tilde~") "tilde~" ))
(should (equal (erc-downcase "\\O/") "\\o/" )))
(ert-info ("rfc1459")
(puthash 'CASEMAPPING '("rfc1459") erc--isupport-params)
- (should (equal (erc-downcase "ABC 123 ΣΛΣΝΟΣ") "abc 123 ΣΛΣΝΟΣ"))
+ (should (equal (erc-downcase "ABC 123 ΔΞΩΣ") "abc 123 ΔΞΩΣ"))
(should (equal (erc-downcase "Bob[m]`") "bob{m}`" ))
(should (equal (erc-downcase "Tilde~") "tilde^" ))
(should (equal (erc-downcase "\\O/") "|o/" )))
(ert-info ("rfc1459-strict")
(puthash 'CASEMAPPING '("rfc1459-strict") erc--isupport-params)
- (should (equal (erc-downcase "ABC 123 ΣΛΣΝΟΣ") "abc 123 ΣΛΣΝΟΣ"))
+ (should (equal (erc-downcase "ABC 123 ΔΞΩΣ") "abc 123 ΔΞΩΣ"))
(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/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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Be-nicer-when-updating-browse-url-var-in-erc-compat.patch --]
[-- Type: text/x-patch, Size: 1388 bytes --]
From 7f832bce68a327d1930eac71f2d937c88e34055e Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0002-Actually-accept-non-symbols-as-IDs-in-erc-open.patch --]
[-- Type: text/x-patch, Size: 3365 bytes --]
From 1b40fcdbe664ba4b364b7665370c66de82998038 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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
[-- Attachment #5: 0003-Limit-casemapping-to-appropriate-ranges-in-ERC.patch --]
[-- Type: text/x-patch, Size: 4994 bytes --]
From f37e2fdf8a5958dd6122c38eefb7261f9983ac52 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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)))
(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.")
-(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))
(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
(ert-info ("ascii")
(puthash 'CASEMAPPING '("ascii") erc--isupport-params)
+ (should (equal (erc-downcase "ABC 123 ΔΞΩΣ") "abc 123 ΔΞΩΣ"))
(should (equal (erc-downcase "Bob[m]`") "bob[m]`"))
(should (equal (erc-downcase "Tilde~") "tilde~" ))
(should (equal (erc-downcase "\\O/") "\\o/" )))
(ert-info ("rfc1459")
(puthash 'CASEMAPPING '("rfc1459") erc--isupport-params)
+ (should (equal (erc-downcase "ABC 123 ΔΞΩΣ") "abc 123 ΔΞΩΣ"))
(should (equal (erc-downcase "Bob[m]`") "bob{m}`" ))
(should (equal (erc-downcase "Tilde~") "tilde^" ))
(should (equal (erc-downcase "\\O/") "|o/" )))
(ert-info ("rfc1459-strict")
(puthash 'CASEMAPPING '("rfc1459-strict") erc--isupport-params)
+ (should (equal (erc-downcase "ABC 123 ΔΞΩΣ") "abc 123 ΔΞΩΣ"))
(should (equal (erc-downcase "Bob[m]`") "bob{m}`"))
(should (equal (erc-downcase "Tilde~") "tilde~" ))
(should (equal (erc-downcase "\\O/") "|o/" )))))
--
2.38.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0004-Set-erc-network-to-a-given-ID-instead-of-failing.patch --]
[-- Type: text/x-patch, Size: 5183 bytes --]
From 5478db49a2949d2835b79ea460eac11853b190c3 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #7: 0005-Add-escape-hatch-for-picky-servers-in-erc-sasl.patch --]
[-- Type: text/x-patch, Size: 3721 bytes --]
From 81e2668d2d20025c38f780b9238e722f5d4e0317 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict
2022-12-13 15:13 ` J.P.
@ 2022-12-15 14:16 ` J.P.
[not found] ` <87sfhgn45z.fsf@neverwas.me>
1 sibling, 0 replies; 8+ messages in thread
From: J.P. @ 2022-12-15 14:16 UTC (permalink / raw)
To: Mike Kazantsev; +Cc: 59976, emacs-erc
[-- Attachment #1: Type: text/plain, Size: 1827 bytes --]
"J.P." <jp@neverwas.me> writes:
>>> 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/issues.
>
> 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.
Actually, an egregious oversight has come to light that makes this a
more general (and more pressing) matter relevant to Emacs 29. Currently,
renaming queries fails if *any* buffer, even a non-ERC buffer, exists
whose name matches that of the target in question. And, AFAICT, no
amount of :id or options twiddling can serve as a workaround. (If this
is what you've been getting at this whole time, then apologies: I'm
rather thick headed, if you haven't noticed.)
Anyhow, I have attempted to address this in the attached patch. If you
or anyone out there is willing, please install it locally atop the
current lisp/erc subtree on the emacs-29 branch and see if you can break
it. Thanks in advance!
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-some-naming-issues-involving-query-buffers-in-ER.patch --]
[-- Type: text/x-patch, Size: 15824 bytes --]
From 22a90166a40f6f333621c05839e1b7a779ed3813 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 12 Dec 2022 07:38:44 -0800
Subject: [PATCH] Fix some naming issues involving query buffers in ERC
* lisp/erc/erc-networks.el
(erc-networks-rename-surviving-target-buffer): Don't kill a surviving
query buffer when another non-target buffer, possibly not even
belonging to ERC, already exists with the target name.
(erc-networks--reconcile-buffer-names): Always append a network-ID
suffix to a target buffer's name if another buffer of that name
already exists. (Bug#59976.)
* test/lisp/erc/erc-networks-tests.el:
(erc-networks-rename-surviving-target-buffer--query-non-target): Add
new test.
* test/lisp/erc/erc-scenarios-base-association-query.el: New file.
* test/lisp/erc/resources/base/assoc/queries/netnick.eld: New file.
* test/lisp/erc/resources/base/assoc/queries/non-erc.eld: New file.
---
lisp/erc/erc-networks.el | 24 ++--
test/lisp/erc/erc-networks-tests.el | 30 +++++
.../erc-scenarios-base-association-query.el | 107 ++++++++++++++++++
.../resources/base/assoc/queries/netnick.eld | 43 +++++++
.../resources/base/assoc/queries/non-erc.eld | 34 ++++++
5 files changed, 227 insertions(+), 11 deletions(-)
create mode 100644 test/lisp/erc/erc-scenarios-base-association-query.el
create mode 100644 test/lisp/erc/resources/base/assoc/queries/netnick.eld
create mode 100644 test/lisp/erc/resources/base/assoc/queries/non-erc.eld
diff --git a/lisp/erc/erc-networks.el b/lisp/erc/erc-networks.el
index fd8bed470a..30a72a5c6a 100644
--- a/lisp/erc/erc-networks.el
+++ b/lisp/erc/erc-networks.el
@@ -1097,7 +1097,8 @@ erc-networks-rename-surviving-target-buffer
(erc--target-symbol erc--target))))))))
((not (cdr others))))
(with-current-buffer (car others)
- (rename-buffer (erc--target-string target)))))
+ (unless (get-buffer (erc--target-string target))
+ (rename-buffer (erc--target-string target))))))
(defun erc-networks-shrink-ids-and-buffer-names ()
"Recompute network IDs and buffer names, ignoring the current buffer.
@@ -1188,19 +1189,20 @@ erc-networks--reconcile-buffer-names
(erc--target-string target)))
placeholder)
;; If we don't exist, claim name temporarily while renaming others
- (when-let* (namesakes
- (ex (get-buffer name))
- ((not (memq ex existing)))
- (temp-name (generate-new-buffer-name (format "*%s*" name))))
- (setq existing (remq ex existing))
- (with-current-buffer ex
- (rename-buffer temp-name)
- (setq placeholder (get-buffer-create name))
- (rename-buffer name 'unique)))
+ (when-let* ((ex (get-buffer name))
+ ((not (memq ex existing))))
+ (if namesakes ; if namesakes is nonempty, it contains ex
+ (with-current-buffer ex
+ (let ((temp-name (generate-new-buffer-name (format "*%s*" name))))
+ (rename-buffer temp-name)
+ (setq placeholder (get-buffer-create name))
+ (rename-buffer name 'unique)))
+ ;; This must be a server buffer or a non-ERC buffer
+ (setq name (erc-networks--construct-target-buffer-name target))))
(unless (with-suppressed-warnings ((obsolete erc-reuse-buffers))
erc-reuse-buffers)
(when (string-suffix-p ">" name)
- (setq name (substring name 0 -3))))
+ (setq name (string-trim-right name (rx "<" (+ digit) ">")))))
(dolist (ex (erc-networks--id-sort-buffers existing))
(with-current-buffer ex
(rename-buffer name 'unique)))
diff --git a/test/lisp/erc/erc-networks-tests.el b/test/lisp/erc/erc-networks-tests.el
index e883174e28..0a8b5935df 100644
--- a/test/lisp/erc/erc-networks-tests.el
+++ b/test/lisp/erc/erc-networks-tests.el
@@ -197,6 +197,36 @@ erc-networks-rename-surviving-target-buffer--query
(erc-networks-tests--clean-bufs))
+;; A non-ERC buffer exists named "bob", and we're killing one of two
+;; ERC target buffers named "bob@<netid>". The surviving buffer
+;; retains its suffix.
+
+(ert-deftest erc-networks-rename-surviving-target-buffer--query-non-target ()
+ (should (memq #'erc-networks-rename-surviving-target-buffer
+ erc-kill-buffer-hook))
+
+ (let ((existing (get-buffer-create "bob"))
+ (bob-foonet (get-buffer-create "bob@foonet")))
+
+ (with-current-buffer bob-foonet
+ (erc-mode)
+ (setq erc-networks--id (make-erc-networks--id-qualifying
+ :parts [foonet "bob"] :len 1)
+ erc--target (erc--target-from-string "bob")))
+
+ (with-current-buffer (get-buffer-create "bob@barnet")
+ (erc-mode)
+ (setq erc-networks--id (make-erc-networks--id-qualifying
+ :parts [barnet "bob"] :len 1)
+ erc--target (erc--target-from-string "bob")))
+
+ (kill-buffer "bob@barnet")
+ (should (buffer-live-p existing))
+ (should (buffer-live-p bob-foonet))
+ (kill-buffer existing))
+
+ (erc-networks-tests--clean-bufs))
+
(ert-deftest erc-networks-rename-surviving-target-buffer--multi ()
(ert-info ("Multiple leftover channels untouched")
diff --git a/test/lisp/erc/erc-scenarios-base-association-query.el b/test/lisp/erc/erc-scenarios-base-association-query.el
new file mode 100644
index 0000000000..78b75a530c
--- /dev/null
+++ b/test/lisp/erc/erc-scenarios-base-association-query.el
@@ -0,0 +1,107 @@
+;;; erc-scenarios-base-association-query.el --- assoc query scenarios -*- lexical-binding: t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert-x)
+(eval-and-compile
+ (let ((load-path (cons (ert-resource-directory) load-path)))
+ (require 'erc-scenarios-common)))
+
+(eval-when-compile (require 'erc-join))
+
+
+;; Non-ERC buffers exist whose names match the nicknames of query
+;; targets, both newly arriving and outgoing. No target buffers yet
+;; exist for these, so new ones are created that feature a net-ID
+;; @suffix.
+
+(ert-deftest erc-scenarios-base-association-existing-non-erc-buffer ()
+ :tags '(:expensive-test)
+ (erc-scenarios-common-with-cleanup
+ ((erc-scenarios-common-dialog "base/assoc/queries")
+ (dumb-server (erc-d-run "localhost" t 'non-erc))
+ (port (process-contact dumb-server :service))
+ (expect (erc-d-t-make-expecter))
+ (nitwit (with-current-buffer (get-buffer-create "nitwit")
+ (prin1 (ert-test-name (ert-running-test)) (current-buffer))
+ (current-buffer))) ; these are killed on completion by macro
+ (dummy (with-current-buffer (get-buffer-create "dummy")
+ (prin1 (ert-test-name (ert-running-test)) (current-buffer))
+ (current-buffer)))
+ (erc-server-flood-penalty 0.1))
+
+ (ert-info ("Connect to foonet")
+ (with-current-buffer (erc :server "127.0.0.1"
+ :port port
+ :nick "tester"
+ :user "tester"
+ :full-name "tester")
+ (erc-scenarios-common-assert-initial-buf-name nil port)
+ (erc-d-t-wait-for 5 (eq erc-network 'foonet))
+ (funcall expect 15 "debug mode")))
+
+ (ert-info ("Nick dummy queries us")
+ (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "dummy@foonet"))
+ (should (erc-query-buffer-p))
+ (funcall expect 5 "hi")
+
+ (ert-info ("We query nick nitwit")
+ (with-current-buffer (erc-cmd-QUERY "nitwit")
+ (should (equal (buffer-name) "nitwit@foonet"))
+ (erc-scenarios-common-say "hola")
+ (funcall expect 5 "ciao")))
+
+ (erc-scenarios-common-say "howdy")
+ (funcall expect 5 "bye")
+ (erc-cmd-QUIT "")))))
+
+;; Someone sending you a PM has the same name as the network (bug#59976)
+
+(ert-deftest erc-scenarios-base-association-some-nick-is-network ()
+ :tags '(:expensive-test)
+ (erc-scenarios-common-with-cleanup
+ ((erc-scenarios-common-dialog "base/assoc/queries")
+ (dumb-server (erc-d-run "localhost" t 'netnick))
+ (port (process-contact dumb-server :service))
+ (expect (erc-d-t-make-expecter))
+ (erc-server-flood-penalty 0.5))
+
+ (ert-info ("Connect to foonet")
+ (with-current-buffer (erc :server "127.0.0.1"
+ :port port
+ :nick "tester"
+ :user "tester"
+ :full-name "tester")
+ (erc-scenarios-common-assert-initial-buf-name nil port)
+ (erc-d-t-wait-for 5 (eq erc-network 'foonet))))
+
+ (ert-info ("Join common channel as nick foonet")
+ (with-current-buffer "foonet"
+ (funcall expect 15 "debug mode")
+ (erc-cmd-JOIN "#chan"))
+ (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
+ (funcall expect 5 "welcome")))
+
+ (ert-info ("Nick foonet PMs us")
+ (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "foonet@foonet"))
+ (should (erc-query-buffer-p))
+ (funcall expect 5 "hi")))))
+
+;;; erc-scenarios-base-association-query.el ends here
diff --git a/test/lisp/erc/resources/base/assoc/queries/netnick.eld b/test/lisp/erc/resources/base/assoc/queries/netnick.eld
new file mode 100644
index 0000000000..9493a7c057
--- /dev/null
+++ b/test/lisp/erc/resources/base/assoc/queries/netnick.eld
@@ -0,0 +1,43 @@
+;; -*- mode: lisp-data; -*-
+((nick 10 "NICK tester"))
+((user 1 "USER tester 0 * :tester")
+ (0.00 ":irc.foonet.org 001 tester :Welcome to the foonet IRC Network tester")
+ (0.00 ":irc.foonet.org 002 tester :Your host is irc.foonet.org, running version ergo-v2.8.0")
+ (0.00 ":irc.foonet.org 003 tester :This server was created Mon, 12 Dec 2022 01:25:38 UTC")
+ (0.00 ":irc.foonet.org 004 tester irc.foonet.org ergo-v2.8.0 BERTZios CEIMRUabefhiklmnoqstuv Iabefhkloqv")
+ (0.00 ":irc.foonet.org 005 tester AWAYLEN=390 BOT=B CASEMAPPING=ascii CHANLIMIT=#:100 CHANMODES=Ibe,k,fl,CEMRUimnstu CHANNELLEN=64 CHANTYPES=# ELIST=U EXCEPTS EXTBAN=,m FORWARD=f INVEX KICKLEN=390 :are supported by this server")
+ (0.00 ":irc.foonet.org 005 tester MAXLIST=beI:60 MAXTARGETS=4 MODES MONITOR=100 NETWORK=foonet NICKLEN=32 PREFIX=(qaohv)~&@%+ STATUSMSG=~&@%+ TARGMAX=NAMES:1,LIST:1,KICK:,WHOIS:1,USERHOST:10,PRIVMSG:4,TAGMSG:4,NOTICE:4,MONITOR:100 TOPICLEN=390 UTF8MAPPING=rfc8265 UTF8ONLY WHOX :are supported by this server")
+ (0.01 ":irc.foonet.org 005 tester draft/CHATHISTORY=100 :are supported by this server")
+ (0.00 ":irc.foonet.org 251 tester :There are 0 users and 4 invisible on 1 server(s)")
+ (0.00 ":irc.foonet.org 252 tester 0 :IRC Operators online")
+ (0.00 ":irc.foonet.org 253 tester 0 :unregistered connections")
+ (0.00 ":irc.foonet.org 254 tester 1 :channels formed")
+ (0.00 ":irc.foonet.org 255 tester :I have 4 clients and 0 servers")
+ (0.00 ":irc.foonet.org 265 tester 4 4 :Current local users 4, max 4")
+ (0.01 ":irc.foonet.org 266 tester 4 4 :Current global users 4, max 4")
+ (0.00 ":irc.foonet.org 422 tester :MOTD File is missing"))
+
+((mode 10 "MODE tester +i")
+ (0.00 ":irc.foonet.org 221 tester +i")
+ (0.00 ":irc.foonet.org NOTICE tester :This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect.")
+ (0.02 ":irc.foonet.org 221 tester +i"))
+
+((join 10 "JOIN #chan")
+ (0.03 ":tester!~u@z5d6jyn8pwxge.irc JOIN #chan"))
+
+((mode-1 10 "MODE #chan")
+ (0.01 ":irc.foonet.org 353 tester = #chan :@alice bob foonet tester")
+ (0.00 ":irc.foonet.org 366 tester #chan :End of NAMES list")
+ (0.03 ":irc.foonet.org 324 tester #chan +nt")
+ (0.00 ":irc.foonet.org 329 tester #chan 1670808354")
+ (0.00 ":bob!~u@d6ftaiqzk8x2k.irc PRIVMSG #chan :tester, welcome!")
+ (0.00 ":alice!~u@d6ftaiqzk8x2k.irc PRIVMSG #chan :tester, welcome!")
+
+ (0.00 ":foonet!~u@z5d6jyn8pwxge.irc PRIVMSG tester :hi")
+
+ (0.03 ":bob!~u@d6ftaiqzk8x2k.irc PRIVMSG #chan :alice: Forbear it therefore; give your cause to heaven.")
+ (0.01 ":alice!~u@d6ftaiqzk8x2k.irc PRIVMSG #chan :bob: Even at thy teat thou hadst thy tyranny.")
+ (0.00 ":foonet!~u@z5d6jyn8pwxge.irc QUIT :connection closed"))
+
+((quit 10 "QUIT :\2ERC\2")
+ (0.03 ":tester!~u@z5d6jyn8pwxge.irc QUIT :Quit: \2ERC\2"))
diff --git a/test/lisp/erc/resources/base/assoc/queries/non-erc.eld b/test/lisp/erc/resources/base/assoc/queries/non-erc.eld
new file mode 100644
index 0000000000..69b9ac8f69
--- /dev/null
+++ b/test/lisp/erc/resources/base/assoc/queries/non-erc.eld
@@ -0,0 +1,34 @@
+;; -*- mode: lisp-data; -*-
+((nick 10 "NICK tester"))
+((user 1 "USER tester 0 * :tester")
+ (0.00 ":irc.foonet.org 001 tester :Welcome to the foonet IRC Network tester")
+ (0.00 ":irc.foonet.org 002 tester :Your host is irc.foonet.org, running version ergo-v2.8.0")
+ (0.00 ":irc.foonet.org 003 tester :This server was created Mon, 12 Dec 2022 01:25:38 UTC")
+ (0.00 ":irc.foonet.org 004 tester irc.foonet.org ergo-v2.8.0 BERTZios CEIMRUabefhiklmnoqstuv Iabefhkloqv")
+ (0.00 ":irc.foonet.org 005 tester AWAYLEN=390 BOT=B CASEMAPPING=ascii CHANLIMIT=#:100 CHANMODES=Ibe,k,fl,CEMRUimnstu CHANNELLEN=64 CHANTYPES=# ELIST=U EXCEPTS EXTBAN=,m FORWARD=f INVEX KICKLEN=390 :are supported by this server")
+ (0.00 ":irc.foonet.org 005 tester MAXLIST=beI:60 MAXTARGETS=4 MODES MONITOR=100 NETWORK=foonet NICKLEN=32 PREFIX=(qaohv)~&@%+ STATUSMSG=~&@%+ TARGMAX=NAMES:1,LIST:1,KICK:,WHOIS:1,USERHOST:10,PRIVMSG:4,TAGMSG:4,NOTICE:4,MONITOR:100 TOPICLEN=390 UTF8MAPPING=rfc8265 UTF8ONLY WHOX :are supported by this server")
+ (0.01 ":irc.foonet.org 005 tester draft/CHATHISTORY=100 :are supported by this server")
+ (0.00 ":irc.foonet.org 251 tester :There are 0 users and 4 invisible on 1 server(s)")
+ (0.00 ":irc.foonet.org 252 tester 0 :IRC Operators online")
+ (0.00 ":irc.foonet.org 253 tester 0 :unregistered connections")
+ (0.00 ":irc.foonet.org 254 tester 1 :channels formed")
+ (0.00 ":irc.foonet.org 255 tester :I have 4 clients and 0 servers")
+ (0.00 ":irc.foonet.org 265 tester 4 4 :Current local users 4, max 4")
+ (0.01 ":irc.foonet.org 266 tester 4 4 :Current global users 4, max 4")
+ (0.00 ":irc.foonet.org 422 tester :MOTD File is missing"))
+
+((mode 10 "MODE tester +i")
+ (0.00 ":irc.foonet.org 221 tester +i")
+ (0.00 ":irc.foonet.org NOTICE tester :This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect.")
+ (0.02 ":irc.foonet.org 221 tester +i")
+ (0.00 ":dummy!~u@z5d6jyn8pwxge.irc PRIVMSG tester :hi"))
+
+((~privmsg-open 10 "PRIVMSG nitwit :hola")
+ (0.00 ":nitwit!~u@m5q6wla8cjktr.irc PRIVMSG tester :ciao"))
+
+((privmsg 10 "PRIVMSG dummy :howdy")
+ (0.00 ":dummy!~u@z5d6jyn8pwxge.irc PRIVMSG tester :bye")
+ (0.01 ":dummy!~u@z5d6jyn8pwxge.irc QUIT :connection closed"))
+
+((quit 10 "QUIT :\2ERC\2")
+ (0.03 ":tester!~u@z5d6jyn8pwxge.irc QUIT :Quit: \2ERC\2"))
--
2.38.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict
[not found] ` <87sfhgn45z.fsf@neverwas.me>
@ 2022-12-15 18:24 ` Mike Kazantsev
[not found] ` <20221215232455.4a9c6677@malediction>
1 sibling, 0 replies; 8+ messages in thread
From: Mike Kazantsev @ 2022-12-15 18:24 UTC (permalink / raw)
To: J.P.; +Cc: 59976, emacs-erc
On Thu, 15 Dec 2022 06:16:08 -0800
"J.P." <jp@neverwas.me> wrote:
> "J.P." <jp@neverwas.me> writes:
>
> >>> 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/issues.
> >
> > 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.
>
> Actually, an egregious oversight has come to light that makes this a
> more general (and more pressing) matter relevant to Emacs 29. Currently,
> renaming queries fails if *any* buffer, even a non-ERC buffer, exists
> whose name matches that of the target in question. And, AFAICT, no
> amount of :id or options twiddling can serve as a workaround. (If this
> is what you've been getting at this whole time, then apologies: I'm
> rather thick headed, if you haven't noticed.)
>
> Anyhow, I have attempted to address this in the attached patch. If you
> or anyone out there is willing, please install it locally atop the
> current lisp/erc subtree on the emacs-29 branch and see if you can break
> it. Thanks in advance!
I think you indeed found the same thing I was thinking of and fixed it, thanks.
You mentioned connection process and how ID is being set in the previous
message, which indeed didn't seem to be same issues as I've tried to describe
(not clearly enough).
So thought to make a quick mockup of an IRC server and then send it,
with precise commands to run from ERC to reproduce exact problem,
as well actual output from that variable-change-tracking (which shows
how/when ID gets erased after successful connection, and only after
triggering conflicting query-buffer creation).
Haven't gotten around to do it yet though, but pretty sure you found
and fixed same exact issue in 0001-Fix-some-naming-issues-*.patch here,
so there should no longer be any need to do it.
I've just tested removing my custom ID-setting advices/code, using :id
instead, as well as reproducing that exact query-buffer-name conflict,
and it all works without any issues that I can see.
So will use that instead of local code hacks and won't need to worry
about making server-buffer names unique/distinct.
One small thing I've noticed about :id is that it has separate "Network
Identifier" section in "Connecting" info page, which doesn't seem to be linked
directly in its description - not sure if intentional, or maybe an oversight:
When present, ID should be an opaque object for identifying the
connection unequivocally. (In most cases, this would be a string or a
symbol composed of letters from the Latin alphabet.) This option is
generally unneeded, however. See info node ‘(erc) Connecting’ for use
cases. Not available interactively.
Come to think of it, I'd probably also add the most obvious effect of :id
for naming the server buffer, i.e. something like this:
When present, ID should be an opaque object for identifying the connection
unequivocally, and will correspond to name of the created erc server buffer
after connection. ...
Can probably be phrased better, but since main effect of "(erc ...)" command is
creating that server buffer, what it will be named seem to be an important detail
(if only for finding it afterwards).
Pretty sure patches you've attached address main issue I wanted to raise
(as well as couple other issues).
Thanks again for doing all the work here.
And apologies for not being able to articulate main problem more clearly.
Given how simple IRC is, should've definitely attached some easy-to-run
protocol example to reproduce it in the first place (maybe as .eld test-case),
instead of meandering description.
--
Mike Kazantsev // fraggod.net
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict
[not found] ` <20221215232455.4a9c6677@malediction>
@ 2022-12-15 18:45 ` Mike Kazantsev
2022-12-16 15:17 ` J.P.
1 sibling, 0 replies; 8+ messages in thread
From: Mike Kazantsev @ 2022-12-15 18:45 UTC (permalink / raw)
To: J.P.; +Cc: 59976, emacs-erc
On Thu, 15 Dec 2022 23:24:55 +0500
Mike Kazantsev <mk.fraggod@gmail.com> wrote:
> Come to think of it, I'd probably also add the most obvious effect of :id
> for naming the server buffer, i.e. something like this:
>
> When present, ID should be an opaque object for identifying the connection
> unequivocally, and will correspond to name of the created erc server buffer
> after connection. ...
>
> Can probably be phrased better, but since main effect of "(erc ...)" command is
> creating that server buffer, what it will be named seem to be an important detail
> (if only for finding it afterwards).
In fact, I think allowing for naming server buffers how you want them
to be named with one easy and reasonably-obvious option should probably
be most prominent thing about it:
https://e.var.nz/scr-20221215233955.jpg
(example of using :id for all networks, in contrast to earlier
inconsistent not-entirely-local naming)
--
Mike Kazantsev // fraggod.net
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict
[not found] ` <20221215232455.4a9c6677@malediction>
2022-12-15 18:45 ` Mike Kazantsev
@ 2022-12-16 15:17 ` J.P.
1 sibling, 0 replies; 8+ messages in thread
From: J.P. @ 2022-12-16 15:17 UTC (permalink / raw)
To: Mike Kazantsev; +Cc: emacs-erc, 59976-done
Mike Kazantsev <mk.fraggod@gmail.com> writes:
> On Thu, 15 Dec 2022 06:16:08 -0800
> "J.P." <jp@neverwas.me> wrote:
>
>> Anyhow, I have attempted to address this in the attached patch. If you
>> or anyone out there is willing, please install it locally atop the
>> current lisp/erc subtree on the emacs-29 branch and see if you can break
>> it. Thanks in advance!
>
> I think you indeed found the same thing I was thinking of and fixed it, thanks.
>
> You mentioned connection process and how ID is being set in the previous
> message, which indeed didn't seem to be same issues as I've tried to describe
> (not clearly enough).
Well, I've long been in the terrible habit of just saying "connection"
when I really mean "logical IRC connection" and not the actual network
process. But sometimes I mean both or one then the other, which is
obviously insane but likely what happened here (so please just take
whatever I said with a grain of salt).
> So thought to make a quick mockup of an IRC server and then send it,
> with precise commands to run from ERC to reproduce exact problem,
> as well actual output from that variable-change-tracking (which shows
> how/when ID gets erased after successful connection, and only after
> triggering conflicting query-buffer creation).
>
> Haven't gotten around to do it yet though, but pretty sure you found
> and fixed same exact issue in 0001-Fix-some-naming-issues-*.patch here,
> so there should no longer be any need to do it.
>
> I've just tested removing my custom ID-setting advices/code, using :id
> instead, as well as reproducing that exact query-buffer-name conflict,
> and it all works without any issues that I can see.
> So will use that instead of local code hacks and won't need to worry
> about making server-buffer names unique/distinct.
Nice!
> One small thing I've noticed about :id is that it has separate "Network
> Identifier" section in "Connecting" info page, which doesn't seem to be linked
> directly in its description - not sure if intentional, or maybe an oversight:
>
> When present, ID should be an opaque object for identifying the
> connection unequivocally. (In most cases, this would be a string or a
> symbol composed of letters from the Latin alphabet.) This option is
> generally unneeded, however. See info node ‘(erc) Connecting’ for use
> cases. Not available interactively.
Good catch.
> Come to think of it, I'd probably also add the most obvious effect of :id
> for naming the server buffer, i.e. something like this:
>
> When present, ID should be an opaque object for identifying the connection
> unequivocally, and will correspond to name of the created erc server buffer
> after connection. ...
>
> Can probably be phrased better, but since main effect of "(erc ...)" command is
> creating that server buffer, what it will be named seem to be an important detail
> (if only for finding it afterwards).
>
> [...]
>
> In fact, I think allowing for naming server buffers how you want them
> to be named with one easy and reasonably-obvious option should probably
> be most prominent thing about it:
>
> https://e.var.nz/scr-20221215233955.jpg
>
> (example of using :id for all networks, in contrast to earlier
> inconsistent not-entirely-local naming)
Agreed. Thanks. I have attempted to change it to something along the
lines of what you've suggested. Hopefully it's passable.
> Pretty sure patches you've attached address main issue I wanted to raise
> (as well as couple other issues).
>
> Thanks again for doing all the work here.
My pleasure. I've added the changes to Emacs 29, so they should appear
on HEAD as well, shortly.
> And apologies for not being able to articulate main problem more clearly.
> Given how simple IRC is, should've definitely attached some easy-to-run
> protocol example to reproduce it in the first place (maybe as .eld test-case),
> instead of meandering description.
Nah. Your input has made all the difference. Bug reports, especially
good ones like this, are terribly undervalued as contributions, IMO.
Please consider contributing to ERC (in any form) again in the future.
Until next time!
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-12-16 15:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-11 19:00 bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict Mike Kazantsev
2022-12-12 14:35 ` J.P.
[not found] ` <87pmcowuzv.fsf@neverwas.me>
2022-12-12 15:35 ` Mike Kazantsev
[not found] ` <20221212203508.3cd44bb6@malediction>
2022-12-13 15:13 ` J.P.
2022-12-15 14:16 ` J.P.
[not found] ` <87sfhgn45z.fsf@neverwas.me>
2022-12-15 18:24 ` Mike Kazantsev
[not found] ` <20221215232455.4a9c6677@malediction>
2022-12-15 18:45 ` Mike Kazantsev
2022-12-16 15:17 ` J.P.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).