Page MenuHomePhorge

Fix Almanac page /almanac/interface/edit/1/
Closed, ResolvedPublic

Description

As reported by our kind @arnold we have a Crash from the Almanac page:

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

Regression introduced in:

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

rP1b08be518ef6219f93dcdad1e34c21855bcdecd4

Proposed fix

diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
index 3840bcdf9c..a22e4480a0 100644
--- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php
+++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
@@ -339,7 +339,8 @@ abstract class PhabricatorModularTransactionType
       $value = $xaction->getNewValue();
     }

-    return !phutil_nonempty_string($value);
+    $value_str = phutil_string_cast($value);
+    return $value_str === '';
   }

   /**

Note that casting is a good idea:

https://www.php.net/manual/en/language.types.string.php#language.types.string.casting

A bool true value is converted to the string "1". bool false is converted to "" (the empty string). This allows conversion back and forth between bool and string values.
An int or float is converted to a string representing the number textually (including the exponent part for floats). Floating point numbers can be converted using exponential notation (4.1E+6).
null is always converted to an empty string.

Note I don't want to use phutil_nonempty_scalar() since it's essentially broken (T15239) and D25117 still needs review since a month.

Event Timeline

valerio.bozzolan triaged this task as Unbreak Now! priority.May 12 2023, 16:06
valerio.bozzolan created this task.
valerio.bozzolan created this object in space S1 Public.

I was able to have that page working with:

<omissis - put in the description>

I start hating this framework btw

Definitely when I hear "at least it's a good PHP project" I can say: "no, used a lot of undocumented PHP features like strlen(null) in a massive way, that makes a bad PHP project :D" but don't worry, I do the same in my PHP projects. So I understand.

@valerio.bozzolan I can verify that the proposed patch works for me

Thanks. I've patched that blindly but now I'm also able to reproduce and I also can confirm this fixes the regression.

Hi @arnold please visit the patch D25220 and feel free to test again and share again your opinion (eventually marking that as "Accept Revision"), just to be 100% sure

Hi @aklapper could you please confirm that you are also affected by this? Then feel free to review D25220 - thanks