Skip to content
This repository has been archived by the owner on Mar 27, 2018. It is now read-only.

Autoloading breaks HTMLPurifier #51

Closed
UFOMelkor opened this issue Sep 25, 2014 · 17 comments · Fixed by #52
Closed

Autoloading breaks HTMLPurifier #51

UFOMelkor opened this issue Sep 25, 2014 · 17 comments · Fixed by #52
Assignees
Labels

Comments

@UFOMelkor
Copy link

I use PhpMetrics within a project and PhpMetrics has a dependency on hoa/core.

Today I added the HTMLPurifier to the project (using composer) and the following error appears:

RuntimeException: The autoloader expected class "HTMLPurifier\Bootstrap" to be defined in
file "/path/vendor/ezyang/htmlpurifier/library/HTMLPurifier/Bootstrap.php". The file
was found but the class was not in it, the class name or namespace probably has a typo.

The Stacktrace:

in /path/vendor/symfony/symfony/src/Symfony/Component/Debug/DebugClassLoader.php line 189
at DebugClassLoader->loadClass('HTMLPurifier\Bootstrap')
at spl_autoload_call('HTMLPurifier\Bootstrap') in /path/vendor/hoa/core/Hoa/Core/Consistency.php line 288
at Consistency->_import('HTMLPurifier.Bootstrap', true) in /path/vendor/hoa/core/Hoa/Core/Consistency.php line 335
at Consistency::autoload('HTMLPurifier_Bootstrap')
at spl_autoload_call('HTMLPurifier_Bootstrap') in /path/vendor/exercise/htmlpurifier-bundle/Exercise/HTMLPurifierBundle/ExerciseHTMLPurifierBundle.php line 11
at ExerciseHTMLPurifierBundle->boot() in /path/app/bootstrap.php.cache line 2305
at Kernel->boot() in /path/app/bootstrap.php.cache line 2333
at Kernel->handle(object(Request)) in /path/web/app_dev.php line 27

As you can see the Autoloading is broken because it now looks for HTMLPurifier\Bootstrap instead of HTMLPurifier_Bootstrap.

@Hywan
Copy link
Member

Hywan commented Sep 25, 2014

Hoa's autoloader runs if Composer's one failed. Can you confirm that?

@jubianchi
Copy link
Member

@Hywan I think the fail here is that when Hoa fails to load a class, it forwards the autoloader call to composer but it does not forward it the same way it received it. Taking a look at the stack trace:

  • We got a call to spl_autoload_call('HTMLPurifier_Bootstrap')
  • This call gets handled by Hoa: Consistency::autoload('HTMLPurifier_Bootstrap')
  • Hoa tries to load HTMLPurifier.Bootstrap: Consistency->_import('HTMLPurifier.Bootstrap', true) (Hoa/Core/Consistency.php:335)
  • Hoa does not find the class and it forwards the SPL autoloader: spl_autoload_call('HTMLPurifier\Bootstrap') (Hoa/Core/Consistency.php:288)

The last forward shows that the original class name has been modified from HTMLPurifier_Bootstrap to HTMLPurifier\Bootstrap

@UFOMelkor
Copy link
Author

When I remove the first entry of my autoload_files.php the class will be loaded correctly:

return array(
    $vendorDir . '/hoa/core/Hoa/Core/Core.php',
    $vendorDir . '/swiftmailer/swiftmailer/lib/swift_required.php',
    $vendorDir . '/symfony/symfony/src/Symfony/Component/Intl/Resources/stubs/functions.php',
    $vendorDir . '/kriswallsmith/assetic/src/functions.php',
    $vendorDir . '/ezyang/htmlpurifier/library/HTMLPurifier.composer.php',
    $vendorDir . '/beberlei/assert/lib/Assert/functions.php',
);

Otherwise the error appears. Unfortunately I can't provide a minimal example in a quick way, but I'll investigate time in the next days.

@Hywan
Copy link
Member

Hywan commented Sep 25, 2014

This PSR-0 vs. PSR-4. Hoa's autoloader is PSR-0 so it transforms _ into \ as the recommandation asks. In PSR-4, it has changed: _ has no longer any meaning.

@Hywan
Copy link
Member

Hywan commented Sep 25, 2014

No sorry, Hoa's autoloader is PSR-4 but for _, it works like PSR-0. So, what do we do? We propose a patch to be fully PSR-4 and remove everything from PSR-0?

@Hywan
Copy link
Member

Hywan commented Sep 25, 2014

Also, @UFOMelkor, why Composer does not handle your class?

@jubianchi
Copy link
Member

@Hywan you can't blindly replace _ by \ because the class with \ does not exist. To handle this, composer will likely use a classmap strategy.

@Hywan
Copy link
Member

Hywan commented Sep 25, 2014

@jubianchi I replaced _ by \ because this is what PSR-0 required!

Each _ character in the CLASS NAME is converted to a DIRECTORY_SEPARATOR. The _ character has no special meaning in the namespace.

This is why we have added the 436f30f commit.

Also, Hoa's autoloader runs if and only if Composer has dropped. So, why Hoa's autoloader runs? Why Composer drops?

@jubianchi
Copy link
Member

@Hywan PSR-0 never said that autoloader should replace _ with NS_SEPARATOR : if it did, it's a fail as this can not work.

The only thinkg I see is the following:

$fileName .= str_replace('_', DIRECTORY_SEPARATOR, $className) . '.php';

Which is OK. DIRECTORY_SEPARATOR is different from \ which is NS_SEPARATOR and this is not Hoa's responsibility to do that: the only component allowed to do that is the one which knows how to resolve the path from the classname.

@Hywan
Copy link
Member

Hywan commented Sep 25, 2014

@jubianchi _ is converted into DIRECTORY_SEPARATOR, which is equivalent to say that: _ is converted into NAMESPACE_SEPARATOR and then (because this is PSR-0 or 4), NAMESPACE_SEPARATOR is converted into DIRECTORY_SEPARATOR. This is strictly identical, no?

@jubianchi
Copy link
Member

@Hywan no it's not : when your replace character in class name, you create a bug : classes using _ notation won't always use PSR-0 directory structure.

I think that you should not do such assumption and just forward the autoloader call as you received it when you can't handle it.

@Hywan
Copy link
Member

Hywan commented Sep 25, 2014

The issue is still open. Why Composer does not handle that class?

@UFOMelkor
Copy link
Author

I created a minimal example using symfony2. Perhaps anyone might have a look, I want have much time the next days.

What I did: I created a simple bundle and I added the Exercise/HTMLPurifierBundle and PhpMetrics to composer and activated the first one.

@jubianchi
Copy link
Member

@Hywan autoloader files (which is how hoa/core works with composer) are called first, before the composer PSR autoloaders that's why this is not working: hoa is called first, does not find the requested class (HTMLPurifier_Bootstrap), changes its name (HTMLPurifier\Bootstrap), forwards the call to composer with the wrong classname.

@Hywan
Copy link
Member

Hywan commented Sep 26, 2014

@jubianchi So a solution is to remove this piece of code and everything will be ok. We assume that Hoa's autoloader is PSR-4 and voilà?

@Hywan
Copy link
Member

Hywan commented Sep 26, 2014

@UFOMelkor Could you test with #52 please?

@UFOMelkor
Copy link
Author

Looks good 👍

@Bhoat Bhoat closed this as completed in #52 Sep 28, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

3 participants