fix: improve KeyMatch method to avoid endless loop

php-casbin · May 19, 2020 · e2ce186 · e2ce186
1 parent 970ba59
commit e2ce186
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 29 deletions.
diff --git a/src/Util/BuiltinOperations.php b/src/Util/BuiltinOperations.php
@@ -63,21 +63,17 @@ public static function keyMatch2(string $key1, string $key2): bool
     {
         $key2 = str_replace(['/*'], ['/.*'], $key2);
 
-        $pattern = '/(.*):[^\/]+(.*)/';
-        for (; ;) {
-            if (false === strpos($key2, '/:')) {
-                break;
-            }
-            $key2 = preg_replace_callback(
-                $pattern,
-                function ($m) {
-                    return $m[1].'[^\/]+'.$m[2];
-                },
-                $key2
-            );
-        }
+        $pattern = '/:[^\/]+/';
+
+        $key2 = preg_replace_callback(
+            $pattern,
+            function ($m) {
+                return '[^\/]+';
+            },
+            $key2
+        );
 
-        return self::regexMatch($key1, '^'.$key2.'$');
+        return self::regexMatch($key1, '^' . $key2 . '$');
     }
 
     /**
@@ -108,21 +104,16 @@ public static function keyMatch3(string $key1, string $key2): bool
     {
         $key2 = str_replace(['/*'], ['/.*'], $key2);
 
-        $pattern = '/(.*)\{[^\/]+\}(.*)/';
-        for (; ;) {
-            if (false === strpos($key2, '/{')) {
-                break;
-            }
-            $key2 = preg_replace_callback(
-                $pattern,
-                function ($m) {
-                    return $m[1].'[^\/]+'.$m[2];
-                },
-                $key2
-            );
-        }
+        $pattern = '/\{[^\/]+\}/';
+        $key2 = preg_replace_callback(
+            $pattern,
+            function ($m) {
+                return '[^\/]+';
+            },
+            $key2
+        );
 
-        return self::regexMatch($key1, '^'.$key2.'$');
+        return self::regexMatch($key1, '^' . $key2 . '$');
     }
 
     /**
@@ -150,7 +141,7 @@ public static function keyMatch3Func(...$args): bool
      */
     public static function regexMatch(string $key1, string $key2): bool
     {
-        return (bool) preg_match('~'.$key2.'~', $key1);
+        return (bool) preg_match('~' . $key2 . '~', $key1);
     }
 
     /**

diff --git a/tests/Unit/Util/BuiltinOperationsTest.php b/tests/Unit/Util/BuiltinOperationsTest.php
@@ -68,6 +68,8 @@ public function testKeyMatch2Func()
         $this->assertTrue($this->keyMatch2Func('/alice/all', '/:id/all'));
         $this->assertFalse($this->keyMatch2Func('/alice', '/:id/all'));
         $this->assertFalse($this->keyMatch2Func('/alice/all', '/:id'));
+
+        $this->assertFalse($this->keyMatch2Func('/alice/all', '/:/all'));
     }
 
     public function testKeyMatch3Func()
@@ -93,5 +95,7 @@ public function testKeyMatch3Func()
         $this->assertTrue($this->keyMatch3Func('/proxy/myid/res/res2', '/proxy/{id}/*'));
         $this->assertTrue($this->keyMatch3Func('/proxy/myid/res/res2/res3', '/proxy/{id}/*'));
         $this->assertFalse($this->keyMatch3Func('/proxy/', '/proxy/{id}/*'));
+
+        $this->assertFalse($this->keyMatch3Func('/myid/using/myresid', '/{id/using/{resId}'));
     }
 }