Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PHP8.2 str_split function returns empty arrays for empty strings #588

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

HuongNV13
Copy link

In PHP 8.2, the str_split function will returns empty arrays for empty strings.
See: https://php.watch/versions/8.2/str_split-empty-string-empty-array

We can use mb_str_split() instead

@CLAassistant
Copy link

CLAassistant commented Feb 3, 2023

CLA assistant check
All committers have signed the CLA.

williamdes
williamdes previously approved these changes Feb 3, 2023
@fooman
Copy link

fooman commented Feb 10, 2023

Unfortunately mb_str_split() was only introduced in php7.4 which will make swapping this out trickier.

@williamdes
Copy link
Contributor

Unfortunately mb_str_split() was only introduced in php7.4 which will make swapping this out trickier.

Thanks for the hint!
Okay, some code needs to be added and use PHP_MAJOR_VERSION const and it's friends

@williamdes
Copy link
Contributor

or maybe just function_exists()

@HuongNV13 HuongNV13 force-pushed the php82-str-split-function-return branch from c5ed6b2 to ae4f2e8 Compare February 13, 2023 03:10
@HuongNV13
Copy link
Author

Thanks, everyone.
I have updated the code to use function_exists() to use the correct method.

@MacGritsch
Copy link

I think this is not a valid fix as mb_str_split() behaves the same way as str_split() in PHP 8.2 and above.
So why dont check if its an empty string (and if its needed in that case).
Otherwise you could also write a function str_split_old() which returns array('') if the input string is empty.

@williamdes williamdes mentioned this pull request Aug 2, 2023
@@ -638,7 +638,8 @@ public function __construct($code, $eclevel = 'L') {
$barcode_array['bcode'] = array();
foreach ($qrTab as $line) {
$arrAdd = array();
foreach (str_split($line) as $char) {
$chars = function_exists('mb_str_split') ? mb_str_split($line) : str_split($line);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like incorrect indentation, project uses tabs while this code introduces spaces. Same can be said about all other new lines in this MR.

@nicolaasuni
Copy link
Member

Please try to resolve the conflicts.
Could you please instead create a wrapper function in tcpdf_static.php?

@HuongNV13
Copy link
Author

Hi Nicola,
Sorry for the late reply, I was busy with my project.
I will update the patch soon.

Thanks,

@nicolaasuni
Copy link
Member

Hi,
If this is still an issue please try to create a wrapper function in tcpdf_static.php.
Alternatively please consider using the new tc-lib-pdf if you don't need HTML/CSS/SVG/Javascript features that are still in development.

@williamdes
Copy link
Contributor

If this is still an issue please try to create a wrapper function in tcpdf_static.php.

Very good idead
@HuongNV13 can you do it ?

This PR should be marked as draft for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants