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

PHP 8.1 compatibility - do not attempt to count() NULL #178

Closed
Detsieber opened this issue Oct 4, 2023 · 1 comment · Fixed by #180
Closed

PHP 8.1 compatibility - do not attempt to count() NULL #178

Detsieber opened this issue Oct 4, 2023 · 1 comment · Fixed by #180
Labels
bug status:fixed The issue has been resolved (usually by committing/merging code).
Milestone

Comments

@Detsieber
Copy link

Although donrec in general is working fine with PHP 8.0/8.1, in certain conditions we get a wsod.

This is caused by the php function count, that behaves differently with php 8:
https://www.php.net/manual/en/function.count.php

count() will now throw TypeError on invalid countable types passed to the value parameter.

I have prepared a pull request.

Detsieber added a commit to Detsieber/de.systopia.donrec that referenced this issue Oct 4, 2023
Detsieber added a commit to Detsieber/de.systopia.donrec that referenced this issue Oct 4, 2023
@jensschuppe
Copy link
Collaborator

If I get that correctly, you're talking about those two occurences of count() without a guard of the variable being countable:

CRM_Utils_System::url('civicrm/donrec/task', 'conflict=1' . '&sid=' . $result['snapshot']->getId() . '&ccount=' . count($this->_contactIds)));

and
CRM_Utils_System::url('civicrm/donrec/task', 'sid=' . $result['snapshot']->getId() . '&ccount=' . count($this->_contactIds))

and possibly more ...

… while I'm not sure what would cause $this->_contactIds not be an array. But the easiest solution would be to just call $this->getContactIDs() instead of direct access of $this->_contactIds, as the getter does a null-coalesce check with ?? [], which should be enough as the variable will be either an array or NULL.

Your PR, however, is filed against your fork, not the origin repo. I created #180 instead which should fix the issue for all occurences - would you be able to review that one, @Detsieber?

@jensschuppe jensschuppe changed the title PHP 8.1 compatibility (subtle...) PHP 8.1 compatibility - do not attempt to count() NULL Oct 4, 2023
@jensschuppe jensschuppe added bug status:needs review Code needs review and testing. labels Oct 4, 2023
@jensschuppe jensschuppe added this to the 2.3 milestone Oct 4, 2023
@jensschuppe jensschuppe added status:fixed The issue has been resolved (usually by committing/merging code). and removed status:needs review Code needs review and testing. labels Oct 11, 2023
@jensschuppe jensschuppe modified the milestones: 2.3, 2.2 Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug status:fixed The issue has been resolved (usually by committing/merging code).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants