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

fix: Remove unnecessary disablement of autoload in class_exists usage #364

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

stevebauman
Copy link
Contributor

@stevebauman stevebauman commented Feb 5, 2023

This PR removes the usage of the secondary argument in class_exists() checks in HTML Purifier. This secondary argument is being used to disable autoloading the class if it has not yet been autoloaded.

The current implementation breaks composer based projects where a class may not have been included or required yet via composers autoloader, but has been registered as a definition cache:

$cache = \App\Purifier\RemoteFilesystemDefinition::class;

HTMLPurifier_DefinitionCacheFactory::instance()->register($cache, $cache);

$htmlConfig = HTMLPurifier_Config::create($config);

$htmlConfig->set('Cache.DefinitionImpl', $cache);

Upon usage:

ErrorException : Unrecognized DefinitionCache App\Purifier\RemoteFilesystemDefinition, using Serializer instead

This exception can be resolved by require'ing or include'ing the class manually, or via a spl_autoload_call($cache):

$cache = \App\Purifier\RemoteFilesystemDefinition::class;

spl_autoload_call($cache);

HTMLPurifier_DefinitionCacheFactory::instance()->register($cache, $cache);

However, this should not be necessary in todays PHP, as class strings are frequently used as a way to dynamically register implementations of things, and removing the secondary argument in class_exists() results in the definition cache being properly registered and used.

@ezyang ezyang changed the title Remove unnecessary disablement of autoload in class_exists usage fix: Remove unnecessary disablement of autoload in class_exists usage Feb 6, 2023
@ezyang ezyang merged commit b4136da into ezyang:master Feb 6, 2023
@binaryfire
Copy link

Hi @ezyang. Would it be possible to tag a new release which includes this fix?

Copy link

🎉 This PR is included in version 4.17.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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 this pull request may close these issues.

None yet

3 participants