Fix splitting of string rules (#4957)

codeigniter4 · Sep 7, 2021 · ade0b7b · ade0b7b
1 parent b987704
commit ade0b7b
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 11 deletions.
diff --git a/system/Validation/Validation.php b/system/Validation/Validation.php
@@ -672,20 +672,41 @@ protected function getErrorMessage(string $rule, string $field, ?string $label =
      */
     protected function splitRules(string $rules): array
     {
-        $nonEscapeBracket = '((?<!\\\\)(?:\\\\\\\\)*[\[\]])';
-        $pipeNotInBracket = sprintf(
-            '/\|(?=(?:[^\[\]]*%s[^\[\]]*%s)*(?![^\[\]]*%s))/',
-            $nonEscapeBracket,
-            $nonEscapeBracket,
-            $nonEscapeBracket
-        );
+        if (strpos($rules, '|') === false) {
+            return [$rules];
+        }
 
-        $_rules = preg_split($pipeNotInBracket, $rules);
+        $string = $rules;
+        $rules  = [];
+        $length = strlen($string);
+        $cursor = 0;
 
-        return array_unique($_rules);
-    }
+        while ($cursor < $length) {
+            $pos = strpos($string, '|', $cursor);
+
+            if ($pos === false) {
+                // we're in the last rule
+                $pos = $length;
+            }
+
+            $rule = substr($string, $cursor, $pos - $cursor);
 
-    // Misc
+            while (
+                (substr_count($rule, '[') - substr_count($rule, '\['))
+                !== (substr_count($rule, ']') - substr_count($rule, '\]'))
+            ) {
+                // the pipe is inside the brackets causing the closing bracket to
+                // not be included. so, we adjust the rule to include that portion.
+                $pos  = strpos($string, '|', $cursor + strlen($rule) + 1) ?: $length;
+                $rule = substr($string, $cursor, $pos - $cursor);
+            }
+
+            $rules[] = $rule;
+            $cursor += strlen($rule) + 1; // +1 to exclude the pipe
+        }
+
+        return array_unique($rules);
+    }
 
     /**
      * Resets the class to a blank slate. Should be called whenever

diff --git a/tests/system/Validation/ValidationTest.php b/tests/system/Validation/ValidationTest.php
@@ -984,4 +984,68 @@ public function validationArrayDataCaseProvider(): iterable
             ]],
         ];
     }
+
+    /**
+     * @dataProvider provideStringRulesCases
+     *
+     * @param string $input
+     * @param array  $expected
+     *
+     * @return void
+     *
+     * @see https://github.com/codeigniter4/CodeIgniter4/issues/4929
+     */
+    public function testSplittingOfComplexStringRules(string $input, array $expected): void
+    {
+        $splitter = $this->getPrivateMethodInvoker($this->validation, 'splitRules');
+        $this->assertSame($expected, $splitter($input));
+    }
+
+    public function provideStringRulesCases(): iterable
+    {
+        yield [
+            'required',
+            ['required'],
+        ];
+
+        yield [
+            'required|numeric',
+            ['required', 'numeric'],
+        ];
+
+        yield [
+            'required|max_length[500]|hex',
+            ['required', 'max_length[500]', 'hex'],
+        ];
+
+        yield [
+            'required|numeric|regex_match[/[a-zA-Z]+/]',
+            ['required', 'numeric', 'regex_match[/[a-zA-Z]+/]'],
+        ];
+
+        yield [
+            'required|max_length[500]|regex_match[/^;"\'{}\[\]^<>=/]',
+            ['required', 'max_length[500]', 'regex_match[/^;"\'{}\[\]^<>=/]'],
+        ];
+
+        yield [
+            'regex_match[/^;"\'{}\[\]^<>=/]|regex_match[/[^a-z0-9.\|_]+/]',
+            ['regex_match[/^;"\'{}\[\]^<>=/]', 'regex_match[/[^a-z0-9.\|_]+/]'],
+        ];
+
+        yield [
+            'required|regex_match[/^(01[2689]|09)[0-9]{8}$/]|numeric',
+            ['required', 'regex_match[/^(01[2689]|09)[0-9]{8}$/]', 'numeric'],
+        ];
+
+        yield [
+            'required|regex_match[/^[0-9]{4}[\-\.\[\/][0-9]{2}[\-\.\[\/][0-9]{2}/]|max_length[10]',
+            ['required', 'regex_match[/^[0-9]{4}[\-\.\[\/][0-9]{2}[\-\.\[\/][0-9]{2}/]', 'max_length[10]'],
+        ];
+
+        yield [
+            'required|regex_match[/^(01|2689|09)[0-9]{8}$/]|numeric',
+            ['required', 'regex_match[/^(01|2689|09)[0-9]{8}$/]', 'numeric'],
+        ];
+    }
 }