Re: dash bug: double-quoted "\" breaks glob protection for next char

[Martijn Dekker <martijn@xxxxxxxx>]

Op 04-03-18 om 11:44 schreef Harald van Dijk:
> When CTLENDVAR is seen, I double-check that syntax has the expected
> value. This fixes the handling of "${$+"}"}", where the inner } was
> seen as ending the variable substitution.

Also looks like dash with your patch considers the "}" to be quoted:

$ src/dash -c 'IFS=}; printf %s\\n "${$+"}"}"'
}

Only AT&T ksh93 prints an empty string there, as it doesn't consider the
double quotes to be nested, so the "}" is unquoted. Ash produces a
syntax error, like dash before this patch. All other shells print a }.
So dash with this patch behaves like the majority.


> This fixes more cases than just backslashes and single quotes:
> another character that's special in unquoted contexts is ~, so
> "${HOME#~}" should expand to an empty string.

Yes. That was a bug and this patch fixes it.


> This changes how $(( ${$+"123"} )) gets handled: POSIX doesn't really
> answer this, I think. POSIX says that $(( "123" )) is a syntax error,
> but doesn't address whether " is special when it appears in other
> places than directly in the $((...)). Most shells accept $((
> ${$+"123"} )), and with this patch, dash accepts it too.

I've no strong feelings about this. The change doesn't seem like it
could be harmful.


> This changes how "${x+"$y"}" get handled: POSIX is silent about
> whether the $y should be treated as quoted. dash has treated it as
> quoted for a very long time. ash has historically treated it as
> unquoted. With this patch, it gets treated as unquoted.

That seems inconsistent with how it handles "${$+"}"}", in which the "}"
is treated as quoted (see above).

ksh93 is the only existing shell that treats the $y as unquoted, so I
think it would be better if dash continued to treat the $y as quoted.

Even if not, the inconsistency should be fixed.


> Since 7cfd8be0dc83342b4a71f3a8e5b7efab4670e50c had also changed how
> "$@" got handled and reverting that changed it, I looked into how
> this works and fixed another bug. It also changes the handling of $*
> and $@ when IFS is set but empty: dash 0.5.8 didn't handle empty IFS
> properly at all, even if all parameters were non-empty. dash 0.5.9.1
> preserves empty parameters. With this patch, they get removed just
> like in bash. POSIX allows for either.
I don't think it does. POSIX implies that empty $@ and $* generates zero
fields. 2.5.2 Special Parameters:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_05_02
| @
|    Expands to the positional parameters, starting from one, initially
|    producing one field for each positional parameter that is set.

Since there are no positional parameters, no fields should initially be
produced at all. Same for $*.

So I do believe your patch correctly fixes a bug here.


> I would be a bit surprised if the patch is acceptable in its current
> form, but it's worth seeing which of the current results are definitely
> correct, which of the results are acceptable, which results may well be
> unwanted, and which special cases I missed.

According to my tests (i.e. the modernish test suite), nearly everything
is POSIXly correct. There's only one parameter expansion problem left
that I can find, and it existed before this patch as well:

$ src/dash -c 'printf "%s\n" "${$+\}}"'
\}

Expected output: }  (no backslash), as in bash 4, yash, ksh93, pdksh,
mksh, zsh. In other words: it should be possible to escape a '}' with a
backslash within the parameter expansion, even if the expansion is quoted.

POSIX ref.: 2.6.2 Parameter Expansion
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
| Any '}' escaped by a <backslash> or within a quoted string, and
| characters in embedded arithmetic expansions, command substitutions,
| and variable expansions, shall not be examined in determining the
| matching '}'.


Thanks,

- M.
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html