bug#54486: 29.0.50; Eshell `escaped' string property can "leak" into output

bug#54486: 29.0.50; Eshell `escaped' string property can "leak" into output

[Jim Porter]

When using Eshell, it's possible to inadvertently add an `escaped' 
string property to strings, resulting in some pretty surprising 
behavior. Starting from "emacs -Q --eval '(eshell)'":

   ~ $ setq var (list "foo" "bar")
   ("foo" "bar")
   ~ $ echo $var
   ("foo" "bar")
   ~ $ echo $var[0]
   foo
   ~ $ echo $var
   (#("foo" 0 3
      (escaped t))
    "bar")

This happens because when the `$var[0]' argument is parsed in the third 
command, the function `eshell-interpolate-variable' wraps the 
code-to-be-called with `eshell-escape-arg'. That function adds an 
`escaped' property to any string passed into it.

The `escaped' property is used to indicate that the string should be 
treated literally (i.e. any special characters like "$" will no longer 
have any special meaning in Eshell). That's the right *behavior*, but 
ideally, there'd be a way to do so that doesn't involve manipulating the 
string like this. Eshell can't know the lifetime of the string, and it 
seems like a bad idea in general to go around messing with string 
properties just because you referenced that string somehow in Eshell.

I can't think of an obvious fix for this though. Any ideas?

bug#54486: 29.0.50; Eshell `escaped' string property can "leak" into output

[Jim Porter]

On 3/20/2022 8:52 PM, Jim Porter wrote:
> I can't think of an obvious fix for this though. Any ideas?

I forgot to add: the obvious solution might be "just copy the string 
before propertizing it", but the string might be very long (e.g. output 
from an external command), so I think it's probably best to avoid making 
more copies than absolutely necessary.

bug#54486: 29.0.50; Eshell `escaped' string property can "leak" into output

[Eli Zaretskii]

> From: Jim Porter <jporterbugs@gmail.com>
> Date: Sun, 20 Mar 2022 20:52:43 -0700
> 
> When using Eshell, it's possible to inadvertently add an `escaped' 
> string property to strings, resulting in some pretty surprising 
> behavior. Starting from "emacs -Q --eval '(eshell)'":
> 
>    ~ $ setq var (list "foo" "bar")
>    ("foo" "bar")
>    ~ $ echo $var
>    ("foo" "bar")
>    ~ $ echo $var[0]
>    foo
>    ~ $ echo $var
>    (#("foo" 0 3
>       (escaped t))
>     "bar")
> 
> This happens because when the `$var[0]' argument is parsed in the third 
> command, the function `eshell-interpolate-variable' wraps the 
> code-to-be-called with `eshell-escape-arg'. That function adds an 
> `escaped' property to any string passed into it.
> 
> The `escaped' property is used to indicate that the string should be 
> treated literally (i.e. any special characters like "$" will no longer 
> have any special meaning in Eshell). That's the right *behavior*, but 
> ideally, there'd be a way to do so that doesn't involve manipulating the 
> string like this. Eshell can't know the lifetime of the string, and it 
> seems like a bad idea in general to go around messing with string 
> properties just because you referenced that string somehow in Eshell.
> 
> I can't think of an obvious fix for this though. Any ideas?

Why exactly do you think this should be fixed?  I don't think I
understand that from your description.  We have many features that add
text properties to strings, so why is this one different?  About the
only thing I can think of is to add this property to the default value
of yank-excluded-properties, but that's all.

bug#54486: 29.0.50; Eshell `escaped' string property can "leak" into output

[Jim Porter]

On 3/21/2022 5:19 AM, Eli Zaretskii wrote:
> Why exactly do you think this should be fixed?  I don't think I
> understand that from your description.  We have many features that add
> text properties to strings, so why is this one different?  About the
> only thing I can think of is to add this property to the default value
> of yank-excluded-properties, but that's all.

I didn't know `yank-excluded-properties' existed. I think that would be 
a good thing to use for this.

Besides that, I think it would be best if Eshell didn't show the 
`escaped' property when printing things under normal circumstances. It's 
really just for internal bookkeeping in Eshell, so I think users are 
unlikely to want to see it in most cases. The best way to fix this might 
be to rethink how Eshell (specifically `eshell-stringify') prints lists 
by default. This is mentioned in bug#12689 (though that bug additionally 
describes an unrelated issue with Eshell subcommands that I've since fixed).

Finally, maybe it would be good to change the name of the `escaped' 
property to something like `eshell-escaped'. It took me quite a while to 
figure out where that property was even coming from when I first noticed 
it. Technically, that would be an incompatible change, but since this 
property is for internal bookkeeping in Eshell, I'd be very surprised if 
anyone else was relying on it.

bug#54486: 29.0.50; Eshell `escaped' string property can "leak" into output

[Eli Zaretskii]

> Cc: 54486@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Mon, 21 Mar 2022 12:24:09 -0700
> 
> I didn't know `yank-excluded-properties' existed. I think that would be 
> a good thing to use for this.
> 
> Besides that, I think it would be best if Eshell didn't show the 
> `escaped' property when printing things under normal circumstances. It's 
> really just for internal bookkeeping in Eshell, so I think users are 
> unlikely to want to see it in most cases. The best way to fix this might 
> be to rethink how Eshell (specifically `eshell-stringify') prints lists 
> by default. This is mentioned in bug#12689 (though that bug additionally 
> describes an unrelated issue with Eshell subcommands that I've since fixed).

Eshell commands that print strings could remove that property.

> Finally, maybe it would be good to change the name of the `escaped' 
> property to something like `eshell-escaped'.

Yes, that'd be a good thing.  But I'm worried that it could cause
failures in existing code, couldn't it?  We could do that as an
experiment and see if anyone complains.

bug#54486: 29.0.50; Eshell `escaped' string property can "leak" into output

[Jim Porter]

On 3/20/2022 8:52 PM, Jim Porter wrote:
> When using Eshell, it's possible to inadvertently add an `escaped' 
> string property to strings, resulting in some pretty surprising 
> behavior. Starting from "emacs -Q --eval '(eshell)'":
> 
>    ~ $ setq var (list "foo" "bar")
>    ("foo" "bar")
>    ~ $ echo $var
>    ("foo" "bar")
>    ~ $ echo $var[0]
>    foo
>    ~ $ echo $var
>    (#("foo" 0 3
>       (escaped t))
>     "bar")

In the intervening years, I've improved Eshell's parser to prevent other 
bugs, which has resulted in the 'escaped' string property no longer 
being useful.

Instead, Eshell now propertizes text that has actual syntactic meaning: 
for example a globbing character like "*" gets the 'eshell-glob-char' 
text property. By marking *syntactic* characters with a text property, 
we ensure that Eshell only ever adds properties to text literally 
written into the Eshell buffer, which avoids the issue here.

As a result of all this, we can now remove the calls that added that 
property in Eshell. I've now made this change in b4655ff99b5, so closing 
this bug.