-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Routing] Fixes fatal errors with object resources in AnnotationDirectoryLoader::supports #11232
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
Conversation
if (!is_string($resource)) { | ||
return false; | ||
} | ||
|
||
try { | ||
$path = $this->locator->locate($resource); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileLocatorInterface allows mixed
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Config/FileLocatorInterface.php#L22 . But the actual implementation https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Config/FileLocator.php#L44 doesn't actually work with any parameter. So this issue is there and not really in this class here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well so the question is if FileLocator(Interface) shouldn't allow mixed
in which case this change is relevant or if it should and appropriate string conversion and or exceptions would've to be implemented there.
In both cases moving is_string
upfront removes some overhead or it could be removed of course.
👍 as it fixes a fatal error and is only about moving things around. |
As I said, the real problem is the locator. And depending on how the locator is fixed, this fix here applies or does not apply. If the locator really works with mixed, then the |
So how do i continue? Should I create an issue to discuss the FileLocator? |
Since the FileLocatorInterface has been recently changed to only "allow" strings, it'd be nice if this change could be implemented. |
Thank you @Tischoi. |
…tationDirectoryLoader::supports (Tischoi) This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #11232). Discussion ---------- [Routing] Fixes fatal errors with object resources in AnnotationDirectoryLoader::supports | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Fixes fatal errors that occur in the supports method with objects that aren't string convertible / don't implement ArrayAccess. This is mostly a problem because some locators try to access a specific character in the resource name. Since the resource is checked if it's a string either way, it's the most simple solution to just move that check a bit ahead. Commits ------- 5e80585 Update AnnotationDirectoryLoader.php
Fixes fatal errors that occur in the supports method with objects that aren't string convertible / don't implement ArrayAccess. This is mostly a problem because some locators try to access a specific character in the resource name.
Since the resource is checked if it's a string either way, it's the most simple solution to just move that check a bit ahead.