a='foo"'; echo ${a/foo"/"bar} outputs bar

a='foo"'; echo ${a/foo"/"bar} outputs bar

[Stephane Chazelas]

$ a='foo"'
$ echo ${a/foo"/"bar}
bar

Whether quotes should escape the "/" is not clearly documented,
though the doc does tell us to use backslash for that.

However the fact that the first " is taken as being part of the
pattern while the second one is removed doesn't make much sense.

In ksh93, bash and mksh, quotes can be used to escape the /,
i.e. one can do:

$ a='/x/y/z' bash -c 'echo "${a/"/y/"/+}"'
/x+z
$ a='/x/y/z' ksh -c 'echo "${a/"/y/"/+}"'
/x+z
$ a='/x/y/z' mksh -c 'echo "${a/"/y/"/+}"'
/x+z

Maybe zsh could align with them?

The csh-style one looks better:

$ a='foo"' zsh -c 'echo ${a:s/foo"/"bar}'
bar"

quotes don't escape the / either but at least the " is to taken  as part of the
pattern.

Better than tcsh anyway:

$ a='foo"' tcsh -c 'echo ${a:s/foo"/"bar}'
^C^C^C^Z
zsh: suspended  a='foo"' tcsh -c 'echo ${a:s/foo"/"bar}'
(148)$ kill %

(ended up using all my RAM).

-- 
Stephane

Re: a='foo"'; echo ${a/foo"/"bar} outputs bar

[Bart Schaefer]

On Sun, Dec 11, 2022 at 9:50 AM Stephane Chazelas <stephane@chazelas.org> wrote:
>
> $ a='foo"'
> $ echo ${a/foo"/"bar}
> bar
>
> Whether quotes should escape the "/" is not clearly documented,
> though the doc does tell us to use backslash for that.

The whole expression only works because the quotes are balanced:

% echo ${x/foo"/bar}
braceparam dquote>

Clearly something a little wacky is going on here.

> However the fact that the first " is taken as being part of the
> pattern while the second one is removed doesn't make much sense.

This is the fault of singsub() called from line 2670 of
compgetmatch(), which eventually calls remnulargs() via prefork(),
which deletes the Dnull representing the double-quote.  This is only a
problem because an unbalanced quote was able to sneak through.

This was actually caught at paramsubst() line 3118:
                haserr = parse_subst_string(s);
This returned haserr == 1, but the only effect of that is that the
string is retokenized at line 3124.  If you use ${(X)x/foo"/"bar} you
get
  zsh: unmatched "

Next question is where this should be fixed.  The following works for
double-quotes, but not single because of special handling required for
$'...' -- so should be considered only a proof of how the substitution
could work with more conventional quoting.  All tests still pass with
the below.

diff --git a/Src/subst.c b/Src/subst.c
index 0f98e6ea3..86ee1dad8 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -2911,6 +2911,9 @@ paramsubst(LinkList l, LinkNode n, char **str,
int qt, int pf_flags,
             else
             ptr++;
         }
+        if (c == Dnull)
+            while (ptr[1] && ptr[1] != c)
+            ptr++;
         }
         replstr = (*ptr && ptr[1]) ? ptr+1 : "";
         *ptr = '\0';

Re: a='foo"'; echo ${a/foo"/"bar} outputs bar

[Bart Schaefer]

Circling back to this ...

On Mon, Dec 12, 2022 at 10:21 PM Bart Schaefer
<schaefer@brasslantern.com> wrote:
>
> On Sun, Dec 11, 2022 at 9:50 AM Stephane Chazelas <stephane@chazelas.org> wrote:
> >
> > $ a='foo"'
> > $ echo ${a/foo"/"bar}
> > bar
> >
> > Whether quotes should escape the "/" is not clearly documented,
> > though the doc does tell us to use backslash for that.
>
> The whole expression only works because the quotes are balanced:
>
> % echo ${x/foo"/bar}
> braceparam dquote>
>
> Clearly something a little wacky is going on here.

With an increased understanding of lex.c gained by stepping through
variations of ${|...} a thousand times, I think I have identified two
possible ways to address this.

> > However the fact that the first " is taken as being part of the
> > pattern while the second one is removed doesn't make much sense.
>
> This is the fault of singsub() called from line 2670 of
> compgetmatch(), which eventually calls remnulargs() via prefork(),
> which deletes the Dnull representing the double-quote.  This is only a
> problem because an unbalanced quote was able to sneak through.

One possible change is for the first quote to be taken as part of the
pattern but the second quote to NOT be removed.  This results in

$ echo ${a/foo"/"bar}
braceparam dquote>

It's always been possible to do quoted substrings in the replacement
rather than in the pattern, so this is consistent.  But it also means
that

$ a='foo"'
$ x= bar
$ echo ${a/foo"/"$x"}
$ bar

Something similar would have to be done with single quotes, because
handling double quotes doesn't fix this oddity:

% a="foo'"
% x=bar
% echo ${a/foo'/$'${x}y}
$bary

That is, the single quote after the slash is simply ignored in the
replacement.  That happens in 5.9, 5.8, and earlier.

The second possible change is to report "bad substitution" for the
original example.  As mentioned in the previous post,

> This was actually caught at paramsubst() line 3118:
>                 haserr = parse_subst_string(s);
> This returned haserr == 1, but the only effect of that is that the
> string is retokenized at line 3124.

An option I haven't pursued so far is to allow double-quoted
substrings to appear in the pattern.  This would make some sense given
that we allow parameter expansions in the pattern and can protect them
with single quotes, but using single quotes in the pattern has other
unexpected side-effects on the replacement:

% echo ${a/'foo"'/"$x"y}
"bar"y

Incidentally, it's sort-of possible in 5.8 to use $'...' quoting in
patterns, but the same odd stuff is going on there as well.

Thoughts?