Paths

Table of Contentst

Differential D25210

Fix PHP 8.1 "ltrim(null)" exception which blocks adding additional user email address
ClosedPublic
Actions

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

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15376: PHP 8.1 "ltrim(null)" exception blocks adding additional user email address

Commits

rP94d45de2f7dd: Fix PHP 8.1 "ltrim(null)" exception which blocks adding additional user email…

Summary

Since PHP 8.1, passing a null string to ltrim(string $string) is deprecated.

Thus we make sure that $request->getStr('email') does not return null as default.

Closes T15376

Test Plan

Applied this change, afterwards repeated the steps to add a new email address on /settings/panel/email/. This time, it's possible to close the "Verification Email Sent" and the page /settings/panel/email/ renders and lists the new email address.

Diff Detail

Repository

rP Phorge

Branch

arcpatch-D25210

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 421
Build 421: arc lint + arc unit

Event Timeline

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

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

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

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

Harbormaster completed remote builds in B387: Diff 704.May 9 2023, 21:04

Thanks

Please see my inline comment, probably just adding 3 characters fixes, and that would be probably the approach from O1

I accept as behalf of myself in the meanwhile, thanks

src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php
175	Probably the exact minimal fix is just that. Kind of "don't care" logic.

valerio.bozzolan mentioned this in Wikimedia Hackaton 2023 Athens Code Sprint.May 18 2023, 10:16

Yep I confirm my opinion here, let's ping-pong this tomorrow

valerio.bozzolan requested changes to this revision.May 19 2023, 23:03

This revision now requires changes to proceed.May 19 2023, 23:03

(it works indeed, so no need from me to red block)

Use minimal fix instead

Harbormaster completed remote builds in B421: Diff 756.May 20 2023, 09:53

small indentation fix

Harbormaster completed remote builds in B422: Diff 757.May 20 2023, 11:58

Thanks again :)

I tested this locally following the test plan without any nuclear implosion. Green light from me

lgtm

This revision is now accepted and ready to land.May 20 2023, 12:03

Closed by commit rP94d45de2f7dd: Fix PHP 8.1 "ltrim(null)" exception which blocks adding additional user email…. · Explain WhyMay 20 2023, 12:27

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rP94d45de2f7dd: Fix PHP 8.1 "ltrim(null)" exception which blocks adding additional user email….

Revision Contents
Changeset List

Path

Size

Packages

src/

applications/

settings/

panel/

PhabricatorEmailAddressesSettingsPanel.php

2 lines

O1: Blessed Committers

Diff 756

View Options

src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php

Fix PHP 8.1 "ltrim(null)" exception which blocks adding additional user email addressClosedPublicActions