Skip to content

Commit

Permalink
Merge pull request #7823 from kenjis/fix-filter-except
Browse files Browse the repository at this point in the history 
fix: filter except empty
  • Loading branch information
@kenjis
kenjis authored Aug 21, 2023
2 parents 0665d5f + e510da9 commit 700f6e4
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 41 deletions.
49 changes: 44 additions & 5 deletions system/Filters/Filters.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
*/
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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;
Expand Down
128 changes: 92 additions & 36 deletions tests/system/Filters/FiltersTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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';

Expand All @@ -584,7 +597,7 @@ public function testBeforeExceptString(): void
],
'globals' => [
'before' => [
'foo' => ['except' => 'admin/*'],
'foo' => ['except' => $except],
'bar',
],
'after' => [
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions user_guide_src/source/changelogs/v4.3.8.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_
for a complete list of bugs fixed.

0 comments on commit 700f6e4

Please sign in to comment.