Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exceptions which block rendering page of a File
ClosedPublic

Authored by aklapper on May 4 2023, 11:41.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 11, 15:35
Unknown Object (File)
Thu, Apr 11, 09:25
Unknown Object (File)
Thu, Apr 11, 07:42
Unknown Object (File)
Sun, Apr 7, 09:53
Unknown Object (File)
Mon, Apr 1, 01:29
Unknown Object (File)
Sun, Mar 31, 17:22
Unknown Object (File)
Sun, Mar 31, 03:09
Unknown Object (File)
Fri, Mar 29, 20:43

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 T15341

Test Plan

Applied these four changes and file page at /F91 finally rendered in web browser.

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

aklapper requested review of this revision.May 4 2023, 11:41

Thanks for this patch, again (!!!)

I tested this patch, locally, trying to do lot of fuzzy tests. With/withno alternative texts, changing encoding types, etc. I was not able to reproduce any nuclear crash.

Green light

sgtm

src/applications/files/controller/PhabricatorFileViewController.php
314

✅ I verified the above line

The $custom_alt variable is the alternative image text that assumes null as default, or a string.

The function phutil_nonempty_string() will report alien types, and this is OK here.

src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php
63

✅ I verified the above line

The Aphront#getStr() only returns strings, or null (default).

The function phutil_nonempty_string() will report other alien types, and that is OK.

68

✅ I verified the above line

The Aphront#getStr() only returns strings, or null (default).

The function phutil_nonempty_string() will report other alien types, and that is OK.

src/applications/files/storage/PhabricatorFile.php
1281

✅ I verified the above line

The $alt variable is the alternative image text that assumes null as default, or a string.

The function phutil_nonempty_string() will report alien types, and this is good.

This revision is now accepted and ready to land.May 5 2023, 20:34