Page MenuHomePhorge
Differential D25381

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

Authored by bob on Aug 9 2023, 16:07.
Tags
None
Referenced Files
Unknown Object (File)
Fri, May 10, 00:14
Unknown Object (File)
Fri, May 10, 00:14
Unknown Object (File)
Wed, May 8, 07:58
Unknown Object (File)
Wed, May 8, 07:32
Unknown Object (File)
Thu, May 2, 09:55
Unknown Object (File)
Thu, May 2, 09:55
Unknown Object (File)
Thu, May 2, 09:55
Unknown Object (File)
Wed, May 1, 18:43
View All 83 Files
Subscribers
Cigaryno
Matthew
speck
tobiaswiese
valerio.bozzolan

Details

Reviewers
valerio.bozzolan
Group Reviewers
O1: Blessed Committers
Maniphest Tasks
T15596: PHP 8.2 "strlen(): Passing null to parameter #1 ($string) of type string is deprecated" in configuration
Commits
rP2da7594344c9: Fix a PHP 8.1/8.2 deprecated call to strlen with a NULL argument
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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bob created this revision.Aug 9 2023, 16:07
Owners added a reviewer: O1: Blessed Committers.Aug 9 2023, 16:07
Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptAug 9 2023, 16:07
bob requested review of this revision.Aug 9 2023, 16:07
Harbormaster completed remote builds in B736: Diff 1218.Aug 9 2023, 16:07
bob mentioned this in T15596: PHP 8.2 "strlen(): Passing null to parameter #1 ($string) of type string is deprecated" in configuration.Aug 9 2023, 16:09
bob mentioned this in T15598: PHP 8.2 : rtrim(): Passing null to parameter #1 ($string) of type string is deprecated when checking notification servers.Aug 9 2023, 16:54
valerio.bozzolan accepted this revision.Aug 10 2023, 08:10

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
bob added a comment.Aug 10 2023, 08:33

Perfect ! Thanks for the review valerio !

valerio.bozzolan added a comment.EditedAug 10 2023, 14:46

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

Closed by commit rP2da7594344c9: Fix a PHP 8.1/8.2 deprecated call to strlen with a NULL argument. · Explain WhyAug 10 2023, 15:36
This revision was automatically updated to reflect the committed changes.
bob added a commit: rP2da7594344c9: Fix a PHP 8.1/8.2 deprecated call to strlen with a NULL argument.

Revision Contents
Changeset List

PathSizePackages
src/
applications/
notification/
config/
2 linesO1: Blessed Committers

Diff 1222

View Options

src/applications/notification/config/PhabricatorNotificationServersConfigType.php

Loading...
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0