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

configApp (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 369
Build 369: 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.