GNU bug report logs - #46271
28.0.50; [PATCH] Properly quote group names for gnus-search

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 46271 in the body.
You can then email your comments to 46271 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jai Flack <jflack <at> disroot.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; [PATCH] Properly quote group names for gnus-search
Date: Wed, 03 Feb 2021 23:31:58 +1000

Message #10 received at 46271 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: 46271 <at> debbugs.gnu.org
Cc: Jai Flack <jflack <at> disroot.org>
Subject: Re: bug#46271: 28.0.50; [PATCH] Properly quote group names for
 gnus-search
Date: Wed, 03 Feb 2021 18:54:20 +0000

jflack--- via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> 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

Message #13 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: jflack--- via "Bug reports for GNU Emacs, the Swiss army knife of text
 editors" <bug-gnu-emacs <at> gnu.org>
Cc: Jai Flack <jflack <at> disroot.org>, 46271 <at> debbugs.gnu.org
Subject: Re: bug#46271: 28.0.50; [PATCH] Properly quote group names for
 gnus-search
Date: Wed, 03 Feb 2021 15:01:31 -0800

jflack--- via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> 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

Message #19 received at 46271 <at> debbugs.gnu.org (full text, mbox):

From: Jai Flack <jflack <at> disroot.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 46271 <at> debbugs.gnu.org
Subject: Re: bug#46271: 28.0.50; [PATCH] Properly quote group names for
 gnus-search
Date: Thu, 04 Feb 2021 10:52:49 +1000

"Basil L. Contovounesios" <contovob <at> 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

Message #22 received at 46271 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Jai Flack <jflack <at> disroot.org>
Cc: 46271 <at> debbugs.gnu.org
Subject: Re: bug#46271: 28.0.50; [PATCH] Properly quote group names for
 gnus-search
Date: Sat, 06 Feb 2021 10:38:46 +0000

Jai Flack <jflack <at> disroot.org> writes:

> "Basil L. Contovounesios" <contovob <at> 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

Message #25 received at 46271 <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: 46271 <at> debbugs.gnu.org
Cc: jflack <at> disroot.org
Subject: Re: bug#46271: 28.0.50; [PATCH] Properly quote group names for
 gnus-search
Date: Sun, 07 Feb 2021 14:26:39 -0800

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> jflack--- via "Bug reports for GNU Emacs, the Swiss army knife of text
> editors" <bug-gnu-emacs <at> 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

Message #28 received at 46271 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: jflack <at> disroot.org, 46271 <at> debbugs.gnu.org
Subject: Re: bug#46271: 28.0.50; [PATCH] Properly quote group names for
 gnus-search
Date: Mon, 08 Feb 2021 19:11:57 +0000

Eric Abrahamsen <eric <at> 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

Message #31 received at 46271 <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: jflack <at> disroot.org, 46271 <at> debbugs.gnu.org
Subject: Re: bug#46271: 28.0.50; [PATCH] Properly quote group names for
 gnus-search
Date: Mon, 08 Feb 2021 12:13:38 -0800

On 02/08/21 19:11 PM, Basil L. Contovounesios wrote:
> Eric Abrahamsen <eric <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.