Fix PHP 8.1 "strlen(null)" exceptions editing a form when custom field of type Date exists
ClosedPublic
Actions

Authored by aklapper on Aug 14 2023, 20:01.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15601: PHP 8.1 "strlen(null)" exceptions editing a form when custom field of type Date exists

Commits

rPcef12d8dc202: Fix PHP 8.1 "strlen(null)" exceptions editing a form when custom field of type…

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_scalar() as a replacement when both string and integers could be passed as a value like here.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_scalar() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

Also fix similar warning for ctype_digit(): Argument of type null will be interpreted as string in the future by checking for null first.

EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=customOAuthUrlencodeNull, ref.master=788098096e11, ref.customOAuthUrlencodeNull=4f0f2043b7e9), phorge(head=master, ref.master=bcfcd9acfc12)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php:27]

EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=customOAuthUrlencodeNull, ref.master=788098096e11, ref.customOAuthUrlencodeNull=4f0f2043b7e9), phorge(head=customFieldDate, ref.master=bcfcd9acfc12, ref.customFieldDate=bcfcd9acfc12)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php:35]

EXCEPTION: (RuntimeException) ctype_digit(): Argument of type null will be interpreted as string in the future at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=customOAuthUrlencodeNull, ref.master=788098096e11, ref.customOAuthUrlencodeNull=4f0f2043b7e9), phorge(head=customFieldDate, ref.master=bcfcd9acfc12, ref.customFieldDate=bcfcd9acfc12)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
  #1 <#2> ctype_digit(NULL) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php:77]

Closes T15601

Test Plan

After applying these three changes and creating a custom field with "type": "date" under /config/edit/maniphest.custom-field-definitions/, the website /transactions/editengine/maniphest.task/view/5/ renders correctly in the browser, showing "This is a preview of the current form configuration."

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.Aug 14 2023, 20:01

Owners added a reviewer: O1: Blessed Committers.Aug 14 2023, 20:01

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptAug 14 2023, 20:01

aklapper requested review of this revision.Aug 14 2023, 20:01

Harbormaster completed remote builds in B749: Diff 1249.Aug 14 2023, 20:01

...and wrong. Needs to check for a scalar instead because with a custom Date field I get: Call to phutil_nonempty_string() expected null or a string, got: int.

Check for scalar instead of string as the passed value can also be an integer

Harbormaster completed remote builds in B750: Diff 1250.Aug 14 2023, 20:08

aklapper edited the summary of this revision. (Show Details)Aug 14 2023, 20:10

and of course after this round I got an additional RuntimeException: ctype_digit(): Argument of type int will be interpreted as string in the future to also tackle

Harbormaster completed remote builds in B751: Diff 1251.Aug 14 2023, 20:18

aklapper mentioned this in T15602: PHP 8.1 "json_decode(null)" exception editing a form when custom field of type Users exists.Aug 14 2023, 20:27

I love this change, I've looked inside the logic and it seems very correct to me,

There is still a possibility that some weird extensions could use objects with magic __toString() methods to return an int-cast-able integer, and this could break renderEditControl() causing an iper-mega-nuclear-implosion. But this corner case is actually nonsense at the moment to be proactively supported without any proof about the existence of these stakeholders.

So everything works, and we like nuclear implosions for alien values, and aliens will contact us to fix these nuclear implosions, and we like it.

Thanks

sgtm