Page MenuHomePhorge

Fix a PHP 8.1 deprecated use of strlen with a NULL argument

Authored by bob on Aug 16 2023, 16:17.
Referenced Files
Unknown Object (File)
Wed, Apr 10, 23:15
Unknown Object (File)
Thu, Apr 4, 18:52
Unknown Object (File)
Mon, Apr 1, 06:12
Unknown Object (File)
Mon, Apr 1, 06:12
Unknown Object (File)
Mon, Apr 1, 05:40
Unknown Object (File)
Mon, Apr 1, 05:40
Unknown Object (File)
Mon, Apr 1, 02:39
Unknown Object (File)
Mon, Apr 1, 02:39



This call prevents users to view a commit in subversion repositories
Indeed, if commiter and/or author field is not properly defined strlen is call with a NULL argument.
Using strlen to check string validity 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.

Fix T15610

Test Plan
  • Sign in (if needed)
  • Open a diffusion SVN repository
  • Open a commit without user name and or email
  • You should be able to view the commit

Diff Detail

rP Phorge
Lint Not Applicable
Tests Not Applicable

Event Timeline

bob requested review of this revision.Aug 16 2023, 16:17
valerio.bozzolan retitled this revision from Fix a PHP 8.1/8.2 deprecated use of strlen with a NULL argument to Fix a PHP 8.1 deprecated use of strlen with a NULL argument.Aug 19 2023, 02:52
valerio.bozzolan edited the summary of this revision. (Show Details)


Can somebody else test this? I need more time, since I have issues with hosted repositories at the moment :(

Also bob: is "Sign in" really needed in the test plan in your opinion?

Maybe not. Just being able to see the repository is OK (e.g. with a public Phorge and a public repo)

You're right valerio, this sign in is an automatic reflex since my Phorge instance is private ;)

I was trying to test this, but I don't know how to do a Subversion commit without a committer name 🤔

An example repository would be greatly appreciated.

Premising that I 100% blindly trust the changes in PhabricatorRepositoryCommit. I would just to test PhabricatorRepositoryCommitData since it's not about any "raw" string. So I would like to see if getCommitter() could be an object. Because in case, there we need the "like" thing: phutil_nonempty_stringlike().

Honestly, I don't know why we have old commits improperly filled, you can maybe play with svnadmin command on a dummy repo as described here : Change author of SVN commit

Nobody replied for months. I basically like the disclaimer and I basically trust your test in your production. Thanks.


This revision is now accepted and ready to land.Nov 20 2023, 16:38

Great, I'm a little bit busy, professionally speaking but, I'll try to land it this week !