Problem/Motivation
We have noticed in our site that, after updating to 2.1.0, the attributes we were adding to the CAS response have stopped working and the following error is on the code:
Warning: foreach() argument must be of type array|object, true given in Drupal\cas_server\Controller\TicketValidationController->generateSuccessResponse() (line 383 of /var/www/html/web/modules/contrib/cas_server/src/Controller/TicketValidationController.php)
debuggin the error: we see that in L382 of the mentioned file we have:
$value = is_array($value) ?: [$value];
so, if we are passing to the attributes (eg: via a custom alter event, like we do), certain values, if the values is not an array will be OK (cause then $value will be an array) BUT if $value is already an array, then it will be converted to bool (cause is_array will return true instead of $value)
Steps to reproduce
Test with some additional attributes of type array; those will be removed and the error will raise
Proposed resolution
Change L382 to:
$value = is_array($value) ? $value : [$value];
(will do the MR in a bit)
Remaining tasks
- ✅ File an issue
- ✅ Addition/Change/Update/Fix
- ✅ Testing to ensure no regression
- ✅ Automated unit testing coverage
- ✅ Automated functional testing coverage
- ➖ UX/UI designer responsibilities
- ➖ Readability
- ➖ Accessibility
- ➖ Performance
- ➖ Security
- ➖ Documentation
- ✅ Code review by maintainers
- ➖ Full testing and approval
- ➖ Credit contributors
- ➖ Review with the product owner
- ➖ Release notes snippet
- ❌ Release
Issue fork cas_server-3521968
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
gorkagr commentedMR with the change created, can be reviewed :)
Comment #4
elc commentedWhoops, my bad. That certainly fixes the problem. Are you able to run with a patched version for now?
It very much sounds like we should add a test to trigger this same situation to prevent such a fault again.
Comment #5
gorkagr commentedYes, deploying the patch on the MR right now on our site :)
that will do the trick :D
Comment #6
elc commentedCould only think to add an array to the attributes. Is there anything else to alter into the attributes to press the limits?
Comment #9
elc commented