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

Authored by aklapper on May 13 2023, 11:24.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15396: PHP 8.1 "strlen(null)" exception blocks rendering "Browse Dashboards" dialog

Commits

rP402a63c5de96: Fix PHP 8.1 "strlen(null)" exception which blocks rendering "Browse Dashboards"…

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

sigsDashb (branched from master)

Lint

Lint Passed

Unit

Test Failures

Build Status

Buildable 410
Build 410: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	1 ms	PhutilSearchQueryCompilerTestCase::testCompileQueries Assertion failed, expected values to be equal (at PhutilSearchQueryCompilerTestCase.php:268): Compilation of query: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx…
	107 ms	PhabricatorCelerityTestCase::testCelerityMaps 1 assertion passed.
	12 ms	PhabricatorConduitTestCase::testConduitMethods 1 assertion passed.
	0 ms	PhabricatorInfrastructureTestCase::testApplicationsInstalled 1 assertion passed.
	0 ms	PhabricatorInfrastructureTestCase::testRejectMySQLNonUTF8Queries 1 assertion passed.
		View Full Test Results (1 Failed · 11 Passed)

Event Timeline

aklapper created this revision.May 13 2023, 11:24

Owners added a reviewer: O1: Blessed Committers.May 13 2023, 11:24

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptMay 13 2023, 11:24

aklapper requested review of this revision.May 13 2023, 11:24

Harbormaster completed remote builds in B410: Diff 734.May 13 2023, 11:25

valerio.bozzolan requested changes to this revision.May 13 2023, 22:54

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.

Harbormaster completed remote builds in B414: Diff 738.May 13 2023, 23:59

valerio.bozzolan mentioned this in Wikimedia Hackaton 2023 Athens Code Sprint.May 18 2023, 10:16

valerio.bozzolan edited the summary of this revision. (Show Details)May 19 2023, 08:35

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