Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exceptions which block rendering the Maniphest task creation page
ClosedPublic

Authored by aklapper on Apr 30 2023, 09:02.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jul 21, 21:07
Unknown Object (File)
Thu, Jul 18, 04:16
Unknown Object (File)
Thu, Jul 18, 03:36
Unknown Object (File)
Sun, Jul 14, 01:59
Unknown Object (File)
Sun, Jul 14, 01:06
Unknown Object (File)
Sun, Jul 14, 01:06
Unknown Object (File)
Sun, Jul 14, 01:06
Unknown Object (File)
Sat, Jul 13, 22:50

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 T15302

Test Plan

Applied these five changes (on top of D25144, D25145, D25146, D25147, D25151)
and /maniphest/task/edit/form/default/ finally rendered in web browser.

Diff Detail

Repository
rP Phorge
Branch
D25152 (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 298
Build 298: arc lint + arc unit

Event Timeline

src/applications/transactions/editengine/PhabricatorEditEngine.php
981

✅ I tested this locally with phlog() in various pages and it seems $page_key is always null or string like "core". Any other alien value will be reported and this is OK.

1172

⚠️ It seems $template should accept integers or strings. So, phutil_nonempty_string() is probably not appropriate since with an integer it will throw an exception.

src/applications/transactions/editfield/PhabricatorEditField.php
421

✅ I tested this locally with phlog() in various pages and it seems $instructions is always null or string like "Set Full Domain URI if you plan to serve this blog on another hosted domain" etc. - Any other alien value will be reported and this is OK.

src/applications/transactions/editfield/PhabricatorTextEditField.php
21

✅ It seems the usages are just about null or strings. Alien values will be reported and this is OK.

Example usages:

./src/applications/harbormaster/query/HarbormasterBuildableSearchEngine.php:        ->setPlaceholder(pht('rXabcdef, PHID-DIFF-1234, ...'))
./src/applications/harbormaster/query/HarbormasterBuildableSearchEngine.php:        ->setPlaceholder(pht('rXYZ, R123, D456, ...'))
./src/applications/diffusion/controller/DiffusionRepositoryController.php:            ->setPlaceholder(pht('Locate File')));
./src/applications/auth/editor/PhabricatorAuthFactorProviderEditEngine.php:        ->setPlaceholder($factor_name),
./src/applications/calendar/editor/PhabricatorCalendarImportEditEngine.php:        ->setPlaceholder($object->getDisplayName())
./src/applications/home/menuitem/PhabricatorHomeProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/home/menuitem/PhabricatorHomeLauncherProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/people/menuitem/PhabricatorPeopleTasksProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/people/menuitem/PhabricatorPeopleCommitsProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/people/menuitem/PhabricatorPeopleManageProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/people/menuitem/PhabricatorPeopleBadgesProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/people/menuitem/PhabricatorPeopleRevisionsProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/people/menuitem/PhabricatorPeopleDetailsProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/dashboard/menuitem/PhabricatorDashboardPortalMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/transactions/editfield/PhabricatorTextEditField.php:  public function setPlaceholder($placeholder) {
./src/applications/transactions/editfield/PhabricatorTextEditField.php:      $control->setPlaceholder($placeholder);
./src/applications/search/field/PhabricatorSearchDateField.php:      ->setPlaceholder(pht('"2022-12-25" or "7 days ago"...'));
./src/applications/search/field/PhabricatorSearchStringListField.php:  public function setPlaceholder($placeholder) {
./src/applications/search/field/PhabricatorSearchStringListField.php:      $control->setPlaceholder($placeholder);
./src/applications/search/menuitem/PhabricatorManageProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/project/menuitem/PhabricatorProjectDetailsProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/project/menuitem/PhabricatorProjectReportsProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/project/menuitem/PhabricatorProjectSubprojectsProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/project/menuitem/PhabricatorProjectWorkboardProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/project/menuitem/PhabricatorProjectManageProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/project/menuitem/PhabricatorProjectMembersProfileMenuItem.php:        ->setPlaceholder($this->getDefaultName())
./src/applications/project/controller/trigger/PhabricatorProjectTriggerEditController.php:        ->setPlaceholder($trigger->getDefaultName()));
./src/applications/config/custom/PhabricatorCustomLogoConfigType.php:      ->setPlaceholder(PlatformSymbols::getPlatformServerName())
./src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php:      ->setPlaceholder($this->getPlaceholder());
./src/view/form/control/AphrontFormTokenizerControl.php:  public function setPlaceholder($placeholder) {
./src/view/form/control/AphrontFormTextControl.php:  public function setPlaceholder($placeholder) {
./src/view/form/control/AphrontFormTypeaheadControl.php:  public function setPlaceholder($placeholder)

I tested this locally, opening every kind of Engine (list / create / edit Engine, change defaults, use Form, etc.) and I was not able to find any problem.

Green light.

sgtm

src/applications/transactions/editengine/PhabricatorEditEngine.php
1172

OK I do not have a strong opinion about this since the getStr() method above just returns a string. So, green light here.

This revision is now accepted and ready to land.Apr 30 2023, 18:27