dash.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* dash bug: double-quoted "\" breaks glob protection for next char
@ 2018-02-13 13:53 Denys Vlasenko
  2018-02-13 22:45 ` Martijn Dekker
  2018-02-14 20:03 ` Harald van Dijk
  0 siblings, 2 replies; 5+ messages in thread
From: Denys Vlasenko @ 2018-02-13 13:53 UTC (permalink / raw)
  To: Herbert Xu, dash

$ >'\zzzz'
$ >'\wwww'
$ dash -c 'echo "\*"'
\wwww \zzzz

The cause: uses "\\*" pattern instead of "\\\*".
The fix:

                        /* backslash */
                        case CBACK:
                                c = pgetc2();
                                if (c == PEOF) {
                                        USTPUTC(CTLESC, out);
                                        USTPUTC('\\', out);
                                        pungetc();
                                } else if (c == '\n') {
                                        nlprompt();
                                } else {
                                        if (
                                                dblquote &&
                                                c != '\\' && c != '`' &&
                                                c != '$' && (
                                                        c != '"' ||
                                                        eofmark != NULL
                                                )
                                        ) {
USTPUTC(CTLESC, out); // add this line
                                                USTPUTC('\\', out);
                                        }
                                        USTPUTC(CTLESC, out);
                                        USTPUTC(c, out);
                                        quotef++;
                                }

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: dash bug: double-quoted "\" breaks glob protection for next char
  2018-02-13 13:53 dash bug: double-quoted "\" breaks glob protection for next char Denys Vlasenko
@ 2018-02-13 22:45 ` Martijn Dekker
  2018-02-14 20:03 ` Harald van Dijk
  1 sibling, 0 replies; 5+ messages in thread
From: Martijn Dekker @ 2018-02-13 22:45 UTC (permalink / raw)
  To: Denys Vlasenko, Herbert Xu, dash

Op 13-02-18 om 14:53 schreef Denys Vlasenko:
> $ >'\zzzz'
> $ >'\wwww'
> $ dash -c 'echo "\*"'
> \wwww \zzzz
> 
> The cause: uses "\\*" pattern instead of "\\\*".

Also:

$ dash -c 'case \\ab in "\*") echo BUG;; esac'
BUG
$ dash -c 'case \\a in "\?") echo BUG;; esac'
BUG

Yup. Full globbing within double quotes after a backslash.

- M.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: dash bug: double-quoted "\" breaks glob protection for next char
  2018-02-13 13:53 dash bug: double-quoted "\" breaks glob protection for next char Denys Vlasenko
  2018-02-13 22:45 ` Martijn Dekker
@ 2018-02-14 20:03 ` Harald van Dijk
  2018-02-14 21:44   ` Harald van Dijk
  1 sibling, 1 reply; 5+ messages in thread
From: Harald van Dijk @ 2018-02-14 20:03 UTC (permalink / raw)
  To: Denys Vlasenko, Herbert Xu, dash

On 13/02/2018 14:53, Denys Vlasenko wrote:
> $ >'\zzzz'
> $ >'\wwww'
> $ dash -c 'echo "\*"'
> \wwww \zzzz

Nice catch.

> The cause: uses "\\*" pattern instead of "\\\*".
> The fix:
> 
>                          /* backslash */
>                          case CBACK:
>                                  c = pgetc2();
>                                  if (c == PEOF) {
>                                          USTPUTC(CTLESC, out);
>                                          USTPUTC('\\', out);
>                                          pungetc();
>                                  } else if (c == '\n') {
>                                          nlprompt();
>                                  } else {
>                                          if (
>                                                  dblquote &&
>                                                  c != '\\' && c != '`' &&
>                                                  c != '$' && (
>                                                          c != '"' ||
>                                                          eofmark != NULL
>                                                  )
>                                          ) {
> USTPUTC(CTLESC, out); // add this line
>                                                  USTPUTC('\\', out);
>                                          }
>                                          USTPUTC(CTLESC, out);
>                                          USTPUTC(c, out);
>                                          quotef++;
>                                  }

I don't think this is right. The bug was introduced in

 
<https://git.kernel.org/pub/scm/utils/dash/dash.git/commit/?id=7cfd8be0dc83342b4a71f3a8e5b7efab4670e50c>

Prior to that, the line USTPUTC(CTLESC, out); was there. The commit 
message is saying that the logic for detecting whether \ should be taken 
literally doesn't belong in the parser, that the parser will get it 
wrong. The example in the commit message doesn't break with your patch, 
but short modifications to that example do make it fail:

Currently:

$ dash -c 'foo=a; echo "<${foo#[a\]]}>"'
<>

This is what I expect, and also what bash, ksh and posh do.

With your patch:

$ dash -c 'foo=a; echo "<${foo#[a\]]}>"'
<a>

Cheers,
Harald van Dijk

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: dash bug: double-quoted "\" breaks glob protection for next char
  2018-02-14 20:03 ` Harald van Dijk
@ 2018-02-14 21:44   ` Harald van Dijk
  2018-02-14 22:50     ` Harald van Dijk
  0 siblings, 1 reply; 5+ messages in thread
From: Harald van Dijk @ 2018-02-14 21:44 UTC (permalink / raw)
  To: Denys Vlasenko, Herbert Xu, dash

[-- Attachment #1: Type: text/plain, Size: 609 bytes --]

On 2/14/18 9:03 PM, Harald van Dijk wrote:
> On 13/02/2018 14:53, Denys Vlasenko wrote:
>> $ >'\zzzz'
>> $ >'\wwww'
>> $ dash -c 'echo "\*"'
>> \wwww \zzzz
> 
>[...]
> 
> Currently:
> 
> $ dash -c 'foo=a; echo "<${foo#[a\]]}>"'
> <>
> 
> This is what I expect, and also what bash, ksh and posh do.
> 
> With your patch:
> 
> $ dash -c 'foo=a; echo "<${foo#[a\]]}>"'
> <a>

Does the attached look right as an alternative? It treats a quoted 
backslash the same way as if it were preceded by CTLESC in _rmescapes. 
It passes your test case and mine, but I'll do more extensive testing.

Cheers,
Harald van Dijk

[-- Attachment #2: dash-escape-backslash.patch --]
[-- Type: text/x-patch, Size: 585 bytes --]

diff --git a/src/expand.c b/src/expand.c
index 2a50830..af88a69 100644
--- a/src/expand.c
+++ b/src/expand.c
@@ -1686,12 +1686,17 @@ _rmescapes(char *str, int flag)
 		}
 		if (*p == (char)CTLESC) {
 			p++;
-			if (notescaped)
-				*q++ = '\\';
-		} else if (*p == '\\' && !inquotes) {
-			/* naked back slash */
-			notescaped = 0;
-			goto copy;
+			goto escape;
+		} else if (*p == '\\') {
+			if (inquotes) {
+escape:
+				if (notescaped)
+					*q++ = '\\';
+			} else {
+				/* naked back slash */
+				notescaped = 0;
+				goto copy;
+			}
 		}
 		notescaped = globbing;
 copy:

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: dash bug: double-quoted "\" breaks glob protection for next char
  2018-02-14 21:44   ` Harald van Dijk
@ 2018-02-14 22:50     ` Harald van Dijk
  0 siblings, 0 replies; 5+ messages in thread
From: Harald van Dijk @ 2018-02-14 22:50 UTC (permalink / raw)
  To: Denys Vlasenko, Herbert Xu, dash

On 2/14/18 10:44 PM, Harald van Dijk wrote:
> On 2/14/18 9:03 PM, Harald van Dijk wrote:
>> On 13/02/2018 14:53, Denys Vlasenko wrote:
>>> $ >'\zzzz'
>>> $ >'\wwww'
>>> $ dash -c 'echo "\*"'
>>> \wwww \zzzz
>>
>> [...]
>>
>> Currently:
>>
>> $ dash -c 'foo=a; echo "<${foo#[a\]]}>"'
>> <>
>>
>> This is what I expect, and also what bash, ksh and posh do.
>>
>> With your patch:
>>
>> $ dash -c 'foo=a; echo "<${foo#[a\]]}>"'
>> <a>
> 
> Does the attached look right as an alternative? It treats a quoted 
> backslash the same way as if it were preceded by CTLESC in _rmescapes. 
> It passes your test case and mine, but I'll do more extensive testing.

It causes preglob's string to potentially grow larger than the original. 
When called with RMESCAPE_ALLOC, that can be handled by increasing the 
buffer size, but preglob also gets called without RMESCAPE_ALLOC to 
modify a string in-place. That's never going to work with this approach. 
Back to the drawing board...

Cheers,
Harald van Dijk

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-02-14 22:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-13 13:53 dash bug: double-quoted "\" breaks glob protection for next char Denys Vlasenko
2018-02-13 22:45 ` Martijn Dekker
2018-02-14 20:03 ` Harald van Dijk
2018-02-14 21:44   ` Harald van Dijk
2018-02-14 22:50     ` Harald van Dijk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).