* [Bug] incorrect warning when I type rm /*
@ 2020-07-01 10:02 Jonas Bräutigam
2020-07-01 11:00 ` Daniel Shahaf
0 siblings, 1 reply; 5+ messages in thread
From: Jonas Bräutigam @ 2020-07-01 10:02 UTC (permalink / raw)
To: zsh-workers
Hello,
these days I wrote a command wrong. Instead of
"$ rm ./*" <- deleting in the current directory
I wrote:
"$ rm /*" <- deleting in the root path "/"
zsh warn me with:
"zsh: sure you want to delete all 4 files in /home/[user]/Downloads [yn]?"
After I typed "y", it tried to delete the files in "/".
So the correct warning should be:
"zsh: sure you want to delete all X files in / [yn]?"
I use debian 10.4 with no "rm"-specifiy plugin.
Whatch out If you try it! :D
Best Regards
Jonas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bug] incorrect warning when I type rm /*
2020-07-01 10:02 [Bug] incorrect warning when I type rm /* Jonas Bräutigam
@ 2020-07-01 11:00 ` Daniel Shahaf
2020-07-01 11:33 ` Mikael Magnusson
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Shahaf @ 2020-07-01 11:00 UTC (permalink / raw)
To: Jonas Bräutigam; +Cc: zsh-workers
Jonas Bräutigam wrote on Wed, 01 Jul 2020 12:02 +0200:
> Hello,
>
> these days I wrote a command wrong. Instead of
>
> "$ rm ./*" <- deleting in the current directory
>
> I wrote:
>
> "$ rm /*" <- deleting in the root path "/"
>
> zsh warn me with:
>
> "zsh: sure you want to delete all 4 files in /home/[user]/Downloads [yn]?"
>
> After I typed "y", it tried to delete the files in "/".
>
> So the correct warning should be:
>
> "zsh: sure you want to delete all X files in / [yn]?"
>
>
> I use debian 10.4 with no "rm"-specifiy plugin.
>
>
> Whatch out If you try it! :D
Thanks; it's a path arithmetic edge case:
diff --git a/Src/exec.c b/Src/exec.c
index 045b5d2b9..7120a2c34 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3401,7 +3401,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
int rmall;
s[l - 2] = 0;
- rmall = checkrmall(s);
+ rmall = checkrmall(l == 2 ? "/" : s);
s[l - 2] = t;
if (!rmall) {
The "Is the dir empty?" logic likewise operated on $PWD rather than /,
so if someone runs «rm /*» as root in an empty directory, the
RM_STAR_SILENT confirmation prompt would not appear at all.
I haven't figured out how to write a test for this (short of using
expect(1) or so).
Cheers,
Daniel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bug] incorrect warning when I type rm /*
2020-07-01 11:00 ` Daniel Shahaf
@ 2020-07-01 11:33 ` Mikael Magnusson
2020-07-01 12:50 ` Daniel Shahaf
2020-07-03 13:41 ` Daniel Shahaf
0 siblings, 2 replies; 5+ messages in thread
From: Mikael Magnusson @ 2020-07-01 11:33 UTC (permalink / raw)
To: Daniel Shahaf; +Cc: Jonas Bräutigam, zsh-workers
On 7/1/20, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Jonas Bräutigam wrote on Wed, 01 Jul 2020 12:02 +0200:
>> Hello,
>>
>> these days I wrote a command wrong. Instead of
>>
>> "$ rm ./*" <- deleting in the current directory
>>
>> I wrote:
>>
>> "$ rm /*" <- deleting in the root path "/"
>>
>> zsh warn me with:
>>
>> "zsh: sure you want to delete all 4 files in /home/[user]/Downloads
>> [yn]?"
>>
>> After I typed "y", it tried to delete the files in "/".
>>
>> So the correct warning should be:
>>
>> "zsh: sure you want to delete all X files in / [yn]?"
>>
>>
>> I use debian 10.4 with no "rm"-specifiy plugin.
>>
>>
>> Whatch out If you try it! :D
>
> Thanks; it's a path arithmetic edge case:
>
> diff --git a/Src/exec.c b/Src/exec.c
> index 045b5d2b9..7120a2c34 100644
> --- a/Src/exec.c
> +++ b/Src/exec.c
> @@ -3401,7 +3401,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
> int rmall;
>
> s[l - 2] = 0;
> - rmall = checkrmall(s);
> + rmall = checkrmall(l == 2 ? "/" : s);
> s[l - 2] = t;
>
> if (!rmall) {
>
> The "Is the dir empty?" logic likewise operated on $PWD rather than /,
> so if someone runs «rm /*» as root in an empty directory, the
> RM_STAR_SILENT confirmation prompt would not appear at all.
>
> I haven't figured out how to write a test for this (short of using
> expect(1) or so).
zpty_start and variants as in W02jobs, X02zlevi and others?
--
Mikael Magnusson
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bug] incorrect warning when I type rm /*
2020-07-01 11:33 ` Mikael Magnusson
@ 2020-07-01 12:50 ` Daniel Shahaf
2020-07-03 13:41 ` Daniel Shahaf
1 sibling, 0 replies; 5+ messages in thread
From: Daniel Shahaf @ 2020-07-01 12:50 UTC (permalink / raw)
To: Mikael Magnusson; +Cc: Jonas Bräutigam, zsh-workers
Mikael Magnusson wrote on Wed, 01 Jul 2020 13:33 +0200:
> On 7/1/20, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > Jonas Bräutigam wrote on Wed, 01 Jul 2020 12:02 +0200:
> >> Hello,
> >>
> >> these days I wrote a command wrong. Instead of
> >>
> >> "$ rm ./*" <- deleting in the current directory
> >>
> >> I wrote:
> >>
> >> "$ rm /*" <- deleting in the root path "/"
> >>
> >> zsh warn me with:
> >>
> >> "zsh: sure you want to delete all 4 files in /home/[user]/Downloads
> >> [yn]?"
> >>
> >> After I typed "y", it tried to delete the files in "/".
> >>
> >> So the correct warning should be:
> >>
> >> "zsh: sure you want to delete all X files in / [yn]?"
> >>
> >>
> >> I use debian 10.4 with no "rm"-specifiy plugin.
> >>
> >>
> >> Whatch out If you try it! :D
> >
> > Thanks; it's a path arithmetic edge case:
> >
> > diff --git a/Src/exec.c b/Src/exec.c
> > index 045b5d2b9..7120a2c34 100644
> > --- a/Src/exec.c
> > +++ b/Src/exec.c
> > @@ -3401,7 +3401,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
> > int rmall;
> >
> > s[l - 2] = 0;
> > - rmall = checkrmall(s);
> > + rmall = checkrmall(l == 2 ? "/" : s);
> > s[l - 2] = t;
> >
> > if (!rmall) {
> >
> > The "Is the dir empty?" logic likewise operated on $PWD rather than /,
> > so if someone runs «rm /*» as root in an empty directory, the
> > RM_STAR_SILENT confirmation prompt would not appear at all.
> >
> > I haven't figured out how to write a test for this (short of using
> > expect(1) or so).
>
> zpty_start and variants as in W02jobs, X02zlevi and others?
Thanks for the pointer. I'll look when I have time, but if anyone can
beat me to it, please do.
Cheers,
Daniel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bug] incorrect warning when I type rm /*
2020-07-01 11:33 ` Mikael Magnusson
2020-07-01 12:50 ` Daniel Shahaf
@ 2020-07-03 13:41 ` Daniel Shahaf
1 sibling, 0 replies; 5+ messages in thread
From: Daniel Shahaf @ 2020-07-03 13:41 UTC (permalink / raw)
To: Mikael Magnusson; +Cc: Jonas Bräutigam, zsh-workers
Mikael Magnusson wrote on Wed, 01 Jul 2020 13:33 +0200:
> On 7/1/20, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > I haven't figured out how to write a test for this (short of using
> > expect(1) or so).
>
> zpty_start and variants as in W02jobs, X02zlevi and others?
The following works, but I'm sure there's a more elegant way to do it.
8<--8<--
From d2aed981ed378c5634b5fcdfd578acee40d59841 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Fri, 3 Jul 2020 13:31:10 +0000
Subject: [PATCH 1/3] Add a unit test for the RM_STAR_SILENT option.
---
Test/E01options.ztst | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 70736f444..a1b564327 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -57,7 +57,6 @@
# PROMPT_CR
# PUSHD_SILENT
# REC_EXACT
-# RM_STAR_SILENT
# RM_STAR_WAIT
# SHARE_HISTORY
# SINGLE_LINE_ZLE
@@ -92,6 +91,9 @@
catpath=$(which cat)
lspath==ls
+ # If the module fails to load, individual test points will skip.
+ zmodload zsh/zpty 2>/dev/null || true
+
%test
# setopt should move on to the next operation in the face of an error, but
@@ -1423,3 +1425,26 @@ F:If this test fails at the first unsetopt, refer to P01privileged.ztst.
(( UID == EUID ))
fi
0:PRIVILEGED sanity check: default value is correct
+
+ if zmodload -e zsh/zpty 2>/dev/null; then
+ for target_dir target_pattern in \
+ '.' '*'
+ do
+ before=`ls -a -- $target_dir`
+ zpty subshell $ZTST_testdir/../Src/zsh -f
+ [[ $PWD == */options.tmp ]] || return 1 # Sanity check before calling rm(1).
+ zpty -w subshell "rm $target_pattern"
+ zpty -w subshell 'n'
+ sleep 1
+ zpty -rt subshell REPLY # "${PS1} rm *"
+ zpty -rt subshell REPLY && print -r -- ${REPLY%%$'\r\n'}
+ zpty -d subshell
+ after=`ls -a -- $target_dir`
+ [[ $before == $after ]] || return 1
+ done
+ else
+ ZTST_skip="the zsh/zpty module is not available"
+ fi
+ BEL=$'\a'
+0q:RM_STAR_SILENT
+*>zsh: sure you want to delete all 15 files in ${PWD:h}/options.tmp \[yn\]\? ${BEL}
From 6a83700ee1f5f08c17c8438c66bb3fff48060182 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Fri, 3 Jul 2020 13:37:46 +0000
Subject: [PATCH 2/3] Add a regression test for 46169: the RM_STAR_SILENT logic
processes the current directory rather than the root directory.
The bug will be fixed in the next commit.
---
Test/E01options.ztst | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index a1b564327..053affeed 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -1428,7 +1428,8 @@ F:If this test fails at the first unsetopt, refer to P01privileged.ztst.
if zmodload -e zsh/zpty 2>/dev/null; then
for target_dir target_pattern in \
- '.' '*'
+ '.' '*' \
+ '/' '/*'
do
before=`ls -a -- $target_dir`
zpty subshell $ZTST_testdir/../Src/zsh -f
@@ -1446,5 +1447,6 @@ F:If this test fails at the first unsetopt, refer to P01privileged.ztst.
ZTST_skip="the zsh/zpty module is not available"
fi
BEL=$'\a'
-0q:RM_STAR_SILENT
+0qf:RM_STAR_SILENT
*>zsh: sure you want to delete all 15 files in ${PWD:h}/options.tmp \[yn\]\? ${BEL}
+*>zsh: sure you want to delete all <-> files in / \[yn\]\? ${BEL}
From 81b1f0018d2557aa607080570ea1d78237c006c7 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Fri, 3 Jul 2020 13:38:58 +0000
Subject: [PATCH 3/3] Fix the RM_STAR_SILENT bug from the parent commit.
---
Src/exec.c | 2 +-
Test/E01options.ztst | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Src/exec.c b/Src/exec.c
index 045b5d2b9..7120a2c34 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3401,7 +3401,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
int rmall;
s[l - 2] = 0;
- rmall = checkrmall(s);
+ rmall = checkrmall(l == 2 ? "/" : s);
s[l - 2] = t;
if (!rmall) {
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 053affeed..c59509f2e 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -1447,6 +1447,6 @@ F:If this test fails at the first unsetopt, refer to P01privileged.ztst.
ZTST_skip="the zsh/zpty module is not available"
fi
BEL=$'\a'
-0qf:RM_STAR_SILENT
+0q:RM_STAR_SILENT
*>zsh: sure you want to delete all 15 files in ${PWD:h}/options.tmp \[yn\]\? ${BEL}
*>zsh: sure you want to delete all <-> files in / \[yn\]\? ${BEL}
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-07-03 13:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01 10:02 [Bug] incorrect warning when I type rm /* Jonas Bräutigam
2020-07-01 11:00 ` Daniel Shahaf
2020-07-01 11:33 ` Mikael Magnusson
2020-07-01 12:50 ` Daniel Shahaf
2020-07-03 13:41 ` Daniel Shahaf
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
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 NNTP newsgroup(s).