Page MenuHomePhorge

Fix a PHP 8.1/8.2 deprecated call to strlen with a NULL argument
ClosedPublic

Authored by bob on Aug 9 2023, 16:07.

Details

Summary

This call was preventing notification servers configuration to be properly initialized.
Indeed, 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.

Fix T15596

Test Plan

Sign in as an administrator, configure the notification server without filling admin path field,
you shouldn't get an invalid configuration error

Diff Detail

Repository
rP Phorge
Branch
master
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 736
Build 736: arc lint + arc unit

Event Timeline

bob requested review of this revision.Aug 9 2023, 16:07

Thanks bob, ready for land

Indeed NULL must be an allowed value, since this check was designed to exactly look for a non-existence of that. Indeed the only allowed value must be a string.

Any other alien value will cause our lovely-super-mega-implosion, and we like that.

lgtm

src/applications/notification/config/PhabricatorNotificationServersConfigType.php
35

↑ interestingly the path MUST be a string, or NULL, or nothing else, so indeed the change looks good to me.

This revision is now accepted and ready to land.Aug 10 2023, 08:10

Perfect ! Thanks for the review valerio !

Would you like to try yourself arc patch D25381 and then arc land to land into master branch?