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

ScriptUrl: Fix class extendability (issue #187) #188

Merged
merged 1 commit into from
Nov 16, 2020

Conversation

jakubboucek
Copy link
Contributor

UrlScript is not work correctly when is extended. The method withPath() is calling themself recursive in infinite loop and crash with error:

Maximum function nesting level of '256' reached, aborting!

The reason is the callback parent::withPath in call_user_func() function is always called in top of descendants hierarchy context (see more at issue #187).

PR is changing relative reference to parent class to absolute. Here is no able to use only parent because method is called in another instance than $this (cloned one).

Tests:

  • All passed.
  • No new one necessary

Resolve #187

@@ -54,7 +54,8 @@ public function withPath(string $path, string $scriptPath = '')
{
$dolly = clone $this;
$dolly->scriptPath = $scriptPath;
return call_user_func([$dolly, 'parent::withPath'], $path);
$parent = \Closure::fromCallable([UrlImmutable::class, 'withPath'])->bindTo($dolly);
return $parent($path);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closure::call() exists since PHP 7, so it should be possible to simplify this to

Closure::fromCallable([UrlImmutable::class, 'withPath'])->call($dolly, $path)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it first. It works only for anonymous functions.

Cannot rebind scope of closure created by ReflectionFunctionAbstract::getClosure()

More info: https://bugs.php.net/bug.php?id=74558

@dg
Copy link
Member

dg commented Nov 16, 2020

Thanks

@dg dg merged commit 0c0f9d2 into nette:master Nov 16, 2020
@jakubboucek jakubboucek deleted the feature/urlscript-extend branch November 16, 2020 02:38
dg pushed a commit that referenced this pull request Dec 17, 2020
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.

Unable to extend UrlScript
3 participants