GNU bug report logs - #71935
split-string-and-unquote mishandles dired-listing-switches with '

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 71935 in the body.
You can then email your comments to 71935 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: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: split-string-and-unquote mishandles dired-listing-switches with '
Date: Thu, 04 Jul 2024 09:51:10 +0300

0. emacs -Q
1. (setopt dired-listing-switches "-al --block-size='1")
2. C-x d /tmp/*

  /tmp:
  wildcard *
  /bin/bash: -c: line 0: unexpected EOF while looking for matching `''
  /bin/bash: -c: line 1: syntax error: unexpected end of file

3. (setopt dired-listing-switches "-al --block-size=\\'1")
4. C-x d /tmp/

Debugger entered--Lisp error: (error "Listing directory failed but ‘access-file’ worked")
  error("Listing directory failed but `access-file' worked")
  insert-directory("/tmp/" "--dired -N -al --block-size=\\'1" nil t)
  dired-insert-directory("/tmp/" "-al --block-size=\\'1" nil nil t)
  dired-readin-insert()
  dired-readin()
  dired-internal-noselect("/tmp/" nil)
  dired-noselect("/tmp/" nil)
  dired("/tmp/" nil)
  funcall-interactively(dired "/tmp/" nil)
  command-execute(dired)

because 'split-string-and-unquote' in 'insert-directory'
doesn't turn "--block-size=\\'1" into "--block-size='1".

5. (setopt dired-listing-switches "-al --block-size=\"'1\"")
6. C-x d /tmp/

Same error for another reason, because 'split-string-and-unquote'
splits "--block-size=\"'1\"" to '("--block-size=" "'1").

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 71935 <at> debbugs.gnu.org
Subject: Re: bug#71935: split-string-and-unquote mishandles
 dired-listing-switches with '
Date: Thu, 04 Jul 2024 11:10:04 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Date: Thu, 04 Jul 2024 09:51:10 +0300
> 
> 0. emacs -Q
> 1. (setopt dired-listing-switches "-al --block-size='1")
> 2. C-x d /tmp/*
> 
>   /tmp:
>   wildcard *
>   /bin/bash: -c: line 0: unexpected EOF while looking for matching `''
>   /bin/bash: -c: line 1: syntax error: unexpected end of file
> 
> 3. (setopt dired-listing-switches "-al --block-size=\\'1")
> 4. C-x d /tmp/
> 
> Debugger entered--Lisp error: (error "Listing directory failed but ‘access-file’ worked")
>   error("Listing directory failed but `access-file' worked")
>   insert-directory("/tmp/" "--dired -N -al --block-size=\\'1" nil t)
>   dired-insert-directory("/tmp/" "-al --block-size=\\'1" nil nil t)
>   dired-readin-insert()
>   dired-readin()
>   dired-internal-noselect("/tmp/" nil)
>   dired-noselect("/tmp/" nil)
>   dired("/tmp/" nil)
>   funcall-interactively(dired "/tmp/" nil)
>   command-execute(dired)
> 
> because 'split-string-and-unquote' in 'insert-directory'
> doesn't turn "--block-size=\\'1" into "--block-size='1".

I think the part of insert-directory that deals with wildcard lacks
the call to shell-quote-argument here:

		      ;; NB since switches is passed to the shell, be
		      ;; careful of malicious values, eg "-l;reboot".
		      ;; See eg dired-safe-switches-p.
		      (call-process
		       shell-file-name nil t nil
		       shell-command-switch
		       (concat (if (memq system-type '(ms-dos windows-nt))
				   ""
				 "\\") ; Disregard Unix shell aliases!
			       insert-directory-program
			       " -d "
			       (if (stringp switches)
				   switches
				 (mapconcat #'identity switches " "))

Can you try running each switch through shell-quote-argument?

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

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71935 <at> debbugs.gnu.org
Subject: Re: bug#71935: split-string-and-unquote mishandles
 dired-listing-switches with '
Date: Thu, 04 Jul 2024 19:10:17 +0300

>> 1. (setopt dired-listing-switches "-al --block-size='1")
>> 2. C-x d /tmp/*
>>
>>   /tmp:
>>   wildcard *
>>   /bin/bash: -c: line 0: unexpected EOF while looking for matching `''
>>   /bin/bash: -c: line 1: syntax error: unexpected end of file
>>
>> 3. (setopt dired-listing-switches "-al --block-size=\\'1")
>> 4. C-x d /tmp/
>>
>> Debugger entered--Lisp error: (error "Listing directory failed but ‘access-file’ worked")
>>   error("Listing directory failed but `access-file' worked")
>>   insert-directory("/tmp/" "--dired -N -al --block-size=\\'1" nil t)
>
> I think the part of insert-directory that deals with wildcard lacks
> the call to shell-quote-argument here:
>
> 		      ;; NB since switches is passed to the shell, be
> 		      ;; careful of malicious values, eg "-l;reboot".
> 		      ;; See eg dired-safe-switches-p.
> 		      (call-process
> 		       shell-file-name nil t nil
> 		       shell-command-switch
> 		       (concat (if (memq system-type '(ms-dos windows-nt))
> 				   ""
> 				 "\\") ; Disregard Unix shell aliases!
> 			       insert-directory-program
> 			       " -d "
> 			       (if (stringp switches)
> 				   switches
> 				 (mapconcat #'identity switches " "))
>
> Can you try running each switch through shell-quote-argument?

This part of insert-directory is used only in case of 1-2 above,
i.e. for wildcard '/tmp/*'.  In this case the value of 'switches'
is "--dired -N -al --block-size='1", and 'shell-quote-argument'
returns "--dired\\ -N\\ -al\\ --block-size\\=\\'1" that fails.

For the non-wildcard case of 3-4 above, this doesn't help either.
Using (mapcar 'shell-quote-argument (split-string-and-unquote switches))
on ("--dired" "-N" "-al" "--block-size=\\'1") returns
("--dired" "-N" "-al" "--block-size\\=\\\\\\'1") that fails with

  /bin/ls: unrecognized option '--block-size\=\\\'1'

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 71935 <at> debbugs.gnu.org
Subject: Re: bug#71935: split-string-and-unquote mishandles
 dired-listing-switches with '
Date: Thu, 04 Jul 2024 20:56:11 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Cc: 71935 <at> debbugs.gnu.org
> Date: Thu, 04 Jul 2024 19:10:17 +0300
> 
> >> 1. (setopt dired-listing-switches "-al --block-size='1")
> >> 2. C-x d /tmp/*
> >>
> >>   /tmp:
> >>   wildcard *
> >>   /bin/bash: -c: line 0: unexpected EOF while looking for matching `''
> >>   /bin/bash: -c: line 1: syntax error: unexpected end of file
> >>
> >> 3. (setopt dired-listing-switches "-al --block-size=\\'1")
> >> 4. C-x d /tmp/
> >>
> >> Debugger entered--Lisp error: (error "Listing directory failed but ‘access-file’ worked")
> >>   error("Listing directory failed but `access-file' worked")
> >>   insert-directory("/tmp/" "--dired -N -al --block-size=\\'1" nil t)
> >
> > I think the part of insert-directory that deals with wildcard lacks
> > the call to shell-quote-argument here:
> >
> > 		      ;; NB since switches is passed to the shell, be
> > 		      ;; careful of malicious values, eg "-l;reboot".
> > 		      ;; See eg dired-safe-switches-p.
> > 		      (call-process
> > 		       shell-file-name nil t nil
> > 		       shell-command-switch
> > 		       (concat (if (memq system-type '(ms-dos windows-nt))
> > 				   ""
> > 				 "\\") ; Disregard Unix shell aliases!
> > 			       insert-directory-program
> > 			       " -d "
> > 			       (if (stringp switches)
> > 				   switches
> > 				 (mapconcat #'identity switches " "))
> >
> > Can you try running each switch through shell-quote-argument?
> 
> This part of insert-directory is used only in case of 1-2 above,
> i.e. for wildcard '/tmp/*'.  In this case the value of 'switches'
> is "--dired -N -al --block-size='1", and 'shell-quote-argument'
> returns "--dired\\ -N\\ -al\\ --block-size\\=\\'1" that fails.

I meant to call shell-quote-argument on each option, before they are
concatenated.

> For the non-wildcard case of 3-4 above, this doesn't help either.
> Using (mapcar 'shell-quote-argument (split-string-and-unquote switches))
> on ("--dired" "-N" "-al" "--block-size=\\'1") returns
> ("--dired" "-N" "-al" "--block-size\\=\\\\\\'1") that fails with
> 
>   /bin/ls: unrecognized option '--block-size\=\\\'1'

Why did you use "--block-size=\\'1"?  My idea is that the quoting
should not come from the user.

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

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71935 <at> debbugs.gnu.org
Subject: Re: bug#71935: split-string-and-unquote mishandles
 dired-listing-switches with '
Date: Thu, 04 Jul 2024 21:12:01 +0300

>> >> 1. (setopt dired-listing-switches "-al --block-size='1")
>> >> 2. C-x d /tmp/*
>> 
>> This part of insert-directory is used only in case of 1-2 above,
>> i.e. for wildcard '/tmp/*'.  In this case the value of 'switches'
>> is "--dired -N -al --block-size='1", and 'shell-quote-argument'
>> returns "--dired\\ -N\\ -al\\ --block-size\\=\\'1" that fails.
>
> I meant to call shell-quote-argument on each option, before they are
> concatenated.

But switches are never unconcatenated in a list,
they come from the string:

  (setopt dired-listing-switches "-al --block-size='1")

>> For the non-wildcard case of 3-4 above, this doesn't help either.
>> Using (mapcar 'shell-quote-argument (split-string-and-unquote switches))
>> on ("--dired" "-N" "-al" "--block-size=\\'1") returns
>> ("--dired" "-N" "-al" "--block-size\\=\\\\\\'1") that fails with
>> 
>>   /bin/ls: unrecognized option '--block-size\=\\\'1'
>
> Why did you use "--block-size=\\'1"?  My idea is that the quoting
> should not come from the user.

If the wildcard case above could be fixed, then there is no need
to use "--block-size=\\'1".

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 71935 <at> debbugs.gnu.org
Subject: Re: bug#71935: split-string-and-unquote mishandles
 dired-listing-switches with '
Date: Thu, 04 Jul 2024 21:34:22 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Cc: 71935 <at> debbugs.gnu.org
> Date: Thu, 04 Jul 2024 21:12:01 +0300
> 
> >> >> 1. (setopt dired-listing-switches "-al --block-size='1")
> >> >> 2. C-x d /tmp/*
> >> 
> >> This part of insert-directory is used only in case of 1-2 above,
> >> i.e. for wildcard '/tmp/*'.  In this case the value of 'switches'
> >> is "--dired -N -al --block-size='1", and 'shell-quote-argument'
> >> returns "--dired\\ -N\\ -al\\ --block-size\\=\\'1" that fails.
> >
> > I meant to call shell-quote-argument on each option, before they are
> > concatenated.
> 
> But switches are never unconcatenated in a list,
> they come from the string:
> 
>   (setopt dired-listing-switches "-al --block-size='1")

If they are a single string, split it with split-string-and-unquote,
then concatenate after running each one through shell-quote-argument.
If they are a list of strings, quote each one before concatenating
with mapconcat.

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

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71935 <at> debbugs.gnu.org
Subject: Re: bug#71935: split-string-and-unquote mishandles
 dired-listing-switches with '
Date: Thu, 04 Jul 2024 21:54:02 +0300

>>> 1. (setopt dired-listing-switches "-al --block-size='1")
>>> 2. C-x d /tmp/*
>
> If they are a single string, split it with split-string-and-unquote,
> then concatenate after running each one through shell-quote-argument.
> If they are a list of strings, quote each one before concatenating
> with mapconcat.

Ok, this seems to work:

diff --git a/lisp/files.el b/lisp/files.el
index 042b8e2d515..6ed07f02890 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -8195,9 +8193,11 @@ insert-directory
 				 "\\") ; Disregard Unix shell aliases!
 			       insert-directory-program
 			       " -d "
-			       (if (stringp switches)
-				   switches
-				 (mapconcat #'identity switches " "))
+			       (mapconcat #'shell-quote-argument
+                                          (if (stringp switches)
+				              (split-string-and-unquote switches)
+                                            switches)
+                                          " ")
 			       " -- "
 			       ;; Quote some characters that have
 			       ;; special meanings in shells; but

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 71935 <at> debbugs.gnu.org
Subject: Re: bug#71935: split-string-and-unquote mishandles
 dired-listing-switches with '
Date: Thu, 04 Jul 2024 22:03:00 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Cc: 71935 <at> debbugs.gnu.org
> Date: Thu, 04 Jul 2024 21:54:02 +0300
> 
> >>> 1. (setopt dired-listing-switches "-al --block-size='1")
> >>> 2. C-x d /tmp/*
> >
> > If they are a single string, split it with split-string-and-unquote,
> > then concatenate after running each one through shell-quote-argument.
> > If they are a list of strings, quote each one before concatenating
> > with mapconcat.
> 
> Ok, this seems to work:
> 
> diff --git a/lisp/files.el b/lisp/files.el
> index 042b8e2d515..6ed07f02890 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -8195,9 +8193,11 @@ insert-directory
>  				 "\\") ; Disregard Unix shell aliases!
>  			       insert-directory-program
>  			       " -d "
> -			       (if (stringp switches)
> -				   switches
> -				 (mapconcat #'identity switches " "))
> +			       (mapconcat #'shell-quote-argument
> +                                          (if (stringp switches)
> +				              (split-string-and-unquote switches)
> +                                            switches)
> +                                          " ")
>  			       " -- "
>  			       ;; Quote some characters that have
>  			       ;; special meanings in shells; but
> 

Thanks, that's what I had in mind.  Please install on the emacs-30
branch.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: juri <at> linkov.net
Cc: 71935 <at> debbugs.gnu.org
Subject: Re: bug#71935: split-string-and-unquote mishandles
 dired-listing-switches with '
Date: Thu, 04 Jul 2024 22:11:27 +0300

> Cc: 71935 <at> debbugs.gnu.org
> Date: Thu, 04 Jul 2024 22:03:00 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> > diff --git a/lisp/files.el b/lisp/files.el
> > index 042b8e2d515..6ed07f02890 100644
> > --- a/lisp/files.el
> > +++ b/lisp/files.el
> > @@ -8195,9 +8193,11 @@ insert-directory
> >  				 "\\") ; Disregard Unix shell aliases!
> >  			       insert-directory-program
> >  			       " -d "
> > -			       (if (stringp switches)
> > -				   switches
> > -				 (mapconcat #'identity switches " "))
> > +			       (mapconcat #'shell-quote-argument
> > +                                          (if (stringp switches)
> > +				              (split-string-and-unquote switches)
> > +                                            switches)
> > +                                          " ")
> >  			       " -- "
> >  			       ;; Quote some characters that have
> >  			       ;; special meanings in shells; but
> > 
> 
> Thanks, that's what I had in mind.  Please install on the emacs-30
> branch.

On second thought: could there be options that include shell
wildcards, which therefore should not be quoted?  If so, perhaps
instead of shell-quote-argument we should use
shell-quote-wildcard-pattern?

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

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71935 <at> debbugs.gnu.org
Subject: Re: bug#71935: split-string-and-unquote mishandles
 dired-listing-switches with '
Date: Fri, 05 Jul 2024 09:48:39 +0300

>> > @@ -8195,9 +8193,11 @@ insert-directory
>> > -			       (if (stringp switches)
>> > -				   switches
>> > -				 (mapconcat #'identity switches " "))
>> > +			       (mapconcat #'shell-quote-argument
>> > +                                          (if (stringp switches)
>> > +				              (split-string-and-unquote switches)
>> > +                                            switches)
>> > +                                          " ")
>>
>> Thanks, that's what I had in mind.  Please install on the emacs-30
>> branch.
>
> On second thought: could there be options that include shell
> wildcards, which therefore should not be quoted?  If so, perhaps
> instead of shell-quote-argument we should use
> shell-quote-wildcard-pattern?

Indeed, there are ls switches that use wildcards, e.g.
‘--hide=PATTERN’ and ‘--ignore=PATTERN’.  But it seems
they are ignored anyway while using wildcards with 'ls -d',
so I can't test them.  What I can confirm only is that with

1. (setopt dired-listing-switches "-al --block-size='1 --ignore=system*")
2. C-x d /tmp/s*

the switches are correctly quoted by 'shell-quote-wildcard-pattern':

"ls -d --dired -N -al --block-size=\\'1 --ignore=system* -- s*"

diff --git a/lisp/files.el b/lisp/files.el
index 042b8e2d515..e69dfb22a5f 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -8195,9 +8193,15 @@ insert-directory
                                 "\\") ; Disregard Unix shell aliases!
                               insert-directory-program
                               " -d "
-                              (if (stringp switches)
-                                  switches
-                                (mapconcat #'identity switches " "))
+                              ;; Quote switches that require quoting
+                              ;; such as "--block-size='1".  But don't
+                              ;; quote switches that use patterns
+                              ;; such as "--ignore=PATTERN" (bug#71935).
+                              (mapconcat #'shell-quote-wildcard-pattern
+                                         (if (stringp switches)
+                                             (split-string-and-unquote switches)
+                                           switches)
+                                         " ")
                               " -- "
                               ;; Quote some characters that have
                               ;; special meanings in shells; but

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 71935 <at> debbugs.gnu.org
Subject: Re: bug#71935: split-string-and-unquote mishandles
 dired-listing-switches with '
Date: Fri, 05 Jul 2024 10:43:52 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Cc: 71935 <at> debbugs.gnu.org
> Date: Fri, 05 Jul 2024 09:48:39 +0300
> 
> >> Thanks, that's what I had in mind.  Please install on the emacs-30
> >> branch.
> >
> > On second thought: could there be options that include shell
> > wildcards, which therefore should not be quoted?  If so, perhaps
> > instead of shell-quote-argument we should use
> > shell-quote-wildcard-pattern?
> 
> Indeed, there are ls switches that use wildcards, e.g.
> ‘--hide=PATTERN’ and ‘--ignore=PATTERN’.  But it seems
> they are ignored anyway while using wildcards with 'ls -d',
> so I can't test them.  What I can confirm only is that with
> 
> 1. (setopt dired-listing-switches "-al --block-size='1 --ignore=system*")
> 2. C-x d /tmp/s*
> 
> the switches are correctly quoted by 'shell-quote-wildcard-pattern':
> 
> "ls -d --dired -N -al --block-size=\\'1 --ignore=system* -- s*"

Thanks.  So I think this is a better solution for this tricky problem.

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

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71935 <at> debbugs.gnu.org
Subject: Re: bug#71935: split-string-and-unquote mishandles
 dired-listing-switches with '
Date: Sun, 07 Jul 2024 09:57:40 +0300

close 71935 30.0.60
thanks

> Please install on the emacs-30 branch.

Installed.

Previous Next

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