Fix PHP 8.1 "str_replace(null)" exception in DivinerAtomRef
ClosedPublic
Actions

Authored by aklapper on Aug 11 2024, 15:29.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15911: PHP 8.1 "str_replace(null)" exception in DivinerAtomRef

Commits

rP85f51c54303f: Fix PHP 8.1 "str_replace(null)" exception in DivinerAtomRef

Summary

Passing null instead of a string or array to str_replace() deprecated since PHP 8.1.

Thus do not create a title array with a null entry in DivinerFindController when there is no $query_text, later to be read via $this->titles in DivinerAtomRef.

ERROR 8192: str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated at [/var/www/html/phorge/phorge/src/applications/diviner/atom/DivinerAtomRef.php:205]
  #0 str_replace(string, string, NULL) called at [<phorge>/src/applications/diviner/atom/DivinerAtomRef.php:205]
  #1 DivinerAtomRef::normalizeTitleString(NULL) called at [<phorge>/src/applications/diviner/query/DivinerAtomQuery.php:344]

Credits to valerio.bozzolan for finding the right spot in the code.

Closes T15911

Test Plan

Go to http://phorge.localhost/diviner/find/ (not passing a name URI parameter), optionally with D25768 applied to avoid another exception

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.Aug 11 2024, 15:29

Owners added a reviewer: O1: Blessed Committers.Aug 11 2024, 15:29

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptAug 11 2024, 15:29

aklapper requested review of this revision.Aug 11 2024, 15:29

Harbormaster completed remote builds in B1513: Diff 2353.Aug 11 2024, 15:29

valerio.bozzolan added inline comments.Aug 19 2024, 12:47

src/applications/diviner/query/DivinerAtomQuery.php
344–346 ↗	(On Diff #2353)	I see that you want to skip the incoming `array(null)` to fix this: https://we.phorge.it/source/phorge/browse/master/src/applications/diviner/controller/DivinerFindController.php;66bd13b193b666e77c00ccea0fea508134c5397a$61 But we should probably skip all NULL values, not just the first one. For example we can do like the old line 360. In this way: we avoid other unexpected errors when somebody passes an array map without the key `0` (not the case of Phorge, but expected in external calls) we be nice for external calls with `array('something', null)` also to feel less hacky