Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exceptions which block rendering the Dashboard page
ClosedPublic

Authored by aklapper on Apr 29 2023, 20:34.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 26, 12:45
Unknown Object (File)
Tue, Mar 26, 11:33
Unknown Object (File)
Tue, Mar 12, 15:11
Unknown Object (File)
Mon, Mar 11, 02:45
Unknown Object (File)
Feb 25 2024, 07:35
Unknown Object (File)
Feb 25 2024, 07:35
Unknown Object (File)
Feb 25 2024, 07:35
Unknown Object (File)
Feb 25 2024, 06:53

Details

Summary

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 T15295

Test Plan

Applied these four changes (on top of D25144 and D25145) and /dashboard/ finally rendered in web browser.

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

src/view/phui/PHUIInfoView.php
150

Interesting.

I cannot think about any case where $title is false but strlen($title) would be true

valerio.bozzolan added inline comments.
src/applications/search/engine/PhabricatorApplicationSearchEngine.php
182

✅ I used phlog() to test possible values of $order and it seems to me that it always is null or a string like "newest", "relevance", "oldest", etc.

The phutil_nonempty_string() will report alien types, and this is OK.

src/view/form/control/AphrontFormTokenizerControl.php
71

✅ I was not able to test a value with $this->placeholder != null, but it seems to me that the intended usage is with string.

The method phutil_nonempty_string() will report alien types, and this is OK.

src/view/phui/PHUIInfoView.php
156

✅ I tested $title locally with phlog() in various places and it seems it's always null or a string like "Issue Resolved" etc.

The phutil_nonempty_string() will report alien values, and that is OK.

This revision is now accepted and ready to land.May 1 2023, 13:32
valerio.bozzolan removed a reviewer: valerio.bozzolan. valerio.bozzolan added 1 blocking reviewer(s): O1: Blessed Committers.

hoping to be useful, adding some PHPDoc related to title, to clarify that it's not an HTML builder

This revision now requires review to proceed.May 1 2023, 13:32

Thanks!

Tested locally. Nothing explodes in PHP 7.4.

sgtm

src/applications/search/controller/PhabricatorApplicationSearchController.php
778

✅ I used phlog() to test possible values of $this->getQueryKey() and it seems to me that it always is null or a string like "all", "authored", "advanced", "I1bfBxtgm8kF", etc.

The phutil_nonempty_string() will report alien types, and this is OK.

This revision is now accepted and ready to land.May 1 2023, 13:35