[3.10] [PHP 8.1] Mysqli database driver escape function fix

[beat]

Pull Request for Issue # none.

Summary of Changes

Fixes Deprecated: mysqli_real_escape_string(): Passing null to parameter #2 ($string) of type string is deprecated in libraries/joomla/database/driver/mysqli.php on line 254

Testing Instructions

Source-code review should be enough here.

Just test that site continues to work.
I saw this error while saving a user in admin aera with CB.

Actual result BEFORE applying this Pull Request

Deprecated: mysqli_real_escape_string(): Passing null to parameter #2 ($string) of type string is deprecated in libraries/joomla/database/driver/mysqli.php on line 254

Expected result AFTER applying this Pull Request

That warning is not displayed

Documentation Changes Required

None.

[richard67]

I have tested this item ✅ successfully on a807845

Code review.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36787.}

[richard67]

@beat Does it need to be fixed in the other drivers, too?

And for 4.x it needs to make a PR with the same fix here: https://github.com/joomla-framework/database . Will you do that, or shall I?

[richard67]

And should there maybe be a type cast to string for the call to addcslashes in line 258?

Ah no, I see it's not necessary since already handled before.

[beat]

All my PHP 8.1 fixes should be checked if they need to also be done in other Joomla branches as well. Same also as you well asked and checked, if they should be done in other files as well.

At this time, I'm just testing Joomla and CB on PHP 8.1 in all functionality, and fixing in parallel in CB, CB add-ons and Joomla 3.10 and 4.0 things I see (just looking around the fix itself if nearby functions need the same fix). And I guess that I will have a few more days of work to do so at least, way more than I never had to do in a previous PHP new version since PHP 4!

PHP made this strange decision to deprecate null as string in ancient string functions, breaking many code that worked in 8.0. Actually, I think it's not a good decision, ancient string functions should be left alone, and a new string class, like in many OO languages be made native and implicit (e.g. "text".strlen()). Is there a PHP RFC or discussion for that ? is it too late to comment against this for removal in PHP 8.2 ? it will break unneededly a massive amount of PHP code, just because strlen() and other string functions accept only string and not ?string. Or is null value going to be removed alltogether in PHP 9 or10 ?

So, if you have time to check and fix if needed other branches and files, it would be awesome and then just comment in each fix that you checked it and result (as you did above for other functions), so that we all know. If not, I might do it at some time when I catched up other tasks as this takes way longer than planed. Thank you for your reviews and help in all cases 👍

[richard67]

I don't know if I have the time to check everything on other branches, but I can handle the database driver stuff.

[richard67]

@beat Please check beat#5 for the same for for the other db drivers.

[richard67]

For the framework see joomla-framework/database#260 .

[beat]

@beat Please check beat#5 for the same for for the other db drivers.

Looks good on code review 👍

[richard67]

I have tested this item ✅ successfully on 2bfda40

Code review.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36787.}

[alikon]

I have tested this item ✅ successfully on 2bfda40

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36787.}

[alikon]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36787.}

[zero-24]

Hmm I dont like that we try to cast stuff to strings where the original input should have been an string in the first place. So I would like to understand where the issue is comming from, do we have a stack trace?

But I guess we have to do it here to not break the int and float case right?

[richard67]

Hmm I dont like that we try to cast stuff to strings where the original input should have been an string in the first place. So I would like to understand where the issue is comming from, do we have a stack trace?

But I guess we have to do it here to not break the int and float case right?

@zero-24 I've worried especially about the int case, see the additional comment in the description of the PR here: joomla-framework/database#260 .

[zero-24]

Will merge this here as workaround for 3.10. But yes I agree with you that we should find a better solution upstream within J4

[beat]

@zero-24 here the stack trace (saving a user in admin with CB):

CB is calling the save() function without any parameters, so it's the defaults public function save($updateOnly = false).

Didn't see a less B/C-risky way to fix it.

[beat]

Oh, just saw it got merged. All ok then!