* bug#46271: 28.0.50; [PATCH] Properly quote group names for gnus-search
@ 2021-02-03 13:31 jflack--- via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-03 18:54 ` Basil L. Contovounesios
2021-02-03 23:01 ` Eric Abrahamsen
0 siblings, 2 replies; 8+ messages in thread
From: jflack--- via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-02-03 13:31 UTC (permalink / raw)
To: 46271
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: multipart/mixed; boundary="=-=-=", Size: 2383 bytes --]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
- --=-=-=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable
The new gnus-search-indexed-parse-output doesn't properly quote group
names before using them as regexes meaning a group name containing
meta-characters (other than . or \ because of the current replacement)
won't be properly matched in the search results later.
I have attached a diff that fixes this by first quoting the group name
before performing the replacement; which is now constructed with RX
to save \\ soup.
=2D-=20
Thanks,
Jai
- --=-=-=
Content-Type: text/x-diff
Content-Disposition: attachment; filename=gnus-search-regex.diff
Content-Transfer-Encoding: quoted-printable
diff --git a/lisp/gnus/gnus-search.el b/lisp/gnus/gnus-search.el
index 44f43b073c..54603d8792 100644
=2D-- a/lisp/gnus/gnus-search.el
+++ b/lisp/gnus/gnus-search.el
@@ -82,6 +82,7 @@
(require 'gnus-util)
(require 'eieio)
(eval-when-compile (require 'cl-lib))
+(eval-when-compile (require 'rx))
(autoload 'eieio-build-class-alist "eieio-opt")
(autoload 'nnmaildir-base-name-to-article-number "nnmaildir")
=20
@@ -1380,8 +1381,8 @@ gnus-search-indexed-parse-output
(lambda (x)
(replace-regexp-in-string
;; Accept any of [.\/] as path separators.
=2D "[.\\/]" "[.\\\\/]"
=2D (gnus-group-real-name x)))
+ (rx (or "\\." "\\\\" "/")) "[.\\\\/]"
+ (regexp-quote (gnus-group-real-name x))))
groups "\\|")))
artlist vectors article group)
(goto-char (point-min))
- --=-=-=--
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCgAdFiEEuAb6YdjwXI5dBirisAc6s2XQgH0FAmAapdQACgkQsAc6s2XQ
gH1aehAAjdDYFs7rxva8vZQCHO4l6S84Ghgo3kqNyP3CXoitc7wPDAIZhvNYTnz3
/LBH/mtfJZA2TYhQkH6i3iNeF7PpJ5OhxJ1p3/in83yYHrhxsy94O61iBvElA3z0
TM8J5i3vuCc7xogze7vfGjXHe+gD7sCr/7EEFRN3Pan4bZGatJRRM3u5oKYbWmyM
dcQ+y+IzTODJ/ayx3nCX0H9r5grn7Iqh6W413bWEILr2YqTOOm37HJZ3LeDeegIv
0MitBjReUH3ePRhZtnOtyY9adbCX5AX4R65U8vLpIVTmunQ/xogHxg0L962RDlbt
4bSKrYHACbipGsBNvQ1ul76johoFRkCS/POkCAdt284VfxxwqXMxPXTJiy6ZEXjW
/4PpTZR0gGpoGMXgHjS0qT35gwmoF4rQoxGl6H4GDJIgHlg55wCgteSIANvqTEWl
8HHJ6S7pR3tbHTj4UYK+CrkrovAoW/eENtlueeuEFacvZUDfieCKwsS6SY7AdaMG
U2yetfZbLq42ZyVhK96h2pFNhOVUfn/gnLcZy1ghMlJgqMZ0S0836ql3OQGyLaJV
xwXrVJCLqz91f58TisjThMcgm7wm46fnhZ246CA3L+nZnyDQCLYtAQChDCco8O5f
ifG/JGqZqdsWGtXZGtGjj+1orCA4R+v+vxNNuytB5Wa+s8BzCCI=
=QS0M
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 8+ messages in thread* bug#46271: 28.0.50; [PATCH] Properly quote group names for gnus-search
2021-02-03 13:31 bug#46271: 28.0.50; [PATCH] Properly quote group names for gnus-search jflack--- via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-02-03 18:54 ` Basil L. Contovounesios
2021-02-04 0:52 ` jflack--- via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-03 23:01 ` Eric Abrahamsen
1 sibling, 1 reply; 8+ messages in thread
From: Basil L. Contovounesios @ 2021-02-03 18:54 UTC (permalink / raw)
To: 46271; +Cc: Jai Flack
jflack--- via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs@gnu.org> writes:
> diff --git a/lisp/gnus/gnus-search.el b/lisp/gnus/gnus-search.el
> index 44f43b073c..54603d8792 100644
> =2D-- a/lisp/gnus/gnus-search.el
> +++ b/lisp/gnus/gnus-search.el
> @@ -82,6 +82,7 @@
> (require 'gnus-util)
> (require 'eieio)
> (eval-when-compile (require 'cl-lib))
> +(eval-when-compile (require 'rx))
No need for this; rx is preloaded.
> (autoload 'eieio-build-class-alist "eieio-opt")
> (autoload 'nnmaildir-base-name-to-article-number "nnmaildir")
> =20
> @@ -1380,8 +1381,8 @@ gnus-search-indexed-parse-output
> (lambda (x)
> (replace-regexp-in-string
> ;; Accept any of [.\/] as path separators.
> =2D "[.\\/]" "[.\\\\/]"
> =2D (gnus-group-real-name x)))
> + (rx (or "\\." "\\\\" "/")) "[.\\\\/]"
> + (regexp-quote (gnus-group-real-name x))))
> groups "\\|")))
> artlist vectors article group)
> (goto-char (point-min))
BTW, your mail content seems to be mangled somewhere:
https://debbugs.gnu.org/46271.
--
Basil
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#46271: 28.0.50; [PATCH] Properly quote group names for gnus-search
2021-02-03 18:54 ` Basil L. Contovounesios
@ 2021-02-04 0:52 ` jflack--- via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-06 10:38 ` Basil L. Contovounesios
0 siblings, 1 reply; 8+ messages in thread
From: jflack--- via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-02-04 0:52 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: 46271
"Basil L. Contovounesios" <contovob@tcd.ie> writes:
> No need for this; rx is preloaded.
Apologies if this is off topic but; is it loaded before the start of
byte-compilation or somewhere else? I can find it (require 'rx)'d before
the start of tests but nowhere else.
> BTW, your mail content seems to be mangled somewhere:
> https://debbugs.gnu.org/46271.
Thanks for pointing it out, seems to be from giving a signed mail to
`message-send-and-exit'.
--
Thanks,
Jai
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#46271: 28.0.50; [PATCH] Properly quote group names for gnus-search
2021-02-04 0:52 ` jflack--- via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-02-06 10:38 ` Basil L. Contovounesios
0 siblings, 0 replies; 8+ messages in thread
From: Basil L. Contovounesios @ 2021-02-06 10:38 UTC (permalink / raw)
To: Jai Flack; +Cc: 46271
Jai Flack <jflack@disroot.org> writes:
> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> No need for this; rx is preloaded.
> Apologies if this is off topic but; is it loaded before the start of
> byte-compilation or somewhere else? I can find it (require 'rx)'d before
> the start of tests but nowhere else.
Sorry, I meant autoloaded, not preloaded.
There is no need to (require 'rx) unless using internal rx.el
definitions (needed in rx-tests.el, for example). Most other cases of
(require 'rx), regardless of whether in normal libraries or test files,
can be removed.
--
Basil
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#46271: 28.0.50; [PATCH] Properly quote group names for gnus-search
2021-02-03 13:31 bug#46271: 28.0.50; [PATCH] Properly quote group names for gnus-search jflack--- via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-03 18:54 ` Basil L. Contovounesios
@ 2021-02-03 23:01 ` Eric Abrahamsen
2021-02-07 22:26 ` Eric Abrahamsen
1 sibling, 1 reply; 8+ messages in thread
From: Eric Abrahamsen @ 2021-02-03 23:01 UTC (permalink / raw)
To: 46271; +Cc: jflack
jflack--- via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs@gnu.org> writes:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
>
> - --=-=-=
> Content-Type: text/plain
> Content-Transfer-Encoding: quoted-printable
>
>
> The new gnus-search-indexed-parse-output doesn't properly quote group
> names before using them as regexes meaning a group name containing
> meta-characters (other than . or \ because of the current replacement)
> won't be properly matched in the search results later.
>
> I have attached a diff that fixes this by first quoting the group name
> before performing the replacement; which is now constructed with RX
> to save \\ soup.
>
> =2D-=20
> Thanks,
> Jai
>
> - --=-=-=
> Content-Type: text/x-diff
> Content-Disposition: attachment; filename=gnus-search-regex.diff
> Content-Transfer-Encoding: quoted-printable
>
> diff --git a/lisp/gnus/gnus-search.el b/lisp/gnus/gnus-search.el
> index 44f43b073c..54603d8792 100644
> =2D-- a/lisp/gnus/gnus-search.el
> +++ b/lisp/gnus/gnus-search.el
> @@ -82,6 +82,7 @@
> (require 'gnus-util)
> (require 'eieio)
> (eval-when-compile (require 'cl-lib))
> +(eval-when-compile (require 'rx))
> (autoload 'eieio-build-class-alist "eieio-opt")
> (autoload 'nnmaildir-base-name-to-article-number "nnmaildir")
> =20
> @@ -1380,8 +1381,8 @@ gnus-search-indexed-parse-output
> (lambda (x)
> (replace-regexp-in-string
> ;; Accept any of [.\/] as path separators.
> =2D "[.\\/]" "[.\\\\/]"
> =2D (gnus-group-real-name x)))
> + (rx (or "\\." "\\\\" "/")) "[.\\\\/]"
> + (regexp-quote (gnus-group-real-name x))))
> groups "\\|")))
> artlist vectors article group)
> (goto-char (point-min))
Thanks very much for the patch! Let me do some local testing over the
next couple of days, but at first glance this looks like it will do the
trick.
Thanks,
Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#46271: 28.0.50; [PATCH] Properly quote group names for gnus-search
2021-02-03 23:01 ` Eric Abrahamsen
@ 2021-02-07 22:26 ` Eric Abrahamsen
2021-02-08 19:11 ` Basil L. Contovounesios
0 siblings, 1 reply; 8+ messages in thread
From: Eric Abrahamsen @ 2021-02-07 22:26 UTC (permalink / raw)
To: 46271; +Cc: jflack
Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> jflack--- via "Bug reports for GNU Emacs, the Swiss army knife of text
> editors" <bug-gnu-emacs@gnu.org> writes:
>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA512
>>
>> - --=-=-=
>> Content-Type: text/plain
>> Content-Transfer-Encoding: quoted-printable
>>
>>
>> The new gnus-search-indexed-parse-output doesn't properly quote group
>> names before using them as regexes meaning a group name containing
>> meta-characters (other than . or \ because of the current replacement)
>> won't be properly matched in the search results later.
>>
>> I have attached a diff that fixes this by first quoting the group name
>> before performing the replacement; which is now constructed with RX
>> to save \\ soup.
>>
>> =2D-=20
>> Thanks,
>> Jai
>>
>> - --=-=-=
>> Content-Type: text/x-diff
>> Content-Disposition: attachment; filename=gnus-search-regex.diff
>> Content-Transfer-Encoding: quoted-printable
>>
>> diff --git a/lisp/gnus/gnus-search.el b/lisp/gnus/gnus-search.el
>> index 44f43b073c..54603d8792 100644
>> =2D-- a/lisp/gnus/gnus-search.el
>> +++ b/lisp/gnus/gnus-search.el
>> @@ -82,6 +82,7 @@
>> (require 'gnus-util)
>> (require 'eieio)
>> (eval-when-compile (require 'cl-lib))
>> +(eval-when-compile (require 'rx))
>> (autoload 'eieio-build-class-alist "eieio-opt")
>> (autoload 'nnmaildir-base-name-to-article-number "nnmaildir")
>> =20
>> @@ -1380,8 +1381,8 @@ gnus-search-indexed-parse-output
>> (lambda (x)
>> (replace-regexp-in-string
>> ;; Accept any of [.\/] as path separators.
>> =2D "[.\\/]" "[.\\\\/]"
>> =2D (gnus-group-real-name x)))
>> + (rx (or "\\." "\\\\" "/")) "[.\\\\/]"
>> + (regexp-quote (gnus-group-real-name x))))
>> groups "\\|")))
>> artlist vectors article group)
>> (goto-char (point-min))
>
> Thanks very much for the patch! Let me do some local testing over the
> next couple of days, but at first glance this looks like it will do the
> trick.
Finally getting to this...
This doesn't quite do what it intends to do, as the
`replace-regexp-in-string' interferes with the `regexp-quote'. To wit:
(let ((group-name "mail+"))
(replace-regexp-in-string
"[.\\/]" "[.\\\\/]"
(regexp-quote (gnus-group-real-name group-name))))
-> "mail[.\\/]+"
But what we wanted was:
"mail\\+"
So I think it has to get messier:
(let ((groups '("mail+" "gnus.gener+al" "archive.2007.10")))
(mapconcat
#'identity
(mapcar
(lambda (group-name)
(mapconcat #'regexp-quote
(split-string
(gnus-group-real-name group-name)
"[.\\/]")
"[.\\\\/]"))
groups)
"\\|"))
-> "mail\\+\\|gnus[.\\\\/]gener\\+al\\|archive[.\\\\/]2007[.\\\\/]10"
This looks right to me. WDYT?
Also, I'd prefer not to use rx in this case, simply because this:
"[.\\/]"
turns into this:
(rx (or "\\." "\\\\" "/"))
Which seems like a loss of readability, though in general I think rx is
a very good idea.
Thanks again for the report!
Eric
^ permalink raw reply [flat|nested] 8+ messages in thread* bug#46271: 28.0.50; [PATCH] Properly quote group names for gnus-search
2021-02-07 22:26 ` Eric Abrahamsen
@ 2021-02-08 19:11 ` Basil L. Contovounesios
2021-02-08 20:13 ` Eric Abrahamsen
0 siblings, 1 reply; 8+ messages in thread
From: Basil L. Contovounesios @ 2021-02-08 19:11 UTC (permalink / raw)
To: Eric Abrahamsen; +Cc: jflack, 46271
Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> So I think it has to get messier:
>
> (let ((groups '("mail+" "gnus.gener+al" "archive.2007.10")))
> (mapconcat
> #'identity
> (mapcar
> (lambda (group-name)
> (mapconcat #'regexp-quote
> (split-string
> (gnus-group-real-name group-name)
> "[.\\/]")
> "[.\\\\/]"))
> groups)
> "\\|"))
AKA:
(let ((groups '("mail+" "gnus.gener+al" "archive.2007.10")))
(mapconcat
(lambda (group-name)
(mapconcat #'regexp-quote
(split-string
(gnus-group-real-name group-name)
"[.\\/]")
"[.\\\\/]"))
groups
"\\|"))
> Also, I'd prefer not to use rx in this case, simply because this:
>
> "[.\\/]"
>
> turns into this:
>
> (rx (or "\\." "\\\\" "/"))
Or (rx (in "./\\")), or (rx (in ?. ?/ ?\\)), or...
--
Basil
^ permalink raw reply [flat|nested] 8+ messages in thread* bug#46271: 28.0.50; [PATCH] Properly quote group names for gnus-search
2021-02-08 19:11 ` Basil L. Contovounesios
@ 2021-02-08 20:13 ` Eric Abrahamsen
0 siblings, 0 replies; 8+ messages in thread
From: Eric Abrahamsen @ 2021-02-08 20:13 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: jflack, 46271
On 02/08/21 19:11 PM, Basil L. Contovounesios wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> So I think it has to get messier:
>>
>> (let ((groups '("mail+" "gnus.gener+al" "archive.2007.10")))
>> (mapconcat
>> #'identity
>> (mapcar
>> (lambda (group-name)
>> (mapconcat #'regexp-quote
>> (split-string
>> (gnus-group-real-name group-name)
>> "[.\\/]")
>> "[.\\\\/]"))
>> groups)
>> "\\|"))
>
> AKA:
>
> (let ((groups '("mail+" "gnus.gener+al" "archive.2007.10")))
> (mapconcat
> (lambda (group-name)
> (mapconcat #'regexp-quote
> (split-string
> (gnus-group-real-name group-name)
> "[.\\/]")
> "[.\\\\/]"))
> groups
> "\\|"))
Ha! That was a brain malfunction.
>> Also, I'd prefer not to use rx in this case, simply because this:
>>
>> "[.\\/]"
>>
>> turns into this:
>>
>> (rx (or "\\." "\\\\" "/"))
>
> Or (rx (in "./\\")), or (rx (in ?. ?/ ?\\)), or...
That first one's not bad, but I still don't feel like it's a real
improvement over the plain regexp.
Anyway, thanks for the tuneup. I think we can just go with this.
Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-02-08 20:13 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-03 13:31 bug#46271: 28.0.50; [PATCH] Properly quote group names for gnus-search jflack--- via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-03 18:54 ` Basil L. Contovounesios
2021-02-04 0:52 ` jflack--- via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-06 10:38 ` Basil L. Contovounesios
2021-02-03 23:01 ` Eric Abrahamsen
2021-02-07 22:26 ` Eric Abrahamsen
2021-02-08 19:11 ` Basil L. Contovounesios
2021-02-08 20:13 ` Eric Abrahamsen
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).