YahnisElsts / plugin-update-checker

A custom update checker for WordPress plugins. Useful if you don't want to host your project in the official WP repository, but would still like it to support automatic updates. Despite the name, it also works with themes.
MIT License
2.22k stars 403 forks source link

PHP Scoper namespace bug #496

Closed light-source closed 2 years ago

light-source commented 2 years ago

Hello. I see that the package supports Scoper, but I have found a bug, related to namespaces.

How to reproduce

Use Scoper with a namespace that contains an underline, e.g. org\wplake\acf_views\vendors. Then the Puc autoloader will be broken, and classes, like Puc_v4p11_Plugin_UpdateChecker, will be not loaded, also the Puc_v4_Factory::buildUpdateChecker() method doesn't work in this case.

How to fix

My suggestion - using the __NAMESPACE__ const. Below is my fix, that I added to my local files.

  1. Autoloader CURRENT _construct ` $nameParts = \explode('', CLASS, 3); $this->prefix = $nameParts[0] . '' . $nameParts[1] . '';`

NEW construct `$className = ! NAMESPACE ? CLASS : substr(CLASS, strlen(NAMESPACE_ . '\')); $nameParts = \explode('', $className, 3); $this->prefix = $nameParts[0] . '' . $nameParts[1] . ''; $this->prefix = ! NAMESPACE ? $this->prefix : NAMESPACE . '\' . $this->prefix;`

  1. Factory

CURRENT addVersion $nameParts = \explode('_', __CLASS__, 3);

NEW addVersion $className = ! __NAMESPACE__ ? __CLASS__ : substr(__CLASS__, strlen(__NAMESPACE__ . '\\')); $nameParts = \explode('_', $className, 3);

Thank you in advance for looking into it.

YahnisElsts commented 2 years ago

I added some of your suggested fixes to a new scoper-fixes branch. Please take a look and let me know if it works for you.

I suspect this could also help with #497 since that sounds like it might be an autoloading-related issue.

light-source commented 2 years ago
  1. Your fix have an issue, as __NAMESPACE__ is a magic constant, defined('__NAMESPACE__') returns false, and nothing works, you should use another way to check in Autoloader & Factory, e.g. version_compare(phpversion(), '5.3.0','>=') && __NAMESPACE__
  2. 497 bug still actual in the scoper-fixes branch. The DebugBar/Panel.php contains this declaration : class Puc_v4p11_DebugBar_Panel extends Debug_Bar_Panel, and the Debug_Bar_Panel class is missing in the package by default (I wasn't able to find it). Checking for existing the class already added to the DebugBar/Panel.php , but missing for DebugBar/PluginPanel.php and DebugBar/ThemePanel.php, that expect Puc_v4p11_DebugBar_Panel existing (as they extend it), but by the fact the class is missing and it leads to a fatal error.

light-source commented 2 years ago

Also wanted to ask, what a goal of providing support for 'php>=5.2.0`? Because it adds so many troubles and makes package's code less readable, are there reasons to support it in 2022?

YahnisElsts commented 2 years ago

All right, I've replaced the defined() checks with version_compare().

The DebugBar/Panel.php contains this declaration : class Puc_v4p11_DebugBar_Panel extends Debug_Bar_Panel, and the Debug_Bar_Panel class is missing in the package by default (I wasn't able to find it).

It sounds like I misread #497. I thought it was about the DebugBar_Panel class (named Puc_v4p11_DebugBar_Panel in this version), but you're actually talking about Debug_Bar_Panel. That class is provided by the Debug Bar plugin, and PUC uses it to integrate with that plugin and display some debugging information.

Of course, Debug_Bar_Panel is not included in this package, but that normally shouldn't be an issue because PUC will not try to load or instantiate a panel class unless the Debug Bar plugin is also active and it triggers the custom debug_bar_panels filter.

Also wanted to ask, what a goal of providing support for 'php>=5.2.0`? Because it adds so many troubles and makes package's code less readable, are there reasons to support it in 2022?

Up until a couple of years ago, WordPress core supported PHP 5.2, and so PUC also supported it for the sake of compatibility (and because many sites were still using it). Now that WordPress has finally officially dropped support for PHP 5.2 and requires PHP 5.6.20 instead, it makes sense for PUC to stop supporting PHP 5.2 as well. I just haven't gotten around to it.

light-source commented 2 years ago

Thanks for the update. Now the prefix issue is solved. Please release a new package version with the fix.

Thanks for the explanation about PHP 5.2, I think abandoning this supporting will open many opportunities to improve package's code, e.g. using the built-in composer autoloader, separate namespace for the package instead of global classes and such. Thanks for the great work.

About the #497 issue, thanks for the explanation, after some research I've found that the scoper exposes global classes by default (so it tries to autoload all global classes by default), and solved with adding to an ignore list. Btw : I'm sharing my scoper's exclude list for functions and classes below, so you could add to your docs if you think it's useful for others return [ 'exclude-functions' => [ 'wp_normalize_path', 'apply_filters', 'add_filter', '__', 'esc_url', 'network_admin_url', 'esc_attr', 'wp_nonce_url', 'add_query_arg', 'check_admin_referer', 'set_site_transient', 'wp_redirect', 'get_site_transient', 'delete_site_transient', 'remove_action', 'remove_filter', 'plugin_basename', 'register_deactivation_hook', 'current_user_can', 'get_plugin_data', 'wp_next_scheduled', 'wp_schedule_event', 'wp_clear_scheduled_hook', 'current_filter', 'update_site_option', 'get_site_option', 'delete_site_option', 'did_action', 'is_admin', 'get_user_locale', 'get_locale', 'load_textdomain', 'get_core_updates', 'wp_remote_get', 'is_wp_error', 'do_action', 'get_available_languages', 'wp_get_installed_translations', 'trailingslashit', 'wp_remote_retrieve_response_code', 'wp_remote_retrieve_body', 'untrailingslashit', 'get_stylesheet', 'wp_get_theme', 'get_theme_root', 'get_plugins', 'get_submit_button', 'wp_create_nonce', 'human_time_diff', 'get_option', 'wp_enqueue_style', 'wp_enqueue_script', 'esc_html', 'wp_remote_retrieve_response_message', 'wp_remote_retrieve_headers', 'check_ajax_referer', 'plugins_url', 'get_theme_root_uri', 'balanceTags', 'wp_kses', ], 'exclude-classes' => [ 'Puc_v4p11_DebugBar_PluginPanel', 'Puc_v4p11_DebugBar_ThemePanel', ], ];

YahnisElsts commented 2 years ago

I've made a new release that includes the patch, and so I'm closing the issue.