Paths

Table of Contentst

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.

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

Branch

master

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 736
Build 736: arc lint + arc unit

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

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?

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

Path

Size

Packages

src/

applications/

notification/

config/

PhabricatorNotificationServersConfigType.php

2 lines

O1: Blessed Committers

Diff 1218

View Options

src/applications/notification/config/PhabricatorNotificationServersConfigType.php

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