Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.x] Conditional extender instantiates the extenders array even when the conditional is false #3897

Closed
imorland opened this issue Oct 11, 2023 · 0 comments · Fixed by #3898
Assignees
Labels

Comments

@imorland
Copy link
Member

imorland commented Oct 11, 2023

Current Behavior

The Conditional extender instantiates provided extenders even if the associated condition is false. This can lead to unintended side effects, especially if the constructors of these extenders perform actions. More critically, if the extender class doesn't even exist, it will result in a fatal error due to class not being found, regardless of the condition's value.

Steps to Reproduce

Reference a nonexistent extender class within the Conditional extender.

(new Extend\Conditional())
    ->when(false, [new NonexistentExtender()]);

Upon execution, PHP will throw a fatal error stating that the NonexistentExtender class was not found, even though the condition was set to false.

Expected Behavior

The expected behavior is that the Conditional extender should only attempt to instantiate and use provided extenders when the associated condition evaluates to true, and should completely ignore them when the condition is false.

Screenshots

No response

Environment

  • Flarum version: x.y.z
  • Website URL: http://example.com
  • Webserver: [e.g. apache, nginx]
  • Hosting environment: [e.g. shared, vps]
  • PHP version: x.y.z
  • Browser: [e.g. chrome 67, safari 11]

Output of php flarum info

Output of "php flarum info", run this in terminal in your Flarum directory.

Possible Solution

Instead of directly providing an array of extenders, wrap them in a callback, ie invokable class

Additional Context

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant