Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exception which blocks rendering a File page when Alt Text was altered
ClosedPublic

Authored by aklapper on May 4 2023, 12:01.

Details

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 T15342

Test Plan

Applied these two changes on top of D25186.
Added an Alt Text to a file; page of the File now rendered correctly in web browser.
Removed Alt Text from a file; page of the File now rendered correctly in web browser.

Diff Detail

Repository
rP Phorge
Branch
editFileAltText (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 356
Build 356: arc lint + arc unit

Event Timeline

aklapper requested review of this revision.May 4 2023, 12:01

Thank you for this patch, again (!!!!!!)

I tested this patch locally, putting a phplog() to monitor $new_value and $old_value and doing an extended fuzzy test and I can confirm their assumed input domain: the assumes just a string or null (default).

The function phutil_nonempty_string() will report any alien type, and that is OK in these cases.

Green light and thanks again

LONG LIFE TO PHORGE

yesyes

This revision is now accepted and ready to land.May 5 2023, 21:09