Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exception on Diffusion repository History page after setting Callsign
ClosedPublic

Authored by aklapper on Jun 12 2023, 01:23.
Tags
None
Referenced Files
Unknown Object (File)
Tue, May 14, 19:44
Unknown Object (File)
Tue, May 14, 19:44
Unknown Object (File)
Sun, May 12, 12:28
Unknown Object (File)
Wed, May 8, 11:23
Unknown Object (File)
Wed, May 8, 11:23
Unknown Object (File)
Wed, May 8, 11:23
Unknown Object (File)
Wed, May 8, 10:11
Unknown Object (File)
Wed, May 8, 09:27

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.

EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionCallsignHistory, ref.master=9b99123ff931, ref.diffusionCallsignHistory=9b99123ff931)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryCallsignTransaction.php:28]

Closes T15465

Test Plan

After applying this change, /diffusion/XYZ/manage/history/ renders correctly and shows user set the callsign for this repository to XYZ.

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

speck requested changes to this revision.Jun 12 2023, 01:24

Similar here it seems the values might not always be strings so should be null-check, and both old and new should be checked

This revision now requires changes to proceed.Jun 12 2023, 01:24

Use null checks instead of phutil_nonempty_string

Do you think this also requires the strlen() check?

In D25289#8438, @speck wrote:

Do you think this also requires the strlen() check?

You mean checking that strlen !== 0? I don't think it's needed here

This revision is now accepted and ready to land.Jun 12 2023, 15:33