Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exception which blocks rendering error pages for Packages' Create Package and Fund's Create Initiative
ClosedPublic

Authored by aklapper on May 6 2023, 23:13.

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 T15365

Test Plan

Applied this change and /packages/package/edit/form/default/ and /fund/create/ finally rendered in web browser, showing the expected error messages about not having entered any data in the creation page.

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

aklapper requested review of this revision.May 6 2023, 23:13

Thanks again

Tested this patch locally. I logged $value with phlog() while doing things and I can confirm it only contains the exact user input value of generic text inputs, so it should be always a string or null as we think.

Tested creating a Package, a Task, a Fund, an Herald Rule and a Project.

No implosions

The function phutil_nonempty_string() will report any alien value, and that is OK.

sgtm

This revision is now accepted and ready to land.May 8 2023, 19:25
arnold subscribed.

When editing almanac hosts, I get this: (url /almanac/interface/edit/1/)

2023/05/12 11:34:13 [error] 6844#6844: *51 FastCGI sent in stderr: "PHP message: [2023-05-12 08:34:13] EXCEPTION: (InvalidArgumentException) Call to phutil_nonempty_string() expected null or a string, got: int. at [<arcanist>/src/utils/utils.php:2127]; PHP message: arcanist(head=master, ref.master=c14785c3795c), phorge(head=master, ref.master=4d9719539773); PHP message: #0 <#2> phutil_nonempty_string(integer) called at [<phorge>/src/applications/transactions/storage/PhabricatorModularTransactionType.php:342]; PHP message: #1 <#2> PhabricatorModularTransactionType::isEmptyTextTransaction(integer, array) called at [<phorge>/src/applications/almanac/xaction/AlmanacInterfacePortTransaction.php:33]; PHP message: #2 <#2> AlmanacInterfacePortTransaction::validateTransactions(AlmanacInterface, array) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2871]; PHP message: #3 <#2> PhabricatorApplicationTransactionEditor::validateTransaction(AlmanacInterface, string, array) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1234]; PHP message: #4 <#2> PhabricatorApplicationTransactionEditor::applyTransactions(AlmanacInterface, array) called at [<phorge>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1149]; PHP message: #5 <#2> PhabricatorEditEngine::buildEditResponse(AlmanacInterface) called at [<phorge>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1000]; PHP message: #6 <#2> PhabricatorEditEngine::buildResponse() called at [<phorge>/src/applications/almanac/controller/AlmanacInterfaceEditController.php:32]; PHP message: #7 <#2> AlmanacInterfaceEditController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]; PHP message: #8 phlog(InvalidArgumentException) called at [<phorge>/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php:41]; PHP message: #9 PhabricatorDefaultRequestExceptionHan

Fantastic, the method is called isEmptyTextTransaction() but handles integers

Let's fix

src/applications/transactions/storage/PhabricatorModularTransactionType.php
342

Hi @arnold can you please test the above change? Does it fix to you?

src/applications/transactions/storage/PhabricatorModularTransactionType.php
342

Is there a way for me to copy that patch? I'm not sure how to do that despite using phabricator for years :)

I could manually make those changes, but I don't want to make typos as I copy it

src/applications/transactions/storage/PhabricatorModularTransactionType.php
342

Let's continue this discussion here:

T15385: Fix Almanac page /almanac/interface/edit/1/