yiisoft / yii

Yii PHP Framework 1.1.x
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
4.84k stars 2.28k forks source link

fix PHP 8.2 error "Passing null to parameter is deprecated" #4528

Closed lubosdz closed 1 year ago

lubosdz commented 1 year ago
Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Tests pass? ✔️

Output buffer(s) may also be empty (containing NULL), which throws an error in PHP 8.2+.

what-the-diff[bot] commented 1 year ago

PR Summary

rob006 commented 1 year ago

Output buffer(s) may also be empty (containing NULL),

In what scenario it will be null?

lubosdz commented 1 year ago

@rob006 I really don't know. I was testing older Yii.1 online app on PHP 8.2.3 to see how costy it would to to upgrade. Out of all traced issues there was the only one caused by framework (Yii 1.1.28) - on line 414 (method CClientScript::renderHead()). Casting to string fixed the issue. Then I noted same preg_match() expressions in renderBodyBegin() and renderBodyEnd() and added to PR to potentially fix those too. I am not going to investigate details why was buffer NULL on line 414, no time for that. Feel free to close this PR if you think it's not justified or wait till someone confirms.

rob006 commented 1 year ago

These methods do not accept nulls according to phpdoc, so if we want to handle nulls here, we should update phpdoc too. But IMO it would be better to identify case when this is happening and ensure that output is string before we pass it to methods for output processing.

lubosdz commented 1 year ago

These methods do not accept nulls according to phpdoc.

Well, they are public methods with pointer argument :-) So technically anything can be passed in whatever the documentation says. CClientScript may also be extended and the method(s) modified. I agree it would be better to provide particular details how NULL was passed in, so if I have some time on hands I may look into that.

marcovtwout commented 1 year ago

Closing the PR as the issue seems to be in user code, otherwise feel free to reopen.