Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exception which blocks rendering Notifications' Advanced Search page

Authored by aklapper on May 1 2023, 17:20.
Referenced Files
Unknown Object (File)
Thu, Jun 1, 11:56
Unknown Object (File)
Wed, May 31, 06:31
Unknown Object (File)
Tue, May 23, 23:13
Unknown Object (File)
Mon, May 22, 22:00
Unknown Object (File)
Sun, May 21, 22:07
Unknown Object (File)
Thu, May 18, 12:20



strlen() was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts phutil_nonempty_string() as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

Closes T15320

Test Plan

Applied this change and /notification/query/advanced/ rendered in web browser.

Diff Detail

rP Phorge
Lint Not Applicable
Tests Not Applicable

Event Timeline

aklapper requested review of this revision.May 1 2023, 17:20


Tested locally with / without the Notification checkbox. No implosions.

I also tested this on the advanced search on my Maniphest Tasks with custom boolean fields but that seems unrelated since the filters are "(Any)" and "(Require)" in that case. This confusing filter is not specific to this change, but it can be discussed here: T15177: Allow to query Tasks with a custom boolean field set to false

Ready to land.



✅ I tested $request->getStr($key) locally with phlog() and it seems it only assumes null or the string "1". It should never assume the integer 1 to me, since the method getStr() is supposed to cast to string.

The phutil_nonempty_string() will report any alien type (like integers), and that is OK.

This revision is now accepted and ready to land.May 1 2023, 19:45