Fix numerous PHP 8.1 "strlen(null)" exceptions trying access Configuration page
ClosedPublic
Actions

Authored by aklapper on Apr 28 2023, 17:49.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15287: Numerous PHP 8.1 "strlen(null)" exceptions trying to go to Configuration page

Commits

rPc37412df45da: Fix numerous PHP 8.1 "strlen(null)" exceptions trying access Configuration page

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 your phutil_nonempty_string() throws an exception, just
report it to Phorge to evaluate and fix together that specific corner case.

Closes T15287

Test Plan

Applied these three changes and /config/ finally rendered in web browser.

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.Apr 28 2023, 17:49

Owners added a reviewer: O1: Blessed Committers.Apr 28 2023, 17:49

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptApr 28 2023, 17:49

aklapper requested review of this revision.Apr 28 2023, 17:49

Harbormaster completed remote builds in B282: Diff 545.Apr 28 2023, 17:50

sgtm

src/applications/config/controller/PhabricatorConfigConsoleController.php
88	✅ This is safe to be considered NULL or a string since `$branchpoint` defined by `loadVersion()` is so. Alien values will be reported and that is OK.
95	✅ This is safe to be considered NULL or a string since `$info['hash']` defined by private `loadVersion()` is so. Alien values will be reported and that is OK.
263	✅ Noted: `$hash` is a string since it's a parsing of the `stdout`.
280	✅ Noted: `$branchpoint` is a string
src/view/control/AphrontTableView.php
138	✅ I've manually checked 30+ files grepping for `setColumnClasses()` and I'm damn 99.999% sure that all the incoming values should be just NULL, string, or an empty string. Aliens values will be reported and that's OK.