froger-me / wp-packages-update-server

WP Packages Update Server - Run Your Own Update Server for Plugins and Themes
GNU General Public License v3.0
141 stars 39 forks source link

PHP 7.3 warning #19

Closed RFlector closed 4 years ago

RFlector commented 4 years ago

I try use intergration example - plugin. After activation I see error: <b>Deprecated</b>: strpos(): Non-string needles will be interpreted as strings in the future. Use an explicit chr() call to preserve the current behavior in <b>/var/www/flector/data/www/*****.ru/wp-content/plugins/dummy-plugin/lib/plugin-update-checker/Puc/v4p4/Plugin/UpdateChecker.php</b> on line <b>641</b><br> <br>

froger-me commented 4 years ago

Hi @RFlector ! Thanks for the comprehensive feedback.

Checking the aforementioned line in the Puc library, I see that theoretically the strpos call should be passed 2 strings - $pluginPath and $muPluginDir variables ; in that case the offender seems to be $pluginPath, as it's the needle. When trying to replicate the issue, I simply output the content of $pluginPath and got (obfuscated) /xx/xx/xxt/wp-content/plugins/dummy-plugin/dummy-plugin.php, which is obviously a string here. The warning occurs in PHP 7.3 in case this variable is, I imagine, null. I'm having a hard time figuring out from the top of my head how that could happen (need to dig the source code and crawl back up the method call chain), so I'll leave it for later. However, if null is interpreted as an empty string here, then this warning is harmless.

Does it prevent the integration example plugin from being activated with PHP 7.3? If not, I'll keep it low priority for now - updating the whole Puc library to the latest version might just fix it, who knows. If yes, I'll go in and dig deeper, and push a fix accordingly.

RFlector commented 4 years ago

The error is visible all the time in wp-admin pages after activation plugin.

Now I replace "plugin-update-checker" dir with current version from https://github.com/YahnisElsts/plugin-update-checker, and it's works without errors. But I don't know - so right or not?

froger-me commented 4 years ago

Great info! So updating the dependency would do it - thanks for that, I'll be able to fix it in here. I prefer to include everything here and not use tools like composer to make things easier for devs not familiar with it, so I'll do that manually in the coming days. As a side note: it shows the error on screen because your setup is configured to do so. In production environment, it's recommended to output them in a log file instead.