Fix PHP 8.1 "strlen(null)" exception which blocks rendering MFA Provider page
ClosedPublic
Actions

Authored by aklapper on May 12 2023, 16:50.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15386: PHP 8.1 "strlen(null)" exception blocks rendering MFA Provider page

Commits

rP1b905706b450: Fix PHP 8.1 "strlen(null)" exception which blocks rendering MFA Provider page

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 T15386

Test Plan

Applied this change, afterwards page "Mobile Phone App (TOTP)" at /auth/mfa/4/ renders in web browser.

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.May 12 2023, 16:50

Owners added a reviewer: O1: Blessed Committers.May 12 2023, 16:50

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptMay 12 2023, 16:50

aklapper requested review of this revision.May 12 2023, 16:50

Harbormaster completed remote builds in B400: Diff 723.May 12 2023, 16:50

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

Thanks again for this patch! Tested locally with and without an "enroll message".

Interestingly I was not able to understand what the hell "enroll" means and where this message is shown apart from this Configuration page. For example in the frontend here is not shown:

MFA Frontend message.png (285×838 px, 23 KB)

But, this seems perfectly OK to me. The function phutil_nonempty_string() will explode for any alien type that is not a string or not null. And that is OK here.

sgtm