Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exception adding LDAP authentication
ClosedPublic

Authored by aklapper on Aug 4 2023, 15:01.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Feb 25, 06:44
Unknown Object (File)
Sat, Feb 24, 04:15
Unknown Object (File)
Sat, Feb 24, 03:26
Unknown Object (File)
Sat, Feb 17, 19:09
Unknown Object (File)
Thu, Feb 15, 22:35
Unknown Object (File)
Sun, Feb 11, 01:51
Unknown Object (File)
Jan 22 2024, 17:49
Unknown Object (File)
Jan 22 2024, 17:48

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.

EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=788098096e11), phorge(head=master, ref.master=840a7fab2bc8)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php:430]

Closes T15587

Test Plan

Go to /auth/config/edit/?provider=PhabricatorLDAPAuthProvider and see that "Add Auth Provider: LDAP" page is rendered.

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

aklapper requested review of this revision.Aug 4 2023, 15:01

Tested thanks!

I've debugged the involved line with phlog to verify that it only assumes NULL or the following string:

Optionally, specify a username attribute to use to prefill usernames when registering a new account. This is purely cosmetic and does not affect the login process, but you can configure it to make sure users get the same default username as their LDAP username, so usernames remain consistent across systems.

Any other unexpected value will create a super-nuclear-implosion, and indeed Phorge/Phabricator likes that.

sgtm

This revision is now accepted and ready to land.Aug 4 2023, 15:50