Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exception which can block repository imports
ClosedPublic

Authored by valerio.bozzolan on May 8 2023, 15:02.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 26, 12:46
Unknown Object (File)
Tue, Mar 26, 11:32
Unknown Object (File)
Sun, Mar 24, 06:33
Unknown Object (File)
Wed, Mar 20, 10:37
Unknown Object (File)
Sat, Mar 16, 07:09
Unknown Object (File)
Feb 25 2024, 07:38
Unknown Object (File)
Feb 25 2024, 07:38
Unknown Object (File)
Feb 25 2024, 07:38

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.

Interestingly, in upstream they started fixing this yesterday, with just this minimal change:

https://secure.phabricator.com/rPf6214f060e780ecf7b565c5a0edbd28d85c03275#C11580NL1139

Premising counting the length of a string just to answer the question "is this empty?" may be overkill,
but premising that adopting stuff like phutil_nonempty_string() could be too risky, we just do
an explicit cast to string, to answer the question "Is this string empty?" just like it was done
under the hood by the strlen() function.

In short, this is probably nice and more readable than upstream (but not a competition).

Closes T15370

Test Plan
  • create a Mercurial repository
  • push some things
  • it should conclude the import (or at least unlock a different Exception)

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

@amit Can you please try again with PHP 8.1 enabled, check whenever this patch fixes your problem? (Or, at least, it unlocks another problem)

To apply this patch, you can also run this command:

arc patch D25204

And please restart your daemon

Thanks

avoid phutil_nonempty_string() since upstream is not using that

avivey subscribed.

I prefer if ($thing !== null && strlen($thing)), but whatever.

This revision is now accepted and ready to land.May 25 2023, 13:53