GNU bug report logs - #30724
eshell: escaped tilde is not treated as such

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 30724 in the body.
You can then email your comments to 30724 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: Yegor Timoshenko <yegortimoshenko <at> riseup.net>
To: bug-gnu-emacs <at> gnu.org
Subject: eshell: escaped tilde is not treated as such
Date: Tue, 6 Mar 2018 04:30:21 +0000

In M-x eshell (do not run):

  $ touch \~
  $ ls
  ~
  $ rm \~
  rm: cannot remove '~': Is a directory

I.e. it tried to remove home directory. I've found this by
accidentially wiping my home directory, but no worry, I've restored a
btrfs snapshot that was made several minutes before that.

GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.26)
 of 2018-02-25

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Yegor Timoshenko <yegortimoshenko <at> riseup.net>
Cc: 30724 <at> debbugs.gnu.org
Subject: Re: bug#30724: eshell: escaped tilde is not treated as such
Date: Thu, 08 Mar 2018 20:15:16 -0500

tags 30724 + confirmed
quit

Yegor Timoshenko <yegortimoshenko <at> riseup.net> writes:

> In M-x eshell (do not run):
>
>   $ touch \~
>   $ ls
>   ~
>   $ rm \~
>   rm: cannot remove '~': Is a directory
>
> I.e. it tried to remove home directory. I've found this by
> accidentially wiping my home directory, but no worry, I've restored a
> btrfs snapshot that was made several minutes before that.
>
> GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.26)
>  of 2018-02-25

Stepping through eshell/rm a bit, I see it gets #("~" 0 1 (escaped t))
as an argument.  So each command is responsible for handling the
escaping correctly.  I think every file-handling eshell function needs
review for this.

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

From: Jonathan Kyle Mitchell <kyle <at> jonathanmitchell.org>
To: 30724 <at> debbugs.gnu.org
Cc: Noam Postavsky <npostavs <at> gmail.com>, ";
 Yegor Timoshenko" <yegortimoshenko <at> riseup.net>
Subject: eshell: escaped tilde is not treated as such
Date: Fri, 06 Jul 2018 02:24:20 -0500

[Message part 1 (text/plain, inline)]

tags 30724 + patch
quit

I found a way to get eshell to escape special chars.  It seems most of
the eshell builtin commands that take file arguments are already tagged
with the eshell-no-numeric-conversions property.  By adding the true
part of the if condition in eshell-lisp-command, it is possible to
quote the arguments of special filenames.  The attached patch checks
for "~" and "*".

;; for reference, here's the set of eshell builtins with the
;; eshell-no-numeric-conversions property
./esh-proc.el\0202:(put 'eshell/kill 'eshell-no-numeric-conversions t)
./em-dirs.el\0409:(put 'eshell/cd 'eshell-no-numeric-conversions t)
./em-dirs.el\0472:(put 'eshell/pushd 'eshell-no-numeric-conversions t)
./em-dirs.el\0502:(put 'eshell/popd 'eshell-no-numeric-conversions t)
./esh-ext.el\0261:(put 'eshell/addpath 'eshell-no-numeric-conversions t
./esh-util.el\091:  (put \\='find-file \\='eshell-no-numeric-
./em-script.el\0127:(put 'eshell/source 'eshell-no-numeric-conversions
./em-script.el\0140:(put 'eshell/. 'eshell-no-numeric-conversions t)
./em-unix.el\0167:(put 'eshell/man 'eshell-no-numeric-conversions t)
./em-unix.el\0309:(put 'eshell/rm 'eshell-no-numeric-conversions t)
./em-unix.el\0326:(put 'eshell/mkdir 'eshell-no-numeric-conversions t)
./em-unix.el\0342:(put 'eshell/rmdir 'eshell-no-numeric-conversions t)
./em-unix.el\0526:(put 'eshell/mv 'eshell-no-numeric-conversions t)
./em-unix.el\0563:(put 'eshell/cp 'eshell-no-numeric-conversions t)
./em-unix.el\0595:(put 'eshell/ln 'eshell-no-numeric-conversions t)
./em-unix.el\0647:(put 'eshell/cat 'eshell-no-numeric-conversions t)
./em-unix.el\0664:(put 'eshell/make 'eshell-no-numeric-conversions t)
./em-unix.el\01031:(put 'eshell/diff 'eshell-no-numeric-conversions t)
./em-unix.el\01050:(put 'eshell/locate 'eshell-no-numeric-conversions 
./em-unix.el\01059:(put 'eshell/occur 'eshell-no-numeric-conversions t)
./esh-cmd.el\01185:(put 'eshell/which 'eshell-no-numeric-conversions t)
./em-ls.el\0336:(put 'eshell/ls 'eshell-no-numeric-conversions t)
./em-tramp.el\097:(put 'eshell/su 'eshell-no-numeric-conversions t)
./em-tramp.el\0139:(put 'eshell/sudo 'eshell-no-numeric-conversions t)

--
Jonathan Kyle Mitchell

[0001-Check-for-filenames-special-to-the-shell-before-runn.patch (text/x-patch, attachment)]

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Jonathan Kyle Mitchell <kyle <at> jonathanmitchell.org>
Cc: 30724 <at> debbugs.gnu.org, Yegor Timoshenko <yegortimoshenko <at> riseup.net>
Subject: Re: bug#30724: eshell: escaped tilde is not treated as such
Date: Sat, 07 Jul 2018 15:17:30 -0400

Jonathan Kyle Mitchell <kyle <at> jonathanmitchell.org> writes:

> tags 30724 + patch
> quit

You have to send this control <at> debbugs.gnu.org to have effect, generally
you should use Bcc so that further replies don't go there (which is why
you don't see it when other people do this).

> I found a way to get eshell to escape special chars.  It seems most of
> the eshell builtin commands that take file arguments are already tagged
> with the eshell-no-numeric-conversions property.  By adding the true
> part of the if condition in eshell-lisp-command, it is possible to
> quote the arguments of special filenames.  The attached patch checks
> for "~" and "*".

Perhaps we should distinguish between file and non-numeric arguments
though?  E.g., I think the file-name-quote might not make sense for the
commands below:

> ./esh-proc.el\0202:(put 'eshell/kill 'eshell-no-numeric-conversions t)

> ./em-unix.el\0167:(put 'eshell/man 'eshell-no-numeric-conversions t)

> ./em-unix.el\0664:(put 'eshell/make 'eshell-no-numeric-conversions t)

> ./em-unix.el\01050:(put 'eshell/locate 'eshell-no-numeric-conversions 
> ./em-unix.el\01059:(put 'eshell/occur 'eshell-no-numeric-conversions t)
> ./esh-cmd.el\01185:(put 'eshell/which 'eshell-no-numeric-conversions t)

> ./em-tramp.el\097:(put 'eshell/su 'eshell-no-numeric-conversions t)
> ./em-tramp.el\0139:(put 'eshell/sudo 'eshell-no-numeric-conversions t)


> Fix bug#30724 by checking if "*" and "~" are arguments to the current command
> and quoting them relative to `default-directory' if so.  This leverages the
> the fact that the existing eshell builtin commands that accept file arguments
> are tagged with the eshell-no-numeric-conversions property.  The existing
> details of eshell command execution are left unchanged.
>
> * lisp/eshell/esh-cmd.el (eshell-lisp-command)

The ChangeLog item should at the beginning, as in,

* lisp/eshell/esh-cmd.el (eshell-lisp-command): Fix bug#30724 by
checking if "*" and "~" are arguments to the current command ...

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

From: Jonathan Kyle Mitchell <kyle <at> jonathanmitchell.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 30724 <at> debbugs.gnu.org, Yegor Timoshenko <yegortimoshenko <at> riseup.net>
Subject: Re: bug#30724: eshell: escaped tilde is not treated as such
Date: Sun, 15 Jul 2018 12:18:26 -0500

[Message part 1 (text/plain, inline)]

On Sat, 2018-07-07 at 15:17 -0400, Noam Postavsky wrote:
> Perhaps we should distinguish between file and non-numeric arguments
> though?  E.g., I think the file-name-quote might not make sense for
> the
> commands below:
> 
> > ./esh-proc.el\0202:(put 'eshell/kill 'eshell-no-numeric-conversions 
> > t)
> > ./em-unix.el\0167:(put 'eshell/man 'eshell-no-numeric-conversions
> > t)
> > ./em-unix.el\0664:(put 'eshell/make 'eshell-no-numeric-conversions
> > t)
> > ./em-unix.el\01050:(put 'eshell/locate 'eshell-no-numeric-
> > conversions 
> > ./em-unix.el\01059:(put 'eshell/occur 'eshell-no-numeric-
> > conversions t)
> > ./esh-cmd.el\01185:(put 'eshell/which 'eshell-no-numeric-
> > conversions t)
> > ./em-tramp.el\097:(put 'eshell/su 'eshell-no-numeric-conversions t)
> > ./em-tramp.el\0139:(put 'eshell/sudo 'eshell-no-numeric-conversions 
> > t)

After tracing through the execution of eshell some more, I noticed that
eshell sets an escaped text property on the arguments when the user
escaped them with `\' or quote characters but there is nothing in
eshell that does anything with it apparently.

I've attached a new patch that checks for the escaped text property on
any single character argumentand.  I've also added a new symbol
property to the following functions that take filename arguments and
use that to check instead of the eshell-no-numeric-conversions
property.

(eshell/cd)
(eshell/pushd)
(eshell/popd)
(eshell/ls)
(eshell/rm)
(eshell/mkdir)
(eshell/rmdir)
(eshell/mv)
(eshell/cp)
(eshell/ln)
(eshell/cat)
(eshell/du)
(eshell/diff)
(eshell/addpath)

> 
> The ChangeLog item should at the beginning, as in,
> 
> * lisp/eshell/esh-cmd.el (eshell-lisp-command): Fix bug#30724 by
> checking if "*" and "~" are arguments to the current command ...

Thanks for the comments and suggestions, I fixed the commit message in the new patch.

--
Jonathan Kyle Mitchell

[0001-Check-for-special-filenames-in-eshell-Bug-30724.patch (text/x-patch, attachment)]

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Jonathan Kyle Mitchell <kyle <at> jonathanmitchell.org>
Cc: 30724 <at> debbugs.gnu.org, Yegor Timoshenko <yegortimoshenko <at> riseup.net>
Subject: Re: bug#30724: eshell: escaped tilde is not treated as such
Date: Mon, 16 Jul 2018 20:14:03 -0400

Jonathan Kyle Mitchell <kyle <at> jonathanmitchell.org> writes:

> After tracing through the execution of eshell some more, I noticed that
> eshell sets an escaped text property on the arguments when the user
> escaped them with `\' or quote characters but there is nothing in
> eshell that does anything with it apparently.

Yeah, this sort of half-finished thing seems to pop up in eshell quite a
bit.

> Subject: [PATCH] Check for special filenames in eshell (Bug#30724)
>
> 	* lisp/eshell/em-dirs.el (eshell/cd): Add
> 	eshell-escape-special-filenames to symbol plist.
> 	(eshell/pushd): Likewise.
> 	(eshell/popd): Likewise.
>
> 	* lisp/eshell/em-ls.el (eshell/ls): Add
> 	eshell-escape-special-filenames to symbol plist.
>
> 	* lisp/eshell/em-unix.el (eshell/rm): Add
> 	eshell-escape-special-filenames to symbol plist.
> 	(eshell/mkdir): Likewise.
> 	(eshell/rmdir): Likewise.
> 	(eshell/mv): Likewise.
> 	(eshell/cp): Likewise.
> 	(eshell/ln): Likewise.
> 	(eshell/cat): Likewise.
> 	(eshell/du): Likewise.
> 	(eshell/diff): Likewise.
>
> 	* lisp/eshell/esh-cmd.el (eshell-lisp-command):	Check for
> 	single character escaped string arguments (Bug#30724).
>
> 	* lisp/eshell/esh-ext.el (eshell/addpath): Add
> 	eshell-escape-special-filenames to symbol plist.

The "Add eshell-escape-special-filenames to symbol plist" should be
coalesced, as in:

* lisp/eshell/em-dirs.el (eshell/cd, eshell/pushd, eshell/popd):
* lisp/eshell/em-ls.el (eshell/ls):
* lisp/eshell/em-unix.el (eshell/rm, eshell/mkdir, eshell/rmdir)
(eshell/mv, eshell/cp, eshell/ln, eshell/cat, eshell/du, eshell/diff):
* lisp/eshell/esh-ext.el (eshell/addpath): Add
eshell-escape-special-filenames to symbol plist.

* lisp/eshell/esh-cmd.el (eshell-lisp-command):	Check for
single character escaped string arguments (Bug#30724).
  

> +		  (let ((numeric (not (get object
> +                                           'eshell-no-numeric-conversions)))
> +			(escaped (get object 'eshell-escape-special-filenames)))

I'm not sure 'escaped' or 'eshell-escape-special-filenames' are really
the right names here, as they're not actually indicating any escaping.
Maybe the property should be something like 'eshell-filename-arguments'?

> +		    (when (or numeric escaped)
> +		      (while args
> +			(let ((arg (car args)))
> +			  (cond ((and numeric (stringp arg) (> (length arg) 0)
> +				      (text-property-any 0 (length arg)
> +							 'number t arg))
> +				 ;; if any of the arguments are flagged as
> +				 ;; numbers waiting for conversion, convert
> +				 ;; them now

Comments should be capitalized and end with a period (I know the
original didn't, but since we're updating the code, now is a good time
to fix it).

> +				 (setcar args (string-to-number arg)))
> +				((and escaped (stringp arg) (= (length arg) 1)
> +				      (text-property-any 0 (length arg)
> +							 'escaped t arg))
> +				 ;; if any of the arguments are single
> +				 ;; character escaped filenames,
> +				 ;; prepend "./"
> +				 (setcar args (concat "./" arg)))))

It looks like this kind of escaping isn't actually needed for "*" which
your previous patch mentioned.  Perhaps just check for "~" explicitly,
rather than any single character filenames.  Or are there other examples
besides "~" which need "./" to prevent interpretation?

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

From: Jonathan Kyle Mitchell <kyle <at> jonathanmitchell.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 30724 <at> debbugs.gnu.org, Yegor Timoshenko <yegortimoshenko <at> riseup.net>
Subject: Re: bug#30724: eshell: escaped tilde is not treated as such
Date: Tue, 17 Jul 2018 22:54:27 -0500

[Message part 1 (text/plain, inline)]

On Mon, 2018-07-16 at 20:14 -0400, Noam Postavsky wrote:
> The "Add eshell-escape-special-filenames to symbol plist" should be
> coalesced, as in:

Done in the new attached patch.

> I'm not sure 'escaped' or 'eshell-escape-special-filenames' are
> really
> the right names here, as they're not actually indicating any
> escaping.
> Maybe the property should be something like 'eshell-filename-
> arguments'?

I changed the property name to eshell-filename-arguments.

> Comments should be capitalized and end with a period (I know the
> original didn't, but since we're updating the code, now is a good
> time
> to fix it).

Fixed both the original and new comment.

> It looks like this kind of escaping isn't actually needed for "*"
> which
> your previous patch mentioned.  Perhaps just check for "~"
> explicitly,
> rather than any single character filenames.  Or are there other
> examples
> besides "~" which need "./" to prevent interpretation?

I can't think of any other cases that would need to be check either, so
I changed to to just string-match "~" only.

--
Jonathan Kyle Mitchell

[0001-Check-for-special-filenames-in-eshell-Bug-30724.patch (text/x-patch, attachment)]

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Jonathan Kyle Mitchell <kyle <at> jonathanmitchell.org>
Cc: 30724 <at> debbugs.gnu.org, Yegor Timoshenko <yegortimoshenko <at> riseup.net>
Subject: Re: bug#30724: eshell: escaped tilde is not treated as such
Date: Sat, 21 Jul 2018 21:34:47 -0400

tags 30724 fixed
close 30724 26.2
quit

Jonathan Kyle Mitchell <kyle <at> jonathanmitchell.org> writes:

> Done in the new attached patch.

Pushed to emacs-26.

[1: 5de444112c]: 2018-07-21 21:07:07 -0400
  Check for special filenames in eshell (Bug#30724)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=5de444112cf19c078d4a74752a50e890233ef033>

> 	* lisp/eshell/em-unix.el (eshell/rm, eshell/mkdir, eshell/rmdir):

A minor nitpick about formatting here, ChangeLog entries for one file
continued over multiple lines shouldn't have a colon at the end of line
(I fixed this before pushing).

> 	(eshell/mv, eshell/cp, eshell/ln, eshell/cat, eshell/du, eshell/diff):

Previous Next

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