Page MenuHomePhorge

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

Authored by bob on Aug 16 2023, 16:17.
Referenced Files
F358429: D25400.diff
Tue, Sep 26, 09:19
Unknown Object (File)
Thu, Sep 21, 02:48
Unknown Object (File)
Thu, Sep 7, 10:15
Unknown Object (File)
Sun, Sep 3, 10:09
Unknown Object (File)
Wed, Aug 30, 18:16
Unknown Object (File)
Mon, Aug 28, 11:34
Unknown Object (File)
Aug 25 2023, 17:25
Unknown Object (File)
Aug 23 2023, 06:20



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 Passed
Tests Passed
Build Status
Buildable 769
Build 769: arc lint + arc unit

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