Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exceptions which block adding 2FA factor

Authored by aklapper on Fri, May 12, 17:06.
Referenced Files
Unknown Object (File)
Sun, Jun 4, 11:56
Unknown Object (File)
Thu, Jun 1, 08:15
Unknown Object (File)
Wed, May 31, 20:36
Unknown Object (File)
Wed, May 31, 11:20
Unknown Object (File)
Wed, May 31, 06:43
Unknown Object (File)
Tue, May 30, 14:47
Unknown Object (File)
Fri, May 26, 07:38



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 T15388

Test Plan

Applied these two changes; afterwards managed to add a 2FA factor and /settings/user/username/page/multifactor/?id=1 correctly listed mys Authentication Factors.

Additional tests:

  • Unset any eventual personal MFA
  • Setup a personal MFA
  • Login/Logout using the MFA
  • Remove a personal MFA
  • Setup a personal enroll message from /auth/mfa/1/
  • Setup a personal MFA
  • Login/Logout using the MFA
  • (then cleanup removing your test MFA)

Diff Detail

rP Phorge
Lint Not Applicable
Tests Not Applicable

Event Timeline

adopt empty string default value, avoiding null

valerio.bozzolan edited the test plan for this revision. (Show Details)

I'm quite sure we do not cause any nuclear implosion here. Added some things in the test plan

Thank you again!


✅ I verified the above line

The method AphrontRequest::getStr($something, '') assures that we only receive something that is never null. Since internally it does an isset() lookup against the potentially returned value, excluding null.


✅ I verified the above line that is similar to D25218

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

This revision is now accepted and ready to land.Sun, May 21, 10:08