Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exception which blocks rendering "Browse Dashboards" dialog
ClosedPublic

Authored by aklapper on May 13 2023, 11:24.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 23, 02:47
Unknown Object (File)
Sun, Apr 21, 21:19
Unknown Object (File)
Sun, Apr 21, 14:41
Unknown Object (File)
Thu, Apr 18, 10:34
Unknown Object (File)
Wed, Apr 17, 06:17
Unknown Object (File)
Tue, Apr 16, 15:08
Unknown Object (File)
Wed, Apr 10, 20:15
Unknown Object (File)
Sun, Apr 7, 05:52

Details

Summary

strlen() was used in Phabricator to check if a generic value is a non-empty string.

But, that function does not accept null anymore since PHP 8.1.

Closes T15396

Test Plan

Applied this change (on top of D25179 and D25226) and the "Browse Dashboards" dialog got rendered in web browser.

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

valerio.bozzolan added inline comments.
src/applications/search/compiler/PhutilSearchQueryCompiler.php
107

Wow, finally a place where strlen() was really needed!

Please just do something like this before the strlen:

if($query === null) {
  $query = '';
}
This revision now requires changes to proceed.May 13 2023, 22:54

Uh, that unit test was useful indeed, and I know understand the rest of the code.

Thanks for your update

I tested this locally without any implosion even if I was honestly not able to reproduce a null case while visiting the /dashboard/ page. Anyway, this is 100% legit to me.

lgtm

(#WikimediaHackaton)

This revision is now accepted and ready to land.May 19 2023, 08:54