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

Authored by aklapper on Apr 30 2023, 09:02.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15302: PHP 8.1 "strlen(null)" exceptions block rendering Maniphest task creation page

Commits

rP313d3b7bf2ea: Fix PHP 8.1 "strlen(null)" exceptions which block rendering the Maniphest task…

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

aklapper created this revision.Apr 30 2023, 09:02

Owners added a reviewer: O1: Blessed Committers.Apr 30 2023, 09:02

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptApr 30 2023, 09:02

aklapper requested review of this revision.Apr 30 2023, 09:02

Harbormaster completed remote builds in B298: Diff 564.Apr 30 2023, 09:02

valerio.bozzolan added inline comments.Apr 30 2023, 18:07

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