Page MenuHomePhorge

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

Authored by aklapper on Apr 29 2023, 13:14.
Tags
None
Referenced Files
F2231491: D25144.1718758582.diff
Tue, Jun 18, 00:56
Unknown Object (File)
Thu, Jun 13, 18:37
Unknown Object (File)
Thu, Jun 13, 15:42
Unknown Object (File)
Thu, Jun 13, 14:12
Unknown Object (File)
Thu, Jun 13, 13:25
Unknown Object (File)
Thu, Jun 13, 13:01
Unknown Object (File)
Thu, Jun 13, 05:34
Unknown Object (File)
Wed, Jun 12, 15:58

Details

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

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.

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 and /dashboard/ finally rendered in web browser.

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. :(

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.

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.

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

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.

This revision is now accepted and ready to land.May 1 2023, 08:50