Fix PHP 8.1 "strlen(null)" exception on LDAP login without password
ClosedPublic
Actions

Authored by aklapper on Jul 26 2024, 14:46.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15893: PHP 8.1 "strlen(null)" exception for LDAP login without password

Commits

rP7909f6a91937: Fix PHP 8.1 "strlen(null)" exception on LDAP login without password

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.

Note: This patch also corrects two further strlen() occurrences with the same pattern.

ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php:145]

Closes T15893

Test Plan

Create an LDAP user without setting their password; try to log into Phabricator with that user via the LDAP auth provider.

Diff Detail

Repository

rP Phorge

Branch

ldapNoPasswordStillCry

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1458
Build 1458: arc lint + arc unit

Event Timeline

aklapper created this revision.Jul 26 2024, 14:46

Owners added a reviewer: O1: Blessed Committers.Jul 26 2024, 14:46

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptJul 26 2024, 14:46

aklapper requested review of this revision.Jul 26 2024, 14:46

Harbormaster completed remote builds in B1458: Diff 2275.Jul 26 2024, 14:46

Thaaaanks. I wonder why $has_password has a dedicated variable, and instead $has_username has not 🤔

Feel free to introduce $has_username if your moon isn't full right now 👍 both for readability and micro-optimization.

This revision is now accepted and ready to land.Jul 26 2024, 16:09

The $password variable is not a string anymore in the line after. I guess it's not much more expensive to check if the string $username is nonempty versus comparing to a boolean value