Re: [PATCH] parser: Fix VSLENGTH parsing with trailing garbage

[Denys Vlasenko <vda.linux@xxxxxxxxxxxxxx>]

On Mon, Jun 21, 2021 at 11:57 AM Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:
> On Sat, Jun 19, 2021 at 02:44:46PM +0200, Denys Vlasenko wrote:
> > CTLVAR and CTLBACKQ are not properly handled if encountered
> > inside {$#...}. Testcase:
> >
> > dash -c "`printf 'echo ${#1\x82}'`" 00 111 222
...
> In fact these two bugs are one and the same.  This patch fixes
> both by detecting the invalid substitution and not emitting it
> into the node tree.
>
> Incidentally this reveals a bug in how we parse ${#10} that got
> introduced recently, which is also fixed here.
...
> --- a/src/parser.h
> +++ b/src/parser.h
> @@ -62,6 +62,7 @@
>  #define VSTRIMLEFT     0x8             /* ${var#pattern} */
>  #define VSTRIMLEFTMAX  0x9             /* ${var##pattern} */
>  #define VSLENGTH       0xa             /* ${#var} */
> +/* VSLENGTH must come last. */

The above assumption is not necessary if...


> diff --git a/src/parser.c b/src/parser.c
> index 3c80d17..13c2df5 100644
> --- a/src/parser.c
> +++ b/src/parser.c
> @@ -1252,7 +1252,8 @@ varname:
>                         do {
>                                 STPUTC(c, out);
>                                 c = pgetc_eatbnl();
> -                       } while (!subtype && is_digit(c));
> +                       } while ((subtype <= 0 || subtype >= VSLENGTH) &&
> +                                is_digit(c));

... you use (subtype == 0 || subtype == VSLENGTH) here.
Also, (subtype == 0 || subtype == VSLENGTH) is less confusing:
it says "loop if ${VAR} or ${#VAR} syntax", whereas <= >=
are a bit misleading.