Paths

Table of Contentst

Differential D25144

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

Authored by aklapper on Apr 29 2023, 13:14.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15294: PHP 8.1 "strlen(null)" exceptions block rendering the Applications page

Commits

rP788aa453a2c0: Fix numerous PHP 8.1 "strlen(null)" exceptions which block rendering the…

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 T15294

Test Plan

Applied these three changes and /applications/ 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, 13:14

Owners added a reviewer: O1: Blessed Committers.Apr 29 2023, 13:14

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

aklapper requested review of this revision.Apr 29 2023, 13:14

Harbormaster completed remote builds in B287: Diff 552.Apr 29 2023, 13:14

Fix numerous 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.

Closes T15295

Test Plan:
Applied these four changes and /dashboard/ finally rendered in web browser.

Harbormaster completed remote builds in B288: Diff 553.Apr 29 2023, 13:34

Garr, I screwed up again with branches. Diff 552 is correct. Diff 553 is wrong and includes also stuff for T15295. Sorry, arc has... a learning curve for me. :(

aklapper abandoned this revision.Apr 29 2023, 19:56

In D25144#4582, @aklapper wrote:

Garr, I screwed up again with branches. Diff 552 is correct. Diff 553 is wrong and includes also stuff for T15295. Sorry, arc has... a learning curve for me. :(

No problem, if 552 is good, just download it again:

arc patch --diff 552

And, to update this D25144, just upload that again:

arc diff --update D25144

Fix numerous PHP 8.1 "strlen(null)" exceptions which block rendering the Applications 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.

Closes T15294

Test Plan:
Applied these three changes and /applications/ finally rendered in web browser.

Harbormaster completed remote builds in B290: Diff 556.Apr 29 2023, 20:18

@valerio.bozzolan: Thanks. At least I understood what I did wrong. :P New revision up; local workflow notes improved...

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 accepted this revision.May 1 2023, 08:50

valerio.bozzolan added inline comments.

src/applications/meta/query/PhabricatorAppSearchEngine.php
48	✅ I logged `$name` locally with `phlog()` in various page and I can confirm we can assume it as string or null. It assumes string values like `"Test test"` or `""` if you use the Name Contains field with these values. The `phutil_nonempty_string()` will report alien values and this is OK.
src/applications/search/controller/PhabricatorApplicationSearchController.php
118	✅ I logged `$this->queryKey` locally with `phlog()` in various page and I can confirm we can assume it as string or null. It assumes string values like `"authored"`, `"all"`, `"launcher"`, `"advanced"`, `"gPjpA9kNnigI"` etc. The `phutil_nonempty_string()` will report alien values and this is OK.
src/view/layout/AphrontSideNavFilterView.php
148	✅ I logged `$key` locally with `phlog()` in various page and I can confirm we can assume it as string or null. It assumes string values like `"query/authored"`, `"query/all"`, `"item(0)"`, `"query/ghj7xJtuslxH"` etc. The `phutil_nonempty_string()` will report alien values and this is OK.