q2a / question2answer

Question2Answer is a free and open source platform for Q&A sites, running on PHP/MySQL.
http://www.question2answer.org/
GNU General Public License v3.0
1.63k stars 628 forks source link

Fix many string management functions in PHP 8.1 #948

Closed pupi1985 closed 1 year ago

pupi1985 commented 2 years ago

PHP 8.1 string management functions expect strings as parameters. Q2A was sending lots of null values, which generates deprecation warnings such as:

Deprecated: strlen(): Passing null to parameter #1 ($string) of type string is deprecated in <base-path>/qa-include/qa-base.php on line 722

I took the time to review most (not all because I'm sure I must have missed something) these functions and made them more PHP 8.1 friendly.

BTW, this should also fix #934.

PS: This is expecting PHP version support to go up to PHP 7.0 due to the null-coalesce operator usage. I can make a fall back to 5.x but considering this PR is fixing issues with PHP 8.1, keeping PHP 5.x compatible code seems irrelevant at this point in time.

QROkes commented 2 years ago

Hi Pupi, I'm using your bugfix branch (PHP 8.1) and found these in server logs: (Thanks a lot in advance)

2022/08/05 10:23:49 [error] 2685988#2685988: *1264760 FastCGI sent in stderr: "PHP message: PHP Deprecated: strtolower(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/pages/admin/admin-default.php on line 36" while reading response header from upstream, client: 2806:1016:f:4c19:b470:4992:2e21:4d14, server: example.com, request: "GET /support/admin HTTP/2.0", upstream: "fastcgi://127.0.0.1:9000", host: "example.com", referrer: "https://example.com/support/"

2022/08/05 10:23:51 [error] 2685988#2685988: *1264760 FastCGI sent in stderr: "PHP message: PHP Deprecated: strip_tags(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/qa-theme-base.php on line 298" while reading response header from upstream, client: 2806:1016:f:4c19:b470:4992:2e21:4d14, server: example.com, request: "GET /support/index.php/url/test/%24%26-_~%23%25%5C%40%5E%2A%28%29%5D%5B%60%27%3B%3D%3A%7C%22.%7B%7D%2C%21%3C%3E%3F%23+%CF%80%C2%A7%C2%BD%D0%96%D7%A9?dummy=&param=%24%26-_%7E%23%25%5C%40%5E%2A%28%29%5D%5B%60%27%3B%3D%3A%7C%22.%7B%7D%2C%21%3C%3E%3F%23+%CF%80%C2%A7%C2%BD%D0%96%D7%A9 HTTP/2.0", upstream: "fastcgi://127.0.0.1:9000", host: "example.com", referrer: "https://example.com/support/admin/general"

2022/08/05 10:26:16 [error] 2685988#2685988: *1264926 FastCGI sent in stderr: "PHP message: PHP Deprecated: explode(): Passing null to parameter #2 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/qa-theme-base.php on line 1907" while reading response header from upstream, client: 52.72.245.71, server: example.com, request: "GET /support/1420/we-need-your-opinion?show=1422 HTTP/1.1", upstream: "fastcgi://127.0.0.1:9000", host: "example.com"

2022/08/05 10:33:11 [error] 2685988#2685988: *1265287 FastCGI sent in stderr: "PHP message: PHP Deprecated: strcmp(): Passing null to parameter #2 ($string2) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/app/format.php on line 86PHP message: PHP Deprecated: strcmp(): Passing null to parameter #2 ($string2) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/app/format.php on line 86PHP message: PHP Deprecated: strcmp(): Passing null to parameter #2 ($string2) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/app/format.php on line 86PHP message: PHP Deprecated: strcmp(): Passing null to parameter #2 ($string2) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/app/format.php on line 86" while reading response header from upstream, client: 5.161.128.49, server: example.com, request: "GET /support/208/hit-to-migrate HTTP/1.1", upstream: "fastcgi://127.0.0.1:9000", host: "example.com"

pupi1985 commented 2 years ago

Re-download the branch and try again. Or apply these changes.

QROkes commented 2 years ago

Thanks, Pupi! Bugfix branch is still not updated with the latest changes, but it can be easily applied manually and still getting this error:

2022/08/05 14:17:53 [error] 2685988#2685988: *1277227 FastCGI sent in stderr: "PHP message: PHP Deprecated: strip_tags(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/qa-theme-base.php on line 298" while reading response header from upstream, client: 2806:1016:f:4c19:b470:4992:2e21:4d14, server: webinoly.com, request: "GET /support/index.php/url/test/%24%26-_~%23%25%5C%40%5E%2A%28%29%5D%5B%60%27%3B%3D%3A%7C%22.%7B%7D%2C%21%3C%3E%3F%23+%CF%80%C2%A7%C2%BD%D0%96%D7%A9?dummy=&param=%24%26-_%7E%23%25%5C%40%5E%2A%28%29%5D%5B%60%27%3B%3D%3A%7C%22.%7B%7D%2C%21%3C%3E%3F%23+%CF%80%C2%A7%C2%BD%D0%96%D7%A9 HTTP/2.0", upstream: "fastcgi://127.0.0.1:9000", host: "example.com", referrer: "https://example.com/support/admin/general"

And a new one: 2022/08/05 14:22:59 [error] 2685988#2685988: *1277505 FastCGI sent in stderr: "PHP message: PHP Deprecated: strcmp(): Passing null to parameter #2 ($string2) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/pages/question-view.php on line 201PHP message: PHP Deprecated: strcmp(): Passing null to parameter #2 ($string2) of type string is deprecated in /var/www/qrokes.com/htdocs/support/qa-include/pages/question-view.php on line 201" while reading response header from upstream, client: 2806:1016:f:4c19:b470:4992:2e21:4d14, server: example.com, request: "GET /support/208/failing-to-migrate-site-with-subdomain?show=208 HTTP/2.0", upstream: "fastcgi://127.0.0.1:9000", host: "example.com", referrer: "https://example.com/support/search?q=migrate"

pupi1985 commented 2 years ago

Note you're commenting on the patch-153 branch. Not the bugfix branch in my fork. I hadn't updated my fork's branch by the time I posted my comment but it is updated by now. Try again and let me know if you face more issues

QROkes commented 2 years ago

Thanks Pupi!

QROkes commented 2 years ago

Another one:

2022/08/06 17:06:22 [error] 2685988#2685988: *1342603 FastCGI sent in stderr: "PHP message: PHP Deprecated: mb_strtolower(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/util/string.php on line 765PHP message: PHP Deprecated: mb_strtolower(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/util/string.php on line 765" while reading response header from upstream, client: 2806:1016:f:5966:25fd:c1b6:6f29:c77, server: example.com, request: "POST /support/8332/ssl-does-not-generate HTTP/2.0", upstream: "fastcgi://127.0.0.1:9000", host: "example.com", referrer: "https://example.com/support/8332/ssl-does-not-generate?state=close"

pupi1985 commented 2 years ago

Download and retest :) Luckily, the referrer had an interesting "state=close" hint. Otherwise, I wouldn't have known how to replicate it

QROkes commented 2 years ago

Two more:

2022/08/07 22:58:54 [error] 744#744: *21247 FastCGI sent in stderr: "PHP message: PHP Deprecated: strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/app/format.php on line 1175" while reading response header from upstream, client: 2806:1016:f:5966:25fd:c1b6:6f29:d57, server: example.com, request: "GET /support/user/name HTTP/2.0", upstream: "fastcgi://127.0.0.1:9000", host: "example.com", referrer: "https://example.com/support/8176/how-downgrade?show=8201"

2022/08/07 22:49:57 [error] 744#744: *20751 FastCGI sent in stderr: "PHP message: PHP Deprecated: explode(): Passing null to parameter #2 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/qa-base.php on line 1575" while reading response header from upstream, client: 2806:1016:f:5966:25fd:c1b6:6f29:d57, server: example.com, request: "POST /support/eventnotify HTTP/2.0", upstream: "fastcgi://127.0.0.1:9000", host: "example.com", referrer: "https://example.com/support/"

pupi1985 commented 2 years ago

First one is fixed. Second one is from the on-site notifications plugin.

Try removing this line.

Or maybe turning it into header('Access-Control-Allow-Origin: ' . qa_path_absolute(''));

QROkes commented 2 years ago

I know the second one is triggered by a plugin, but the issue can be fixed here: /qa-include/qa-base.php on line 1575, maybe is not the optimal solution but I easily fixed it with $requestparts = explode('/', $request ?? '');

Thanks Pupi!

pupi1985 commented 2 years ago

Think of this as pure math. Logarithm function log(num). The input is a number and the result is a number. Does it make sense to change Math so that the original value can be null? I'd say it makes more sense not to modify the math but rather correct the original nonsense request :)

QROkes commented 2 years ago

Couldn't agree more! That's why I said that it might not be the most optimal solution. But, as developers, we sometimes need to prevent some user simple errors, in this case, plugin devs. Mature software should prevent and handle any possible use case.

Always assume that the other people will always use your software in the worst possible way!

QROkes commented 2 years ago

More and more... The event-logger plugin is included in core by default. Thanks, Pupi!

2022/08/09 03:10:33 [error] 756#756: *23126 FastCGI sent in stderr: "PHP message: PHP Deprecated: preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in /var/www/example.com/htdocs/support/qa-include/vendor/PHPMailer/class.phpmailer.php on line 874" while reading response header from upstream, client: 91.222.236.231, server: example.com, request: "POST /support/register?to= HTTP/1.1", upstream: "fastcgi://127.0.0.1:9000", host: "example.com", referrer: "https://example.com/support/register?to="

2022/08/09 03:20:12 [error] 756#756: *23546 FastCGI sent in stderr: "PHP message: PHP Deprecated: strtolower(): Passing null to parameter #1 ($string) of type string is deprecated in
/var/www/example.com/htdocs/support/qa-include/pages/login.php on line 79 PHP message: PHP Deprecated: substr(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/db/users.php on line 38PHP message: PHP Deprecated: substr(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/db/users.php on line 38" while reading response header from upstream, client: 91.222.236.231, server: example.com, request: "POST /support/login?to= HTTP/1.0", upstream: "fastcgi://127.0.0.1:9000", host: "example.com", referrer: "https://example.com/support/"

2022/08/09 03:20:16 [error] 756#756: *23554 FastCGI sent in stderr: "PHP message: PHP Deprecated: preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in /var/www/example.com/htdocs/support/qa-include/vendor/PHPMailer/class.phpmailer.php on line 874 PHP message: PHP Deprecated: strtr(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-plugin/event-logger/qa-event-logger.php on line 157PHP message: PHP Deprecated: strtr(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-plugin/event-logger/qa-event-logger.php on line 157PHP message: PHP Deprecated: strtr(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-plugin/event-logger/qa-event-logger.php on line 157PHP message: PHP Deprecated: strtr(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-plugin/event-logger/qa-event-logger.php on line 157PHP message: PHP Deprecated: strtr(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-plugin/event-logger/qa-event-logger.php on line 157PHP message: PHP Deprecated: strtr(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-plugin/event-logger/qa-event-logger.php on line 157" while reading response header from upstream, client: 91.222.236.231, server: example.com, request: "POST /support/ask HTTP/1.0", upstream: "fastcgi://127.0.0.1:9000", host: "example.com", referrer: "https://example.com/support/ask"

2022/08/09 07:03:31 [error] 756#756: *32808 FastCGI sent in stderr: "PHP message: PHP Deprecated:  preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in /var/www/example.com/htdocs/support/qa-include/vendor/PHPMailer/class.phpmailer.php on line 874" while reading response header from upstream, client: 85.202.195.111, server: example.com, request: "POST /support/register?to= HTTP/1.1", upstream: "fastcgi://127.0.0.1:9000", host: "example.com", referrer: "https://example.com/support/register?to="

2022/08/09 07:41:44 [error] 756#756: *34408 FastCGI sent in stderr: "PHP message: PHP Deprecated:  strtr(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-plugin/event-logger/qa-event-logger.php on line 157PHP message: PHP Deprecated:  strtr(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-plugin/event-logger/qa-event-logger.php on line 157PHP message: PHP Deprecated:  strtr(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-plugin/event-logger/qa-event-logger.php on line 157PHP message: PHP Deprecated:  strtr(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-plugin/event-logger/qa-event-logger.php on line 157" while reading response header from upstream, client: 2806:1016:f:5966:d97f:160e:7ed0:c61, server: example.com, request: "POST /support/user/Breanna28340 HTTP/2.0", upstream: "fastcgi://127.0.0.1:9000", host: "example.com", referrer: "https://example.com/support/user/Breanna"
pupi1985 commented 2 years ago

It seems PHPMailer has their own issues as well. I had to upgrade the library. Bad thing is that this is a breaking change on a bugfix branch. So the approach here was to keep the original library and add the new one. So any plugin using the original library will show the warning. Good thing is that those plugins' code can be updated not to show the warning, though.

QROkes commented 2 years ago

A new one: 2022/08/10 20:12:26 [error] 756#756: *128308 FastCGI sent in stderr: "PHP message: PHP Deprecated: urldecode(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/example.com/htdocs/support/qa-include/qa-index.php on line 79" while reading response header from upstream, client: 2806:1016:f:90de:8921:d9d0:576e:777f, server: example.com, request: "GET /support/399/mysql-db-password-reset? HTTP/2.0", upstream: "fastcgi://127.0.0.1:9000", host: "example.com"

pupi1985 commented 2 years ago

Fixed!

QROkes commented 2 years ago

Another one:

2022/08/13 02:14:43 [error] 675#675: *44095 FastCGI sent in stderr: "PHP message: PHP Warning: Trying to access array offset on value of type null in /var/www/example.com/htdocs/support/qa-include/pages/user-profile.php on line 49" while reading response header from upstream, client: 66.249.64.245, server: example.com, request: "GET /support/user/drimed HTTP/1.1", upstream: "fastcgi://127.0.0.1:9000", host: "example.com"

pupi1985 commented 2 years ago

Fixed!

QROkes commented 2 years ago

Just for the record: almost 2 weeks with zero errors, seems safe to say that now 8.1 is fully supported. Thanks @pupi1985

svivian commented 1 year ago

Thanks for this, and sorry it's taken so long to get around to merging.

I'll look into making a new release. Will have to be v1.9 since we've changed the minimum PHP requirement. Merging with the dev branch might be an issue too with so many changes... :)

pupi1985 commented 1 year ago

Keeping the fixes in bugfix goes against semantic versioning while merging them in the dev branch, aside from the additional work per se, will mean delaying the bugfixes that result in critical usability issues even more.

The middle ground would be tagging 1.9 in the bugfix branch and releasing it with no delay. Then rebasing dev on top of the 1.9 tag. The painful merge will still be needed, we can't avoid it, but at least the merge doesn't block the release with the fixes. Even though 1.9 wouldn't contain new features, it would provide support for PHP 8.

svivian commented 1 year ago

If it's just the ?? operator, I can do a find/replace in the final release package. They just won't be in the git repo.

pupi1985 commented 1 year ago

I think that was the only breaking change. Note I've used it because it has been used in some previous commit in this branch