'.' causes wildcard expansion
PATCH 1/1 '.' causes wildcard expansion
Wildcard expansion in device name activates my floppy drive (PC BIOS). I noticed that
echo .
also did so in 1.99, though there should be no expansion. Current source fixes that case but
echo (.
still runs the floppy. split_path shouldn't treat . as a wildcard.
I believe the regcomp call in make_regex is incorrect because the constant used is intended only for a lower-level interface. The defines are more than a little confusing.
=== modified file 'ChangeLog'
ChangeLog 2012-06-11 06:32:58 +0000 renamed from ChangeLog 2012-06-09 17:58:38 +0000 +5 -0
@@ -1,3 +1,8 @@ +2012-06-11 Bob Mabee <[email protected]> + + * commands/wildcard.c (split_path): . is not a wildcard. + * (make_regex): RE_SYNTAX_GNU_AWK is not valid for regcomp cflags. + 2012-06-09 Vladimir Serbinenko <[email protected]> * tests/grub_script_expansion.in: Explicitly tell grep that we handle
=== modified file 'grub-core/commands/wildcard.c'
grub-core/commands/wildcard.c 2012-06-10 23:16:02 +0000 renamed from grub-core/commands/wildcard.c 2012-06-08 20:54:21 +0000 +2 -2
@@ -153,7 +153,7 @@ buffer[i] = '\0'; grub_dprintf ("expand", "Regexp is %s\n", buffer); - if (regcomp (regexp, buffer, RE_SYNTAX_GNU_AWK)) + if (regcomp (regexp, buffer, REG_EXTENDED)) { grub_free (buffer); return 1;
@@ -181,7 +181,7 @@ if (ch == '\\' && end[1]) end++; - else if (isregexop (ch)) + else if (ch == '*') /* only wildcard currently implemented */ regex = 1; else if (ch == '/' && ! regex)
Having second thoughts. Where I suggested taking out this call
grub-core/commands/wildcard.c 2012-06-10 23:16:02 +0000 line 184
Quoted 2 lines
grub-core/commands/wildcard.c 2012-06-10 23:16:02 +0000 renamed from grub-core/commands/wildcard.c 2012-06-08 20:54:21 +0000 +1
-1- else if (isregexop (ch)) + else if (ch == '*') /* only wildcard currently implemented */
I should have just taken the extraneous '.' out of isregexop. That would make it slightly simpler to add the remaining shell wildcards ('?' and '[') when desired.
'[' already works, if there's also a * to trigger expansion, with the small bug that special chars within [] still get escaped or translated so the set may include an unexpected '.' or '\'.
Other regex chars also work, which probably should be counted as a bug. I assume the eventual goal is to behave just like a common shell.
Quoted 5 lines · Robert Mabee
Having second thoughts. Where I suggested taking out this call
Review
Quoted 2 lines
(quoted diff) +1
-1- else if (isregexop (ch)) + else if (ch == '*') /* only wildcard currently implemented */I should have just taken the extraneous '.' out of isregexop.
It's used in other contexts as "needing escaping". So no, this part is fine
Quoted 5 lines
That would
make it slightly simpler to add the remaining shell wildcards ('?' and '[') when desired.'[' already works,
Not properly: try a[b*
Quoted 6 lines
if there's also a * to trigger expansion, with the small
bug that special chars within [] still get escaped or translated so the set may include an unexpected '.' or '\'.Other regex chars also work, which probably should be counted as a bug. I assume the eventual goal is to behave just like a common shell.
Please try attached patch
Signature · 2 lines
Regards Vladimir 'φ-coder/phcoder' Serbinenko
=== modified file 'grub-core/commands/wildcard.c'
commands/wildcard.c +15 -3
@@ -80,7 +80,7 @@ merge (char **dest, char **ps)static inline int isregexop (char ch) { -return grub_strchr ("*.\\" , ch) ?1 :0 ; +return grub_strchr ("*.\\|+{}[]" , ch) ?1 :0 ; } static char *
@@ -128,14 +128,22 @@ make_regex (const char *start, const cha {case '\\' : buffer[i++] = ch; -if (*start !='\0' ) +if (*start =='*' || *start =='?' ) buffer[i++] = *start++; +else + buffer[i++] ='\\' ;break ;case '.' :case '(' :case ')' :case '@' : +case '|' : +case '+' : +case '{' : +case '}' : +case '[' : +case ']' : buffer[i++] ='\\' ; buffer[i++] = ch;break ;
@@ -145,6 +153,10 @@ make_regex (const char *start, const cha buffer[i++] ='*' ;break ; +case '?' : + buffer[i++] ='.' ; +break ; +default : buffer[i++] = ch; }
@@ -181,7 +193,7 @@ split_path (const char *str, const charif (ch =='\\' && end[1 ]) end++; -else if (isregexop (ch)) +else if (ch =='*' || ch =='?' ) regex =1 ;else if (ch =='/' && ! regex)
Quoted 4 lines · Robert Mabee
Having second thoughts. Where I suggested taking out this call
grub-core/commands/wildcard.c 2012-06-10 23:16:02 +0000 line 184
Quoted 2 lines
grub-core/commands/wildcard.c 2012-06-10 23:16:02 +0000 renamed from grub-core/commands/wildcard.c 2012-06-08 20:54:21 +0000 +1
-1- else if (isregexop (ch)) + else if (ch == '*') /* only wildcard currently implemented */
Quoted 8 lines
I should have just taken the extraneous '.' out of isregexop. That would make it slightly simpler to add the remaining shell wildcards ('?' and '[') when desired.
'[' already works, if there's also a * to trigger expansion, with the small bug that special chars within [] still get escaped or translated so the set may include an unexpected '.' or '\'.
I've fixed these problems AFAICT now. Can you confirm?
Quoted 9 lines
Other regex chars also work, which probably should be counted as a bug. I assume the eventual goal is to behave just like a common shell.
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Signature · 2 lines
Regards Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 1 line · Vladimir 'φ-coder/phcoder' Serbinenko
I've fixed these problems AFAICT now. Can you confirm?
I agree that many extraneous regex chars are now properly quoted so no more functionality is exposed than is intended, now * and ?. I suggest also quoting ^ and $ just for completeness. I can not explain why @ is quoted; it's not mentioned in the documentation for POSIX-compatible regex or in gnulib/regcomp.c.
wildcard_escape in script/execute.c quotes the wild chars and [ , which is no longer wild. This can lead to a surprising extra level of quoting, so for example *'['* becomes the useless regexp pattern ^.*\\\[.*$
It appears the only remaining use of isregexop is in make_dir, which removes \
quoting only before regex chars, from path components that weren't wild. If this quoting was inserted by wildcard_escape then it should be removed similarly to wildcard_unescape, which doesn't examine the quoted char.