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, Mar 26, 08:38
Unknown Object (File)
Tue, Mar 26, 08:38
Unknown Object (File)
Tue, Mar 26, 08:38
Unknown Object (File)
Tue, Mar 26, 07:57
Unknown Object (File)
Tue, Mar 26, 07:28
Unknown Object (File)
Sun, Mar 24, 06:41
Unknown Object (File)
Wed, Mar 20, 10:43
Unknown Object (File)
Feb 25 2024, 07:37

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
Branch
arcpatch-D25227
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 414
Build 414: arc lint + arc unit

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