[PATCH] '.' causes wildcard expansion

[PATCH] '.' causes wildcard expansion

[Robert Mabee]

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'
--- old/ChangeLog    2012-06-09 17:58:38 +0000
+++ new/ChangeLog    2012-06-11 06:32:58 +0000
@@ -1,3 +1,8 @@
+2012-06-11  Bob Mabee <rmabee@comcast.net>
+
+    * 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 <phcoder@gmail.com>

      * tests/grub_script_expansion.in: Explicitly tell grep that we handle

=== modified file 'grub-core/commands/wildcard.c'
--- old/grub-core/commands/wildcard.c    2012-06-08 20:54:21 +0000
+++ new/grub-core/commands/wildcard.c    2012-06-10 23:16:02 +0000
@@ -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)

Re: [PATCH] '.' causes wildcard expansion

[Robert Mabee]

Having second thoughts.  Where I suggested taking out this call

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

Re: [PATCH] '.' causes wildcard expansion

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1.1: Type: text/plain, Size: 952 bytes --]

On 14.06.2012 00:42, Robert Mabee wrote:

> Having second thoughts.  Where I suggested taking out this call
> 
>> -      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

>  That would
> make it slightly simpler to add the remaining shell wildcards ('?' and '[')
> when desired.
> 
> '[' already works,

Not properly: try a[b*

> 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
-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: regex.diff --]
[-- Type: text/x-diff; name="regex.diff", Size: 1267 bytes --]

=== modified file 'grub-core/commands/wildcard.c'
--- grub-core/commands/wildcard.c	2012-06-08 20:54:21 +0000
+++ grub-core/commands/wildcard.c	2012-06-18 11:01:31 +0000
@@ -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 char
       if (ch == '\\' && end[1])
 	end++;
 
-      else if (isregexop (ch))
+      else if (ch == '*' || ch == '?')
 	regex = 1;
 
       else if (ch == '/' && ! regex)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [PATCH] '.' causes wildcard expansion

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 14.06.2012 00:42, Robert Mabee wrote:

> Having second thoughts.  Where I suggested taking out this call
> 
>> -      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 '\'.
> 

I've fixed these problems AFAICT now. Can you confirm?

> 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
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [PATCH] '.' causes wildcard expansion

[Robert Mabee]

On 06/19/2012 08:19 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> 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.