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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/4/18 4:26 PM, Martijn Dekker wrote:
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.

FreeBSD sh also prints a blank line here.

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.

Like above, FreeBSD sh behaves like ksh.

Yes, the inconsistency should be fixed. Either it should be treated as quoted or as unquoted, but not quoted-unless-it-comes-from-a-variable. I have no strong feelings on which it should be.

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.

By empty parameters, I meant parameters that had been set to an empty string. It's covered by the 'set -- a ""' in my tests. You're right that after 'set --', unquoted $@ should not produce any fields. I hadn't even noticed that dash got that wrong and that my patch had fixed it.

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 '}'.

I believe this actually requires dash's behaviour. This says the first } isn't examined in determining the matching '}', but only that: it just says the parameter expansion expression is $+\}. It doesn't say the backslash is removed. Then, \} is taken as part of a double-quoted string, and inside double-quoted strings, a backslash that isn't followed by $, `, ", \ or a newline is taken as a literal backslash. I agree that it would be much better to print } here though.

Thanks for the testing! I'd noticed I had an off-by-one error that causes problems when an expansion ends in another expansion (${x+${x+}}). I'll try to improve it to also handle the issues you've pointed out.

Cheers,
Harald van Dijk
--
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



[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux