Fix PHP 8.1 "strlen(null)" exception when trying to add empty user email address
ClosedPublic
Actions

Authored by aklapper on May 9 2023, 21:09.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15377: PHP 8.1 "strlen(null)" exception when trying to add empty user email address

Commits

rP4d9719539773: Fix PHP 8.1 "strlen(null)" exception when trying to add empty user email address

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.

Closes T15377

Test Plan

Applied this change, afterwards repeated the steps to add a new email address on /settings/panel/email/ and left the "Email" field empty. This time, after selecting the "Save" button, the error message "Email is required." is displayed in the "New Address" overlay dialog.

Diff Detail

Repository

rP Phorge

Branch

addEmptyOwnNewEmailAd (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 388
Build 388: arc lint + arc unit

Event Timeline

aklapper created this revision.May 9 2023, 21:09

Owners added a reviewer: O1: Blessed Committers.May 9 2023, 21:09

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptMay 9 2023, 21:09

aklapper requested review of this revision.May 9 2023, 21:09

Harbormaster completed remote builds in B388: Diff 705.May 9 2023, 21:09

Thanks

Tested locally. Adding an email. Adding no email. Multiple fuzz tests. No implosions.

lgtm

src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php
188	💚 I verifies that the `$email` comes from `AphrontRequest#getStr()` that always returns null or a string. The function `phutil_nonempty_string()` will report alien types (such as objects), and that extra check is OK here.