* Bug: [ "(" = ")" ] is true
@ 2015-12-09 15:34 Martijn Dekker
2015-12-09 15:58 ` Stephane Chazelas
2015-12-09 16:24 ` Peter Stephenson
0 siblings, 2 replies; 5+ messages in thread
From: Martijn Dekker @ 2015-12-09 15:34 UTC (permalink / raw)
To: zsh-workers
There is a string comparison bug with `[' and `test'; the result is true
if the first string starts with '(' and the second string starts with ')'.
$ [ "(" = ")" ] && echo oops || echo ok
oops
$ [ ")" = "(" ] && echo oops || echo ok
ok
$ [ "((" = "))" ] && echo oops || echo ok
oops
$ [ "((" = ")x" ] && echo oops || echo ok
oops
$ [ "(x" = ")" ] && echo oops || echo ok
oops
$ [ "x(" = ")" ] && echo oops || echo ok
ok
$ [ "(" = "x)" ] && echo oops || echo ok
ok
This appears to be a long-standing bug. I confirmed it in:
zsh 4.3.6
zsh 5.1.1
zsh 5.2
I also found that the bug does *not* exist in zsh 4.1.1.
Also, `[[' does not have this problem.
Credit to /u/tilkau on reddit for discovering this bug:
https://www.reddit.com/r/bash/comments/3w1hm4/how_to_compare_values_to_literal/cxsknzh
Thanks,
- Martijn
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Bug: [ "(" = ")" ] is true
2015-12-09 15:34 Bug: [ "(" = ")" ] is true Martijn Dekker
@ 2015-12-09 15:58 ` Stephane Chazelas
2015-12-09 16:31 ` Martijn Dekker
2015-12-09 16:24 ` Peter Stephenson
1 sibling, 1 reply; 5+ messages in thread
From: Stephane Chazelas @ 2015-12-09 15:58 UTC (permalink / raw)
To: Martijn Dekker; +Cc: zsh-workers
2015-12-09 16:34:34 +0100, Martijn Dekker:
> There is a string comparison bug with `[' and `test'; the result is true
> if the first string starts with '(' and the second string starts with ')'.
>
> $ [ "(" = ")" ] && echo oops || echo ok
> oops
> $ [ ")" = "(" ] && echo oops || echo ok
> ok
> $ [ "((" = "))" ] && echo oops || echo ok
> oops
> $ [ "((" = ")x" ] && echo oops || echo ok
> oops
> $ [ "(x" = ")" ] && echo oops || echo ok
> oops
> $ [ "x(" = ")" ] && echo oops || echo ok
> ok
> $ [ "(" = "x)" ] && echo oops || echo ok
> ok
>
> This appears to be a long-standing bug. I confirmed it in:
>
> zsh 4.3.6
> zsh 5.1.1
> zsh 5.2
>
> I also found that the bug does *not* exist in zsh 4.1.1.
[...]
http://www.zsh.org/mla/users/2007/msg01223.html seems to be to
blame.
If the first argument starts with a "(" and the last one starts in
a ")", they are both removed.
So all operators are affected not just "=" ([ "(file1)" -nt
")file2(" ]...)
--
Stephane
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Bug: [ "(" = ")" ] is true
2015-12-09 15:34 Bug: [ "(" = ")" ] is true Martijn Dekker
2015-12-09 15:58 ` Stephane Chazelas
@ 2015-12-09 16:24 ` Peter Stephenson
1 sibling, 0 replies; 5+ messages in thread
From: Peter Stephenson @ 2015-12-09 16:24 UTC (permalink / raw)
To: zsh-workers
On Wed, 9 Dec 2015 16:34:34 +0100
Martijn Dekker <martijn@inlv.org> wrote:
> There is a string comparison bug with `[' and `test'; the result is true
> if the first string starts with '(' and the second string starts with ')'.
>
> $ [ "(" = ")" ] && echo oops || echo ok
> oops
This is yet another result of a folorn attempt to handle extensions of
the "[" syntax consistently. We can't tell if "(" ... ")" are for group
or are arguments to a command, so we guess. However, as groups are an
extension of the most basic syntax, we should first try the latter.
Note that
[ "(" string ")" ]
and
[ "(" "" ")" ]
are valid tests that return true and false respectively. (They tend to
suggest that whoever wrote those tests has lost the plot, but, well.)
> $ [ "((" = "))" ] && echo oops || echo ok
> oops
This seems to be a more obvious bug that we only care about the first
character. I can't see how that could ever possibly be right. It's
possible there may have been some confusion over the fact that if we
were parsing this properly for "[[" then the "(" and ")" wouldn't
need to be in separate words, but that doesn't apply to "test" / "[".
Added some "const"s in text.c with a little trepidation --- but I think
it's a long time since we had problems with older OSes in that department.
diff --git a/Src/builtin.c b/Src/builtin.c
index cac4f42..b06bc6d 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -6463,7 +6463,13 @@ bin_test(char *name, char **argv, UNUSED(Options ops), int func)
nargs = arrlen(argv);
if (nargs == 3 || nargs == 4)
{
- if (*argv[0] == '(' && *argv[nargs-1] == ')') {
+ /*
+ * As parentheses are an extension, we need to be careful ---
+ * if this is a three-argument expression that could
+ * be a binary operator, prefer that.
+ */
+ if (!strcmp(argv[0], "(") && !strcmp(argv[nargs-1],")") &&
+ (nargs != 3 || !is_cond_binary_op(argv[1]))) {
argv[nargs-1] = NULL;
argv++;
}
diff --git a/Src/text.c b/Src/text.c
index 9421d70..04acd2a 100644
--- a/Src/text.c
+++ b/Src/text.c
@@ -40,9 +40,32 @@
/**/
int text_expand_tabs;
+/*
+ * Binary operators in conditions.
+ * There order is tied to the order of the definitions COND_STREQ
+ * et seq. in zsh.h.
+ */
+static const char *cond_binary_ops[] = {
+ "=", "!=", "<", ">", "-nt", "-ot", "-ef", "-eq",
+ "-ne", "-lt", "-gt", "-le", "-ge", "=~"
+};
+
static char *tptr, *tbuf, *tlim, *tpending;
static int tsiz, tindent, tnewlins, tjob;
+/**/
+int
+is_cond_binary_op(const char *str)
+{
+ const char **op;
+ for (op = cond_binary_ops; *op; op++)
+ {
+ if (!strcmp(str, *op))
+ return 1;
+ }
+ return 0;
+}
+
static void
dec_tindent(void)
{
@@ -120,7 +143,7 @@ taddchr(int c)
/**/
static void
-taddstr(char *s)
+taddstr(const char *s)
{
int sl = strlen(s);
char c;
@@ -822,11 +845,6 @@ gettext2(Estate state)
break;
case WC_COND:
{
- static char *c1[] = {
- "=", "!=", "<", ">", "-nt", "-ot", "-ef", "-eq",
- "-ne", "-lt", "-gt", "-le", "-ge", "=~"
- };
-
int ctype;
if (!s) {
@@ -912,7 +930,7 @@ gettext2(Estate state)
/* Binary test: `a = b' etc. */
taddstr(ecgetstr(state, EC_NODUP, NULL));
taddstr(" ");
- taddstr(c1[ctype - COND_STREQ]);
+ taddstr(cond_binary_ops[ctype - COND_STREQ]);
taddstr(" ");
taddstr(ecgetstr(state, EC_NODUP, NULL));
if (ctype == COND_STREQ ||
diff --git a/Test/C02cond.ztst b/Test/C02cond.ztst
index 40bbf42..9e13696 100644
--- a/Test/C02cond.ztst
+++ b/Test/C02cond.ztst
@@ -389,6 +389,18 @@ F:Failures in these cases do not indicate a problem in the shell.
>Not zero 5
>Not zero 6
+ [ '(' = ')' ] || print OK 1
+ [ '((' = '))' ] || print OK 2
+ [ '(' = '(' ] && print OK 3
+ [ '(' non-empty-string ')' ] && echo OK 4
+ [ '(' '' ')' ] || echo OK 5
+0:yet more old-fashioned test fix ups: prefer comparison to parentheses
+>OK 1
+>OK 2
+>OK 3
+>OK 4
+>OK 5
+
%clean
# This works around a bug in rm -f in some versions of Cygwin
chmod 644 unmodish
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Bug: [ "(" = ")" ] is true
2015-12-09 15:58 ` Stephane Chazelas
@ 2015-12-09 16:31 ` Martijn Dekker
2015-12-09 20:27 ` Bart Schaefer
0 siblings, 1 reply; 5+ messages in thread
From: Martijn Dekker @ 2015-12-09 16:31 UTC (permalink / raw)
To: zsh-workers
Stephane Chazelas schreef op 09-12-15 om 16:58:
> http://www.zsh.org/mla/users/2007/msg01223.html seems to be to
> blame.
>
> If the first argument starts with a "(" and the last one starts in
> a ")", they are both removed.
>
> So all operators are affected not just "=" ([ "(file1)" -nt
> ")file2(" ]...)
Wow.
Not only that, the results are incorrect even after the removal.
$ test '(1' -eq ')2' && echo oops || echo ok
oops
$ test '1' -eq '2' && echo oops || echo ok
ok
- M.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Bug: [ "(" = ")" ] is true
2015-12-09 16:31 ` Martijn Dekker
@ 2015-12-09 20:27 ` Bart Schaefer
0 siblings, 0 replies; 5+ messages in thread
From: Bart Schaefer @ 2015-12-09 20:27 UTC (permalink / raw)
To: zsh-workers
On Dec 9, 5:31pm, Martijn Dekker wrote:
}
} Wow.
}
} Not only that, the results are incorrect even after the removal.
}
} $ test '(1' -eq ')2' && echo oops || echo ok
} oops
That's because the parens weren't removed, the entire argument that
contains each paren was removed. [ \(1 -eq \)2 ] was becoming [ -eq ].
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-12-09 20:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-09 15:34 Bug: [ "(" = ")" ] is true Martijn Dekker
2015-12-09 15:58 ` Stephane Chazelas
2015-12-09 16:31 ` Martijn Dekker
2015-12-09 20:27 ` Bart Schaefer
2015-12-09 16:24 ` Peter Stephenson
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
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).