diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index fd15ba445838..e4070b9eeccc 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -156,7 +156,9 @@ public function setResponse(ResponseInterface $response) * Runs through all of the filters for the specified * uri and position. * - * @return mixed|RequestInterface|ResponseInterface + * @param string $uri URI path relative to baseURL + * + * @return RequestInterface|ResponseInterface|string|null * * @throws FilterException */ @@ -221,6 +223,8 @@ public function run(string $uri, string $position = 'before') * run through both a before and after and don't want to double * process the rows. * + * @param string|null $uri URI path relative to baseURL (all lowercase) + * * @return Filters */ public function initialize(?string $uri = null) @@ -391,7 +395,7 @@ public function getArguments(?string $key = null) /** * Add any applicable (not excluded) global filter settings to the mix. * - * @param string $uri + * @param string|null $uri URI path relative to baseURL (all lowercase) * * @return void */ @@ -416,7 +420,7 @@ protected function processGlobals(?string $uri = null) if (isset($rules['except'])) { // grab the exclusion rules $check = $rules['except']; - if ($this->pathApplies($uri, $check)) { + if ($this->checkExcept($uri, $check)) { $keep = false; } } @@ -454,7 +458,7 @@ protected function processMethods() /** * Add any applicable configured filters to the mix. * - * @param string $uri + * @param string|null $uri URI path relative to baseURL (all lowercase) * * @return void */ @@ -536,12 +540,47 @@ private function pathApplies(string $uri, $paths) $paths = [$paths]; } - // treat each paths as pseudo-regex + return $this->checkPseudoRegex($uri, $paths); + } + + /** + * Check except paths + * + * @param string $uri URI path relative to baseURL (all lowercase) + * @param array|string $paths The except path patterns + * + * @return bool True if the URI matches except paths. + */ + private function checkExcept(string $uri, $paths): bool + { + // empty array does not match anything + if ($paths === []) { + return false; + } + + // make sure the paths are iterable + if (is_string($paths)) { + $paths = [$paths]; + } + + return $this->checkPseudoRegex($uri, $paths); + } + + /** + * Check the URI path as pseudo-regex + * + * @param string $uri URI path relative to baseURL (all lowercase) + * @param array $paths The except path patterns + */ + private function checkPseudoRegex(string $uri, array $paths): bool + { + // treat each path as pseudo-regex foreach ($paths as $path) { // need to escape path separators $path = str_replace('/', '\/', trim($path, '/ ')); // need to make pseudo wildcard real $path = strtolower(str_replace('*', '.*', $path)); + // Does this rule apply here? if (preg_match('#^' . $path . '$#', $uri, $match) === 1) { return true; diff --git a/tests/system/Filters/FiltersTest.php b/tests/system/Filters/FiltersTest.php index 5c0fc2efabdf..ae75b14af835 100644 --- a/tests/system/Filters/FiltersTest.php +++ b/tests/system/Filters/FiltersTest.php @@ -215,15 +215,23 @@ public static function provideProcessMethodProcessGlobalsWithExcept(): iterable ['admin/*'], ], [ - [], + ['admin/*', 'foo/*'], + ], + [ + ['*'], + ], + [ + 'admin/*', ], ]; } /** * @dataProvider provideProcessMethodProcessGlobalsWithExcept + * + * @param array|string $except */ - public function testProcessMethodProcessGlobalsWithExcept(array $except): void + public function testProcessMethodProcessGlobalsWithExcept($except): void { $_SERVER['REQUEST_METHOD'] = 'GET'; @@ -572,7 +580,12 @@ public function testOtherResult(): void $this->assertSame('This is curious', $response); } - public function testBeforeExceptString(): void + /** + * @dataProvider provideBeforeExcept + * + * @param array|string $except + */ + public function testBeforeExcept(string $uri, $except, array $expected): void { $_SERVER['REQUEST_METHOD'] = 'GET'; @@ -584,7 +597,7 @@ public function testBeforeExceptString(): void ], 'globals' => [ 'before' => [ - 'foo' => ['except' => 'admin/*'], + 'foo' => ['except' => $except], 'bar', ], 'after' => [ @@ -595,48 +608,91 @@ public function testBeforeExceptString(): void $filtersConfig = $this->createConfigFromArray(FiltersConfig::class, $config); $filters = $this->createFilters($filtersConfig); - $uri = 'admin/foo/bar'; - $expected = [ - 'before' => [ - 'bar', - ], - 'after' => ['baz'], - ]; $this->assertSame($expected, $filters->initialize($uri)->getFilters()); } - public function testBeforeExceptInapplicable(): void + public static function provideBeforeExcept(): iterable { - $_SERVER['REQUEST_METHOD'] = 'GET'; - - $config = [ - 'aliases' => [ - 'foo' => '', - 'bar' => '', - 'baz' => '', + return [ + 'string exclude' => [ + 'admin/foo/bar', + 'admin/*', + [ + 'before' => [ + 'bar', + ], + 'after' => ['baz'], + ], ], - 'globals' => [ - 'before' => [ - 'foo' => ['except' => 'george/*'], - 'bar', + 'string not exclude' => [ + 'admin/foo/bar', + 'george/*', + [ + 'before' => [ + 'foo', + 'bar', + ], + 'after' => ['baz'], ], - 'after' => [ - 'baz', + ], + 'empty array not exclude' => [ + 'admin/foo/bar', + [], + [ + 'before' => [ + 'foo', + 'bar', + ], + 'after' => ['baz'], ], ], - ]; - $filtersConfig = $this->createConfigFromArray(FiltersConfig::class, $config); - $filters = $this->createFilters($filtersConfig); - - $uri = 'admin/foo/bar'; - $expected = [ - 'before' => [ - 'foo', - 'bar', + 'empty string not exclude' => [ + 'admin/foo/bar', + // The URI path '' means the baseURL. + '', + [ + 'before' => [ + 'foo', + 'bar', + ], + 'after' => ['baz'], + ], + ], + 'empty string exclude' => [ + // The URI path '' means the baseURL. + '', + // So this setting excludes `foo` filter only to the baseURL. + '', + [ + 'before' => [ + 'bar', + ], + 'after' => ['baz'], + ], + ], + 'slash not exclude' => [ + 'admin/foo/bar', + '/', + [ + 'before' => [ + 'foo', + 'bar', + ], + 'after' => ['baz'], + ], + ], + 'slash exclude' => [ + // The URI path '' means the baseURL. + '', + '/', + [ + 'before' => [ + 'bar', + ], + 'after' => ['baz'], + ], ], - 'after' => ['baz'], ]; - $this->assertSame($expected, $filters->initialize($uri)->getFilters()); } public function testAfterExceptString(): void diff --git a/user_guide_src/source/changelogs/v4.3.8.rst b/user_guide_src/source/changelogs/v4.3.8.rst index b4c8e4da4a69..d62e5a5ef65b 100644 --- a/user_guide_src/source/changelogs/v4.3.8.rst +++ b/user_guide_src/source/changelogs/v4.3.8.rst @@ -24,6 +24,12 @@ Deprecations Bugs Fixed ********** +- **Controller Filters:** In previous versions, ``['except' => []]`` or ``['except' => '']`` + meant "except all". The bug has been fixed, and now + + - ``['except' => []]`` means to exclude nothing. + - ``['except' => '']`` means to exclude the baseURL only. + See the repo's `CHANGELOG.md `_ for a complete list of bugs fixed.