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

Keep same return for class names without alias #20

Merged
merged 1 commit into from
Aug 7, 2022
Merged

Keep same return for class names without alias #20

merged 1 commit into from
Aug 7, 2022

Conversation

hoogi91
Copy link
Contributor

@hoogi91 hoogi91 commented Mar 17, 2021

Current intention of ClassAliasLoader::getOriginalClassName is to return NULL if no alias mapping is found or the original class name as string`

But the behaviour actually sets a NULL value in case a alias mapping is not found. This would lead on a second call to catch into the first condition which then returns the input parameter class name. This in case leads to the error that ClassAliasLoader::loadClassWithAlias will call loadOriginalClassAndSetAliases on a second try cause the return value is no more NULL but then the class name that was given as parameter. In some cases like for testing this results into PHP Warnings :)

The fix will now check if the value has already been set to NULL and returns the same result like in the first call.

@helhum
Copy link
Contributor

helhum commented Mar 18, 2021

Indeed, good catch.
Can you nevertheless explain in more detail how you can trigger this in practice?

@hoogi91
Copy link
Contributor Author

hoogi91 commented Mar 20, 2021

Of course, it tooked me some time to figure out what exactly was going on but following is the explanation. I need to notice before that we are relying on https://github.com/Roave/BetterReflection for better reflection performances. This library is the reason why we found this behaviour during code test execution. Imagine the following code exaple:

<?php
include 'vendor/autoload.php';
(new \Roave\BetterReflection\BetterReflection)->classReflector()->reflect(\Vendor\Package\MyClass::class);
new \Vendor\Package\MyClass(); 

which results into:

Warning: Invalid argument supplied for foreach() in /home/thorsten/typo3-alias-error/vendor/typo3/class-alias-loader/src/ClassAliasLoader.php on line 175

The reason behind this is inside BetterReflection library at https://github.com/Roave/BetterReflection/blob/4.13.x/src/SourceLocator/Type/AutoloadSourceLocator/FileReadTrapStreamWrapper.php :)
This useful stream wrapper checks the existence of class files by hooking into the usage of include statements to only save the path and returns always false which then prevents PHP from loading the class content. In case of reflecting a class with this library before loading it e.g. by new MyClass, composers autoloader is still triggered and the ClassAliasLoader saves the information in aliasMap. But the class file has never been included by PHP process and therefore the second instantiation leads to a second autoload try!
The second autoload try now get's into the getOriginalClassName method with an already existing aliasMap cache that leads to a different return value of this method and in further processing to the call of loadOriginalClassAndSetAliases instead of loadClass inside ClassAliasLoader

Cheers 🍻

@helhum
Copy link
Contributor

helhum commented Aug 7, 2022

This looks good, thanks and sorry for the large delay!

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.

2 participants