* bug#71935: split-string-and-unquote mishandles dired-listing-switches with '
@ 2024-07-04 6:51 Juri Linkov
2024-07-04 8:10 ` Eli Zaretskii
0 siblings, 1 reply; 12+ messages in thread
From: Juri Linkov @ 2024-07-04 6:51 UTC (permalink / raw)
To: 71935
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").
^ permalink raw reply [flat|nested] 12+ messages in thread* bug#71935: split-string-and-unquote mishandles dired-listing-switches with '
2024-07-04 6:51 bug#71935: split-string-and-unquote mishandles dired-listing-switches with ' Juri Linkov
@ 2024-07-04 8:10 ` Eli Zaretskii
2024-07-04 16:10 ` Juri Linkov
0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2024-07-04 8:10 UTC (permalink / raw)
To: Juri Linkov; +Cc: 71935
> From: Juri Linkov <juri@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?
^ permalink raw reply [flat|nested] 12+ messages in thread* bug#71935: split-string-and-unquote mishandles dired-listing-switches with '
2024-07-04 8:10 ` Eli Zaretskii
@ 2024-07-04 16:10 ` Juri Linkov
2024-07-04 17:56 ` Eli Zaretskii
0 siblings, 1 reply; 12+ messages in thread
From: Juri Linkov @ 2024-07-04 16:10 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 71935
>> 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'
^ permalink raw reply [flat|nested] 12+ messages in thread* bug#71935: split-string-and-unquote mishandles dired-listing-switches with '
2024-07-04 16:10 ` Juri Linkov
@ 2024-07-04 17:56 ` Eli Zaretskii
2024-07-04 18:12 ` Juri Linkov
0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2024-07-04 17:56 UTC (permalink / raw)
To: Juri Linkov; +Cc: 71935
> From: Juri Linkov <juri@linkov.net>
> Cc: 71935@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.
^ permalink raw reply [flat|nested] 12+ messages in thread* bug#71935: split-string-and-unquote mishandles dired-listing-switches with '
2024-07-04 17:56 ` Eli Zaretskii
@ 2024-07-04 18:12 ` Juri Linkov
2024-07-04 18:34 ` Eli Zaretskii
0 siblings, 1 reply; 12+ messages in thread
From: Juri Linkov @ 2024-07-04 18:12 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 71935
>> >> 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".
^ permalink raw reply [flat|nested] 12+ messages in thread* bug#71935: split-string-and-unquote mishandles dired-listing-switches with '
2024-07-04 18:12 ` Juri Linkov
@ 2024-07-04 18:34 ` Eli Zaretskii
2024-07-04 18:54 ` Juri Linkov
0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2024-07-04 18:34 UTC (permalink / raw)
To: Juri Linkov; +Cc: 71935
> From: Juri Linkov <juri@linkov.net>
> Cc: 71935@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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#71935: split-string-and-unquote mishandles dired-listing-switches with '
2024-07-04 18:34 ` Eli Zaretskii
@ 2024-07-04 18:54 ` Juri Linkov
2024-07-04 19:03 ` Eli Zaretskii
0 siblings, 1 reply; 12+ messages in thread
From: Juri Linkov @ 2024-07-04 18:54 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 71935
>>> 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#71935: split-string-and-unquote mishandles dired-listing-switches with '
2024-07-04 18:54 ` Juri Linkov
@ 2024-07-04 19:03 ` Eli Zaretskii
2024-07-04 19:11 ` Eli Zaretskii
2024-07-07 6:57 ` Juri Linkov
0 siblings, 2 replies; 12+ messages in thread
From: Eli Zaretskii @ 2024-07-04 19:03 UTC (permalink / raw)
To: Juri Linkov; +Cc: 71935
> From: Juri Linkov <juri@linkov.net>
> Cc: 71935@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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#71935: split-string-and-unquote mishandles dired-listing-switches with '
2024-07-04 19:03 ` Eli Zaretskii
@ 2024-07-04 19:11 ` Eli Zaretskii
2024-07-05 6:48 ` Juri Linkov
2024-07-07 6:57 ` Juri Linkov
1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2024-07-04 19:11 UTC (permalink / raw)
To: juri; +Cc: 71935
> Cc: 71935@debbugs.gnu.org
> Date: Thu, 04 Jul 2024 22:03:00 +0300
> From: Eli Zaretskii <eliz@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?
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#71935: split-string-and-unquote mishandles dired-listing-switches with '
2024-07-04 19:11 ` Eli Zaretskii
@ 2024-07-05 6:48 ` Juri Linkov
2024-07-05 7:43 ` Eli Zaretskii
0 siblings, 1 reply; 12+ messages in thread
From: Juri Linkov @ 2024-07-05 6:48 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 71935
>> > @@ -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
^ permalink raw reply related [flat|nested] 12+ messages in thread* bug#71935: split-string-and-unquote mishandles dired-listing-switches with '
2024-07-05 6:48 ` Juri Linkov
@ 2024-07-05 7:43 ` Eli Zaretskii
0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2024-07-05 7:43 UTC (permalink / raw)
To: Juri Linkov; +Cc: 71935
> From: Juri Linkov <juri@linkov.net>
> Cc: 71935@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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#71935: split-string-and-unquote mishandles dired-listing-switches with '
2024-07-04 19:03 ` Eli Zaretskii
2024-07-04 19:11 ` Eli Zaretskii
@ 2024-07-07 6:57 ` Juri Linkov
1 sibling, 0 replies; 12+ messages in thread
From: Juri Linkov @ 2024-07-07 6:57 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 71935
close 71935 30.0.60
thanks
> Please install on the emacs-30 branch.
Installed.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-07-07 6:57 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-04 6:51 bug#71935: split-string-and-unquote mishandles dired-listing-switches with ' Juri Linkov
2024-07-04 8:10 ` Eli Zaretskii
2024-07-04 16:10 ` Juri Linkov
2024-07-04 17:56 ` Eli Zaretskii
2024-07-04 18:12 ` Juri Linkov
2024-07-04 18:34 ` Eli Zaretskii
2024-07-04 18:54 ` Juri Linkov
2024-07-04 19:03 ` Eli Zaretskii
2024-07-04 19:11 ` Eli Zaretskii
2024-07-05 6:48 ` Juri Linkov
2024-07-05 7:43 ` Eli Zaretskii
2024-07-07 6:57 ` Juri Linkov
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).