Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exception rendering dashboard panel with latest tasks when custom int field configured
ClosedPublic

Authored by aklapper on Dec 5 2023, 03:23.

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.

ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php:56]

Closes T15685

Test Plan

After configuring a custom int field and a dashboard panel to query and listed the latest created tasks, access the panel and check the PHP error log.

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

aklapper requested review of this revision.Dec 5 2023, 03:23
This revision is now accepted and ready to land.Dec 5 2023, 12:04

I think that the value may be an int, so, causing a nuclear implosion. Probably nice to check for a concrete populated example, and eventually do line 27 or similar ones.

speck requested changes to this revision.Dec 5 2023, 12:18

Good catch. I don’t think it would cause an implosion but might result in not applying constraints when it should

This revision now requires changes to proceed.Dec 5 2023, 12:18

Urgh, indeed not the brightest proposed patch (though it worked locally).
Also need to rework/expand patch because there are several more places in this very file to fix.
More stacktraces triggered by going to http://phorge.localhost/maniphest/ :

[2023-12-06 18:43:19] ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php:14]
arcanist(head=master, ref.master=e46025f7a914), phorge(head=master, ref.master=5ddca7da55e3)
  #0 PhabricatorStandardCustomFieldInt::buildFieldIndexes() called at [<phorge>/src/infrastructure/customfield/field/PhabricatorCustomField.php:664]
  #1 PhabricatorCustomField::buildFieldIndexes() called at [<phorge>/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php:268]
  #2 PhabricatorCustomFieldList::rebuildIndexes(ManiphestTask) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1539]
  #3 PhabricatorApplicationTransactionEditor::applyTransactions(ManiphestTask, array) called at [<phorge>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1149]
  #4 PhabricatorEditEngine::buildEditResponse(ManiphestTask) called at [<phorge>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1000]
  #5 PhabricatorEditEngine::buildResponse() called at [<phorge>/src/applications/maniphest/controller/ManiphestTaskEditController.php:12]
  #6 ManiphestTaskEditController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]
  #7 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:203]
  #8 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phorge>/webroot/index.php:35]

[2023-12-06 18:43:19] ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php:87]
arcanist(head=master, ref.master=e46025f7a914), phorge(head=master, ref.master=5ddca7da55e3)
  #0 PhabricatorStandardCustomFieldInt::validateApplicationTransactions(ManiphestTransactionEditor, string, array) called at [<phorge>/src/infrastructure/customfield/field/PhabricatorCustomField.php:1010]
  #1 PhabricatorCustomField::validateApplicationTransactions(ManiphestTransactionEditor, string, array) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2923]
  #2 PhabricatorApplicationTransactionEditor::validateTransaction(ManiphestTask, string, array) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1234]
  #3 PhabricatorApplicationTransactionEditor::applyTransactions(ManiphestTask, array) called at [<phorge>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1149]
  #4 PhabricatorEditEngine::buildEditResponse(ManiphestTask) called at [<phorge>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1000]
  #5 PhabricatorEditEngine::buildResponse() called at [<phorge>/src/applications/maniphest/controller/ManiphestTaskEditController.php:12]
  #6 ManiphestTaskEditController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]
  #7 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:203]
  #8 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phorge>/webroot/index.php:35]

[2023-12-06 18:43:19] ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php:107]
arcanist(head=master, ref.master=e46025f7a914), phorge(head=master, ref.master=5ddca7da55e3)
  #0 PhabricatorStandardCustomFieldInt::getApplicationTransactionHasEffect(ManiphestTransaction) called at [<phorge>/src/infrastructure/customfield/field/PhabricatorCustomField.php:917]
  #1 PhabricatorCustomField::getApplicationTransactionHasEffect(ManiphestTransaction) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:679]
  #2 PhabricatorApplicationTransactionEditor::transactionHasEffect(ManiphestTask, ManiphestTransaction) called at [<phorge>/src/applications/maniphest/editor/ManiphestTransactionEditor.php:69]
  #3 ManiphestTransactionEditor::transactionHasEffect(ManiphestTask, ManiphestTransaction) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2788]
  #4 PhabricatorApplicationTransactionEditor::filterTransactions(ManiphestTask, array) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1283]
  #5 PhabricatorApplicationTransactionEditor::applyTransactions(ManiphestTask, array) called at [<phorge>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1149]
  #6 PhabricatorEditEngine::buildEditResponse(ManiphestTask) called at [<phorge>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1000]
  #7 PhabricatorEditEngine::buildResponse() called at [<phorge>/src/applications/maniphest/controller/ManiphestTaskEditController.php:12]
  #8 ManiphestTaskEditController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]
  #9 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:203]
  #10 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phorge>/webroot/index.php:35]

[2023-12-06 18:43:19] ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php:109]
arcanist(head=master, ref.master=e46025f7a914), phorge(head=master, ref.master=5ddca7da55e3)
  #0 PhabricatorStandardCustomFieldInt::getApplicationTransactionHasEffect(ManiphestTransaction) called at [<phorge>/src/infrastructure/customfield/field/PhabricatorCustomField.php:917]
  #1 PhabricatorCustomField::getApplicationTransactionHasEffect(ManiphestTransaction) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:679]
  #2 PhabricatorApplicationTransactionEditor::transactionHasEffect(ManiphestTask, ManiphestTransaction) called at [<phorge>/src/applications/maniphest/editor/ManiphestTransactionEditor.php:69]
  #3 ManiphestTransactionEditor::transactionHasEffect(ManiphestTask, ManiphestTransaction) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2788]
  #4 PhabricatorApplicationTransactionEditor::filterTransactions(ManiphestTask, array) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1283]
  #5 PhabricatorApplicationTransactionEditor::applyTransactions(ManiphestTask, array) called at [<phorge>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1149]
  #6 PhabricatorEditEngine::buildEditResponse(ManiphestTask) called at [<phorge>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1000]
  #7 PhabricatorEditEngine::buildResponse() called at [<phorge>/src/applications/maniphest/controller/ManiphestTaskEditController.php:12]
  #8 ManiphestTaskEditController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]
  #9 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:203]
  #10 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phorge>/webroot/index.php:35]

Still WIP, was just checking that nothing explodes locally (yet)

Useful riepilogue for wild integers

$v(bool)strlen($v)phutil_nonempty_scalar($v)is_int($v) && $v !== null(int) $v(int) phutil_string_cast($v)
nullfalsefalsefalse00
""falsefalsefalse00
1truetruetrue11
"lol"truetruefalse00
"1"truetruefalse11
1.1truetruefalse11
"1.1"truetruefalse11
{"1"}truetruefalseexception1
{""}falsefalsefalseexception0
NOTE: With {"1"} I mean an object with a __toString() that returns "1" - like class One { function __toString() { return "1"; } }
NOTE: With {""} I mean an object with a __toString() that returns "" - like class None { function __toString() { return ""; } }

In short, for wild integers, the is_int() is not OK because of it excludes "1". Maybe is_numeric() but I have not tested. Maybe just:

FromTo
if (strlen($v))if (phutil_nonempty_scalar($v))
if (!strlen($v))if (!phutil_nonempty_scalar($v))

Edited: note that is_int($v) && $v !== null is exactly the same as is_int($v)

Play it safe by using phutil_nonempty_scalar() and do not assume that integers are actually integers (because "1" and such)

Whops, forgot to approve this. Sorry! And thanks.

I was able to test this with custom int fields, successfully.

This revision is now accepted and ready to land.Feb 12 2024, 12:23