Skip to content

[9.x] Fix ExcludeIf regression to use Closure over is_callable()#41969

Merged
taylorotwell merged 1 commit into
laravel:9.xfrom
BrandonSurowiec:fix_exclude_if
Apr 14, 2022
Merged

[9.x] Fix ExcludeIf regression to use Closure over is_callable()#41969
taylorotwell merged 1 commit into
laravel:9.xfrom
BrandonSurowiec:fix_exclude_if

Conversation

@BrandonSurowiec

Copy link
Copy Markdown
Contributor

This fixes a regression introduced by #41931. In the original test, the string phpinfo was being pass into the constructor. After the rewrite, this check was changed to foobar.

// Original Test
$this->expectException(InvalidArgumentException::class);

$rule = new ExcludeIf('phpinfo');

Why this matters is that is_callable('phpinfo') will evaluate to TRUE as it is a function in the global namespace. This switches the check from is_callable($condition) to the more strict $condition instanceof Closure and updates the test to check the string phpinfo once again.

@taylorotwell taylorotwell merged commit e5775a7 into laravel:9.x Apr 14, 2022
@BrandonSurowiec BrandonSurowiec deleted the fix_exclude_if branch April 14, 2022 13:58
BrandonSurowiec added a commit to BrandonSurowiec/framework that referenced this pull request Jun 27, 2022
This applies the same fixes to ProhibitIf as ExcludeIf.
- Missing @throws declaration in docblock: laravel#41729
- Enforces a boolean in the constructor: laravel#41931 laravel#41969

Also fixes the ExcludeIf @var docblock to match as a \Closure
taylorotwell pushed a commit that referenced this pull request Jun 27, 2022
This applies the same fixes to ProhibitIf as ExcludeIf.
- Missing @throws declaration in docblock: #41729
- Enforces a boolean in the constructor: #41931 #41969

Also fixes the ExcludeIf @var docblock to match as a \Closure
taylorotwell pushed a commit to illuminate/validation that referenced this pull request Jun 27, 2022
This applies the same fixes to ProhibitIf as ExcludeIf.
- Missing @throws declaration in docblock: laravel/framework#41729
- Enforces a boolean in the constructor: laravel/framework#41931 laravel/framework#41969

Also fixes the ExcludeIf @var docblock to match as a \Closure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants