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

Authored by aklapper on Apr 29 2023, 20:34.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15295: PHP 8.1 "strlen(null)" exceptions block rendering the Dashboard page

Commits

rPbccd4f5981a7: Fix PHP 8.1 "strlen(null)" exceptions which block rendering the Dashboard 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 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

aklapper created this revision.Apr 29 2023, 20:34

Owners added a reviewer: O1: Blessed Committers.Apr 29 2023, 20:34

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptApr 29 2023, 20:34

aklapper requested review of this revision.Apr 29 2023, 20:34

Harbormaster completed remote builds in B291: Diff 557.Apr 29 2023, 20:34

valerio.bozzolan added inline comments.Apr 29 2023, 23:56

src/view/phui/PHUIInfoView.php
150	Interesting. I cannot think about any case where `$title` is false but `strlen($title)` would be true

aklapper mentioned this in D25150: Fix PHP 8.1 "strlen(null)" exceptions which block rendering the Diffusion page.Apr 30 2023, 08:08

aklapper mentioned this in rP1b8294341916: Fix PHP 8.1 "strlen(null)" exceptions which block rendering the Diffusion page.Apr 30 2023, 13:13

valerio.bozzolan mentioned this in D25161: Fix PHP 8.1 "strlen(null)" exception which blocks rendering Upload SSH Public key settings dialog.Apr 30 2023, 22:52

aklapper mentioned this in rPe3ad37f79259: Fix PHP 8.1 "strlen(null)" exception which blocks rendering Upload SSH Public….Apr 30 2023, 23:08

valerio.bozzolan mentioned this in D25152: Fix PHP 8.1 "strlen(null)" exceptions which block rendering the Maniphest task creation page.May 1 2023, 08:16

valerio.bozzolan mentioned this in D25162: Fix PHP 8.1 "strlen(null)" exception which blocks rendering list of Active/All projects.May 1 2023, 08:21

valerio.bozzolan mentioned this in D25153: Fix PHP 8.1 "strlen(null)" exception which blocks rendering the Projects page (and log alien values).May 1 2023, 08:37

valerio.bozzolan mentioned this in D25147: Fix PHP 8.1 "strlen(null)" exceptions which block rendering the People page.May 1 2023, 09:06

valerio.bozzolan edited the test plan for this revision. (Show Details)May 1 2023, 09:18

valerio.bozzolan accepted this revision.May 1 2023, 13:32

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

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

Harbormaster completed remote builds in B317: Diff 590.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.