Fix PHP 8.1 "strlen(null)" exception which blocks rendering most applications' Configure pages
ClosedPublic
Actions

Authored by aklapper on May 6 2023, 12:57.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15358: PHP 8.1 "strlen(null)" exception blocks rendering most projects' Configure pages

Commits

rPd74e9d825a2c: Fix PHP 8.1 "strlen(null)" exception which blocks rendering most applications'…

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.

Added also a small inline documentation to better leave what I find.

Closes T15358

Test Plan

Applied this change change and way more applications' Configure pages finally rendered in web browser.
For example, /applications/view/PhabricatorAuditApplication/ is now correctly rendered.
(However, some pages expose followup exceptions to be handled in separate tasks.)

Diff Detail

Repository

rP Phorge

Branch

arcpatch-D25197

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 377
Build 377: arc lint + arc unit

Event Timeline

aklapper created this revision.May 6 2023, 12:57

Owners added a reviewer: O1: Blessed Committers.May 6 2023, 12:57

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptMay 6 2023, 12:57

Harbormaster completed remote builds in B369: Diff 673.May 6 2023, 12:57

aklapper requested review of this revision.May 6 2023, 12:57

aklapper mentioned this in T15359: PHP 8.1 "ltrim(null)" exception blocks rendering five applications' Configure pages.May 6 2023, 13:21

aklapper mentioned this in D25198: Fix PHP 8.1 "ltrim(null)" exception which blocks rendering five applications' Configure pages.May 6 2023, 13:22

Thanks for this patch (again)

I tested this locally against all launcher applications, no implosions.

The Overview is never an object.

Hoping to be useful I will also attach some minor related PHPDoc.

src/applications/meta/controller/PhabricatorApplicationDetailViewController.php
117	✅ I verified that the `$application->getOverview()` method only returns null or strings (raw remarkup). The method `phutil_nonempty_string()` will report any alien type, and that is OK here.