Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exception which blocks creating a project with an empty Description field
ClosedPublic

Authored by aklapper on May 2 2023, 19:20.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 26, 12:44
Unknown Object (File)
Tue, Mar 26, 11:32
Unknown Object (File)
Sun, Mar 24, 06:27
Unknown Object (File)
Wed, Mar 20, 10:31
Unknown Object (File)
Sun, Mar 10, 12:48
Unknown Object (File)
Feb 13 2024, 17:32
Unknown Object (File)
Feb 9 2024, 16:37
Unknown Object (File)
Feb 9 2024, 16:37

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_stringlike() as a replacement for string-alike variables.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_stringlike() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

Closes first half of T15331

Test Plan

Applied these two changes (one in Arcanist, one in Phorge). Project with empty Description field was created and /project/view/projectid/ rendered in web browser.

Diff Detail

Repository
rARC Arcanist
Branch
emptyProjDesc (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 339
Build 339: arc lint + arc unit

Event Timeline

aklapper requested review of this revision.May 2 2023, 19:20

Thanks for this patch.

This needs the usual stringlike() trick to make Phorge happy

Also it probably needs the phlog() trick suggested by avivey to make O1 happy

I would say, let's wait for what happens in D25157, then let's turn back here

I'm sorry for this waiting

src/utils/utils.php
903

The already existing PHPDoc was wrong.

The $corpus parameter is not necessarily a string but can be a PhutilSafeHTML as can be see at line 928

911

Sadly it seems using phutil_nonempty_string() in this case can crash since $corpus can be an object.

We probably need the phutil_nonempty_stringlike() trick instead.

928

Interestingly this line highlights that $corpus is not always a string

This revision now requires changes to proceed.May 3 2023, 09:38

Use phutil_nonempty_stringlike; update doc comment.

Not sure if some additional phlog code is wanted or needed here - what exactly do we want to find out, apart from string and PhutilSafeHTML being input types?

Accepting (but not as O1) since this seems totally legit to me now. Thanks for your update

I don't approve as O1 because I know probably somebody would like to share different opinions.

This revision is now accepted and ready to land.Jun 8 2023, 01:00