[EXPAND] Nested parameter expansion results in an empty string when quoted

[EXPAND] Nested parameter expansion results in an empty string when quoted

[Todor Vlaev]

Hi,

While playing around with parameter expansion I noticed that the
following didn't work in dash  (dash 0.5.5.1-7.4ubuntu1) as compared
to bash even though I believe it should be POSIX-compliant:

my_str=swan; last_char="${my_str#${my_str%?}}"; echo ${last_char}

If the double quotes are removed, the last character is printed correctly.

At a quick glance through the commits after the 0.5.5.1 release I saw
the following bug fix. Could it be related?

0d7d66039b614b642c775432fd64aa8c11f9a64d
[EXPAND] Fix quoted pattern patch breakage

Thanks,
Todor

Re: [EXPAND] Nested parameter expansion results in an empty string when quoted

[Jilles Tjoelker]

On Tue, Aug 28, 2012 at 04:27:24PM +0300, Todor Vlaev wrote:
> While playing around with parameter expansion I noticed that the
> following didn't work in dash  (dash 0.5.5.1-7.4ubuntu1) as compared
> to bash even though I believe it should be POSIX-compliant:

> my_str=swan; last_char="${my_str#${my_str%?}}"; echo ${last_char}

> If the double quotes are removed, the last character is printed
> correctly.

The double-quotes here are unnecessary and should be left out for
optimal compatibility. Note that this may require a temporary variable
if the expansion is in a context where word splitting is performed.
However, you are right that the double-quotes are permitted per POSIX.

On the other hand, some double-quotes that are really necessary are
missing:

my_str=swan; last_char=${my_str#"${my_str%?}"}; echo "${last_char}"

The inner double-quotes force any wildcard characters in $my_str to be
taken literally. This is required to work correctly with the string
sw*n, for example.

It also happens to work around dash's bugs, both the above version and a
version with the redundant quotes you like:

my_str=swan; last_char="${my_str#"${my_str%?}"}"; echo "${last_char}"

> At a quick glance through the commits after the 0.5.5.1 release I saw
> the following bug fix. Could it be related?

> 0d7d66039b614b642c775432fd64aa8c11f9a64d
> [EXPAND] Fix quoted pattern patch breakage

It is still broken with master (46abc8c6d8a5e9a5712bdc1312c0b6960eec65a4
[BUILTIN] Add support for ulimit -r 2012-07-03) here.

Fixing this requires a test suite. Without a test suite, any attempt to
fix something is likely to break something else. This is not only
because the expansions are inherently complicated but also because
dash's code for them is hard to understand.

The code for expansions in FreeBSD's sh, a related Almquist shell
variant heavily modified by me, is more correct and slightly easier to
understand. There is also a test suite. However, the dash maintainer
does not agree with some of the choices I have made where POSIX (as
modified by interpretations) is silent.

The mksh, a shell from a different lineage, also has a good set of test
cases.

-- 
Jilles Tjoelker

Re: [EXPAND] Nested parameter expansion results in an empty string when quoted

[Todor Vlaev]

> The double-quotes here are unnecessary and should be left out for
> optimal compatibility. Note that this may require a temporary variable
> if the expansion is in a context where word splitting is performed.
> However, you are right that the double-quotes are permitted per POSIX.
>
> On the other hand, some double-quotes that are really necessary are
> missing:
>
> my_str=swan; last_char=${my_str#"${my_str%?}"}; echo "${last_char}"
>
> The inner double-quotes force any wildcard characters in $my_str to be
> taken literally. This is required to work correctly with the string
> sw*n, for example.
>
> It also happens to work around dash's bugs, both the above version and a
> version with the redundant quotes you like:
>
> my_str=swan; last_char="${my_str#"${my_str%?}"}"; echo "${last_char}"

Thanks for the useful tips!

> Fixing this requires a test suite. Without a test suite, any attempt to
> fix something is likely to break something else.

A bit worrying not to have regression tests, indeed.

In any case, there are plenty of workarounds to this minor issue, even
if not fixed there is a reference now for someone else who might
wonder.

Br,
Todor

Re: [EXPAND] Nested parameter expansion results in an empty string when quoted

[Jonathan Nieder]

Todor Vlaev wrote:

> A bit worrying not to have regression tests, indeed.

Yes, I'd also like functional tests in the dash.git repo.

In the meantime, I think Herbert runs tests that are not in that repo,
and others reviewing patches use other test suites if I understand
correctly.

Jonathan

Re: [EXPAND] Nested parameter expansion results in an empty string when quoted

[Herbert Xu]

On Tue, Aug 28, 2012 at 01:27:24PM +0000, Todor Vlaev wrote:
> 
> While playing around with parameter expansion I noticed that the
> following didn't work in dash  (dash 0.5.5.1-7.4ubuntu1) as compared
> to bash even though I believe it should be POSIX-compliant:
> 
> my_str=swan; last_char="${my_str#${my_str%?}}"; echo ${last_char}
> 
> If the double quotes are removed, the last character is printed correctly.
> 
> At a quick glance through the commits after the 0.5.5.1 release I saw
> the following bug fix. Could it be related?
> 
> 0d7d66039b614b642c775432fd64aa8c11f9a64d
> [EXPAND] Fix quoted pattern patch breakage

This patch should fix the problem.

commit a7c21a6f4cb42d967854cae954efd4ee66bdea9c
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Fri Aug 23 20:04:12 2013 +1000

    [EXPAND] Propagate EXP_QPAT in subevalvar
    
    On Tue, Aug 28, 2012 at 01:27:24PM +0000, Todor Vlaev wrote:
    >
    > While playing around with parameter expansion I noticed that the
    > following didn't work in dash  (dash 0.5.5.1-7.4ubuntu1) as compared
    > to bash even though I believe it should be POSIX-compliant:
    >
    > my_str=swan; last_char="${my_str#${my_str%?}}"; echo ${last_char}
    >
    > If the double quotes are removed, the last character is printed correctly.
    >
    > At a quick glance through the commits after the 0.5.5.1 release I saw
    > the following bug fix. Could it be related?
    >
    > 0d7d66039b614b642c775432fd64aa8c11f9a64d
    > [EXPAND] Fix quoted pattern patch breakage
    
    We need to propagate EXP_QPAT in subevalvar as otherwise a nested
    parameter expansion within subevalvar may be expanded incorrectly.
    
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/ChangeLog b/ChangeLog
index 7354832..911d31e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2013-08-23  Herbert Xu <herbert@gondor.apana.org.au>
+
+	* Propagate EXP_QPAT in subevalvar.
+
 2012-07-20  Kimo Rosenbaum <kimor79@yahoo.com>
 
 	* Fix typo for wait in manual.
diff --git a/src/expand.c b/src/expand.c
index ce60fe9..11fd7b7 100644
--- a/src/expand.c
+++ b/src/expand.c
@@ -650,7 +650,8 @@ subevalvar(char *p, char *str, int strloc, int subtype, int startloc, int varfla
 	char *(*scan)(char *, char *, char *, char *, int , int);
 
 	argstr(p, EXP_TILDE | (subtype != VSASSIGN && subtype != VSQUESTION ?
-			       (flag & EXP_QUOTED ? EXP_QPAT : EXP_CASE) : 0));
+			       (flag & (EXP_QUOTED | EXP_QPAT) ?
+			        EXP_QPAT : EXP_CASE) : 0));
 	STPUTC('\0', expdest);
 	argbackq = saveargbackq;
 	startp = stackblock() + startloc;

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt