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

Branch

T15911divinerAtomStrlen

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1597
Build 1597: arc lint + arc unit

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