PATCH: quote parameter expansion from GLOB_SUBST

PATCH: quote parameter expansion from GLOB_SUBST

[Peter Stephenson]

While I'm waiting for some last changes for 5.0.8 to roll in, this adds
the (b) parameter flag to backslash-quote against pattern characters
only, needed to protect contents against later GLOB_SUBST including
${~....}.  The doc is supposed to explain.  We could use this at various
places in the completion system instead of the hard-to-figure-out mess
you need instead.

pws

diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo
index 85191ed..9bba4f4 100644
--- a/Doc/Zsh/expn.yo
+++ b/Doc/Zsh/expn.yo
@@ -1014,6 +1014,25 @@ form of single quoting is used that only quotes the string if needed to
 protect special characters.  Typically this form gives the most readable
 output.
 )
+item(tt(b))(
+Quote with backslashes only characters that are special to pattern
+matching. This is useful when the contents of the variable are to be
+tested using tt(GLOB_SUBST), including the tt(${~)var(...)tt(}) switch.
+
+Quoting using one of the tt(q) family of flags does not work
+for this purpose since quotes are not stripped from non-pattern
+characters by tt(GLOB_SUBST).  In other words,
+
+example(foo='a\ b'
+[[ 'a b' = ${~foo} ]])
+
+fails, whereas
+
+example(foo='a\*b'
+[[ 'a*b' = ${~foo} ]])
+
+succeeds.  The tt(b) flag ensures the correct quoting.
+)
 item(tt(Q))(
 Remove one level of quotes from the resulting words.
 )
diff --git a/Src/subst.c b/Src/subst.c
index f52bcdf..bf80495 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -1592,7 +1592,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags)
     /* combination of (L), (U) and (C) flags. */
     int casmod = CASMOD_NONE;
     /*
-     * quotemod says we are doing either (q) (positive), (Q) (negative)
+     * quotemod says we are doing either (q/b) (positive), (Q) (negative)
      * or not (0).  quotetype counts the q's for the first case.
      * quoterr is simply (X) but gets passed around a lot because the
      * combination (eX) needs it.
@@ -1861,6 +1861,12 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags)
 			quotemod++, quotetype++;
 		    }
 		    break;
+		case 'b':
+		    if (quotemod || quotetype != QT_NONE)
+			goto flagerr;
+		    quotemod = 1;
+		    quotetype = QT_BACKSLASH_PATTERN;
+		    break;
 		case 'Q':
 		    quotemod--;
 		    break;
@@ -3460,7 +3466,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags)
     if (quotemod) {
 	int pre = 0, post = 0;
 
-	if (quotemod > 0 && quotetype > QT_BACKSLASH) {
+	if (quotemod > 0) {
 	    switch (quotetype)
 	    {
 	    case QT_DOLLARS:
@@ -3471,6 +3477,9 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags)
 
 	    case QT_SINGLE_OPTIONAL:
 		/* quotes will be added for us */
+	    case QT_BACKSLASH:
+	    case QT_BACKSLASH_PATTERN:
+		/* no quotes */
 		break;
 
 	    default:
diff --git a/Src/utils.c b/Src/utils.c
index 13d4b83..271c800 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -3694,6 +3694,8 @@ inittyptab(void)
 	typtab[bangchar] |= ISPECIAL;
     } else
 	typtab_flags &= ~ZTF_BANGCHAR;
+    for (s = PATCHARS; *s; s++)
+	typtab[STOUC(*s)] |= IPATTERN;
 
     unqueue_signals();
 }
@@ -5075,6 +5077,10 @@ quotestring(const char *s, char **e, int instring)
 	alloclen = slen * 7 + 1;
 	break;
 
+    case QT_BACKSLASH_PATTERN:
+	alloclen = slen * 2  + 1;
+	break;
+
     case QT_SINGLE_OPTIONAL:
 	/*
 	 * Here, we may need to add single quotes.
@@ -5094,7 +5100,7 @@ quotestring(const char *s, char **e, int instring)
     quotestart = v = buf = zshcalloc(alloclen);
 
     DPUTS(instring < QT_BACKSLASH || instring == QT_BACKTICK ||
-	  instring > QT_SINGLE_OPTIONAL,
+	  instring > QT_BACKSLASH_PATTERN,
 	  "BUG: bad quote type in quotestring");
     u = s;
     if (instring == QT_DOLLARS) {
@@ -5134,9 +5140,18 @@ quotestring(const char *s, char **e, int instring)
 		u = uend;
 	    }
 	}
-    }
-    else
-    {
+    } else if (instring == QT_BACKSLASH_PATTERN) {
+	while (*u) {
+	    if (e && !sf && *e == u) {
+		*e = v;
+		sf = 1;
+	    }
+
+	    if (ipattern(*u))
+		*v++ = '\\';
+	    *v++ = *u++;
+	}
+    } else {
 	if (shownull) {
 	    /* We can't show an empty string with just backslash quoting. */
 	    if (!*u) {
diff --git a/Src/zsh.h b/Src/zsh.h
index 486ad80..5479198 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -215,6 +215,10 @@ struct mathfunc {
 
 #define SPECCHARS "#$^*()=|{}[]`<>?~;&\n\t \\\'\""
 
+/* chars that need to be quoted for pattern matching */
+
+#define PATCHARS "#^*()|[]<>?~"
+
 /*
  * Types of quote.  This is used in various places, so care needs
  * to be taken when changing them.  (Oooh, don't you look surprised.)
@@ -249,6 +253,12 @@ enum {
      */
     QT_SINGLE_OPTIONAL,
     /*
+     * Only quote pattern characters.
+     * ${(b)foo} guarantees that ${~foo} matches the string
+     * contained in foo.
+     */
+    QT_BACKSLASH_PATTERN,
+    /*
      * As QT_BACKSLASH, but a NULL string is shown as ''.
      */
     QT_BACKSLASH_SHOWNULL
diff --git a/Src/ztype.h b/Src/ztype.h
index b73e3f8..76589b1 100644
--- a/Src/ztype.h
+++ b/Src/ztype.h
@@ -42,6 +42,7 @@
 #define IMETA    (1 << 12)
 #define IWSEP    (1 << 13)
 #define INULL    (1 << 14)
+#define IPATTERN (1 << 15)
 #define zistype(X,Y) (typtab[STOUC(X)] & Y)
 #define idigit(X) zistype(X,IDIGIT)
 #define ialnum(X) zistype(X,IALNUM)
@@ -58,6 +59,7 @@
 #define imeta(X) zistype(X,IMETA)
 #define iwsep(X) zistype(X,IWSEP)
 #define inull(X) zistype(X,INULL)
+#define ipattern(X) zistype(X,IPATTERN)
 
 /*
  * Bit flags for typtab_flags --- preserved after
diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index 42c7b4e..17a59cb 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -1699,3 +1699,7 @@
 >
 >
 >Four
+
+  funnychars='The qu*nk br!wan f@x j/mps o[]r the la~# ^"&;'
+  [[ $funnychars = ${~${(b)funnychars}} ]]
+0:${(b)...} quoting protects from GLOB_SUBST

Re: PATCH: quote parameter expansion from GLOB_SUBST

[Daniel Shahaf]

Peter Stephenson wrote on Sun, May 10, 2015 at 00:44:44 +0100:
> +item(tt(b))(
> +Quote with backslashes only characters that are special to pattern
> +matching. This is useful when the contents of the variable are to be
> +tested using tt(GLOB_SUBST), including the tt(${~)var(...)tt(}) switch.
> +
> +Quoting using one of the tt(q) family of flags does not work
> +for this purpose since quotes are not stripped from non-pattern
> +characters by tt(GLOB_SUBST).  In other words,
> +
> +example(foo='a\ b'
> +[[ 'a b' = ${~foo} ]])
> +
> +fails, whereas
> +
> +example(foo='a\*b'
> +[[ 'a*b' = ${~foo} ]])
> +
> +succeeds.  The tt(b) flag ensures the correct quoting.
> +)

Maybe state here that '[[ $foo == ${~${(b)foo}} ]]' is always true (like
the code comments already state)?  That way the doc will have an example
of using the (b) modifier, not just an example of how using (q) doesn't
work :-)

Speaking of which, it's not immediately clear when reading the above
example that 'a\ b' and 'a\*b' are ${(q):-"a b"} and ${(q):-"a*b"}; if
that example remains, I would suggest stating that more explicitly.

So, overall:

  +Quoting using one of the tt(q) family of flags does not work
  +for this purpose since quotes are not stripped from non-pattern
  +characters by tt(GLOB_SUBST).  In other words,
  +
  +example(pattern=${(q)str}
  +[[ $str = ${~pattern} ]])
  +
  +works if tt($str) is tt('a*b') but not if it is tt('a b'), whereas
  +
  +example(pattern=${(b)str}
  +[[ $str = ${~pattern} ]])
  +
  +is always true for any possible value of tt($str).
  +)

Cheers,

Daniel

>      /*
> +     * Only quote pattern characters.
> +     * ${(b)foo} guarantees that ${~foo} matches the string
> +     * contained in foo.
> +     */

Re: PATCH: quote parameter expansion from GLOB_SUBST

[Peter Stephenson]

On Sun, 10 May 2015 16:07:59 +0000
Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> So, overall:

Thanks, I've used that.

I missed out backslashing backslash.  You sometimes get away with this;
the amended test shows a case where you don't.

pws

diff --git a/Src/zsh.h b/Src/zsh.h
index b2ab1df..f6e08e2 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -217,7 +217,7 @@ struct mathfunc {
 
 /* chars that need to be quoted for pattern matching */
 
-#define PATCHARS "#^*()|[]<>?~"
+#define PATCHARS "#^*()|[]<>?~\\"
 
 /*
  * Types of quote.  This is used in various places, so care needs
diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index 17a59cb..d96ffb6 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -1700,6 +1700,6 @@
 >
 >Four
 
-  funnychars='The qu*nk br!wan f@x j/mps o[]r the la~# ^"&;'
+  funnychars='The qu*nk br!wan f@x j/mps o[]r \(e la~# ^"&;'
   [[ $funnychars = ${~${(b)funnychars}} ]]
 0:${(b)...} quoting protects from GLOB_SUBST

Re: PATCH: quote parameter expansion from GLOB_SUBST

[Jun T.]

Sorry, it seems I've forgotten to rebase on the latest master
and the following hunk has been reverted by my recent push.
I'll push it again soon.

On 2015/05/10, at 8:44, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> 
> diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo
> index 85191ed..9bba4f4 100644
> --- a/Doc/Zsh/expn.yo
> +++ b/Doc/Zsh/expn.yo
> @@ -1014,6 +1014,25 @@ form of single quoting is used that only quotes the string if needed to
...

Re: PATCH: quote parameter expansion from GLOB_SUBST

[Jun T.]

Sorry again, I reverted expn.yo in a wrong way.
(suggestion by Daniel Shahaf was not included)

Now I reverted it to
commit 6269db883ad6989a578bac299deab9c4285bb1df
Author: Peter Stephenson <p.w.stephenson@ntlworld.com>
Date:   Sun May 10 19:19:34 2015 +0100
and applied my tiny patch.

Re: PATCH: quote parameter expansion from GLOB_SUBST

[Oliver Kiddle]

On 10 May, Peter wrote:
> the (b) parameter flag to backslash-quote against pattern characters

Playing with this, I managed to get a BUG message:

% a='*'
% echo ${(bqq)a}
 utils.c:5104: BUG: bad quote type in quotestring
'*'

We've got (q-) for minimal quoting. If - is a sort of parameter to q,
perhaps we should continue that: (q*) might be more memorable. And
combining (b) with (q) is a bit odd anyway.

Also the completion function should be kept up-to-date.

Oliver

diff --git a/Completion/Zsh/Context/_brace_parameter b/Completion/Zsh/Context/_brace_parameter
index fcca4bf..3955cb7 100644
--- a/Completion/Zsh/Context/_brace_parameter
+++ b/Completion/Zsh/Context/_brace_parameter
@@ -143,6 +143,7 @@ if [[ $PREFIX = *'${('[^\)]# ]]; then
     "@:double-quoted splitting of scalars"
     "A:create array parameter"
     "a:sort in array index order (with O to reverse)"
+    "b:backslash quote pattern characters only"
     "c:count characters in an array (with \${(c)#...})"
     "C:capitalize words"
     "D:perform directory name abbreviation"

Re: PATCH: quote parameter expansion from GLOB_SUBST

[Peter Stephenson]

On Thu, 14 May 2015 23:50:32 +0200
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> On 10 May, Peter wrote:
> > the (b) parameter flag to backslash-quote against pattern characters
> 
> Playing with this, I managed to get a BUG message:
> 
> % a='*'
> % echo ${(bqq)a}
>  utils.c:5104: BUG: bad quote type in quotestring
> '*'

I tested one way round (q first) but not the other.

> We've got (q-) for minimal quoting. If - is a sort of parameter to q,
> perhaps we should continue that: (q*) might be more memorable. And
> combining (b) with (q) is a bit odd anyway.

I thought about an argument to q, but nothing struck me as particularly
obvious, so I left q for generic quoting (all forms produced by any q
variant at the moment protect all characters by some means or another so
are e.g. eval safe --- up to bugs).

pws

diff --git a/Src/subst.c b/Src/subst.c
index bf80495..6345e36 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -1845,7 +1845,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags)
 		    break;
 
 		case 'q':
-		    if (quotetype == QT_DOLLARS)
+		    if (quotetype == QT_DOLLARS || QT_BACKSLASH_PATTERN)
 			goto flagerr;
 		    if (s[1] == '-') {
 			if (quotemod)

Re: PATCH: quote parameter expansion from GLOB_SUBST

[Peter Stephenson]

On Fri, 15 May 2015 09:46:14 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> +		    if (quotetype == QT_DOLLARS || QT_BACKSLASH_PATTERN)

Oh, good grief.  You know what I *meant*...

pws