Page MenuHomePhorge

Fix a PHP 8.1/8.2 deprecated use of strlen deprecated call with a NULL argument
ClosedPublic

Authored by bob on Aug 10 2023, 16:28.
Tags
None
Referenced Files
Unknown Object (File)
Thu, May 2, 09:56
Unknown Object (File)
Wed, May 1, 19:12
Unknown Object (File)
Wed, May 1, 19:12
Unknown Object (File)
Wed, May 1, 19:12
Unknown Object (File)
Tue, Apr 30, 18:39
Unknown Object (File)
Tue, Apr 30, 06:30
Unknown Object (File)
Tue, Apr 30, 06:30
Unknown Object (File)
Tue, Apr 30, 06:30

Details

Summary

This strlen call triggering an exception if an user tried to call the patch command without an authentication token
Indeed, strlen() was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1, we use phutil_nonempty_string() as a replacement.

Fix T15599

Test Plan

Remove your arcanist authentication token file (if you have one) and try to call the patch command in a repository.
You should get an error message suggesting you to call the install-certificate command instead of an exception.

Diff Detail

Repository
rARC Arcanist
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bob requested review of this revision.Aug 10 2023, 16:28

Thanks! Interestingly you fixed an incomplete fix since D25128

It seems that the original patcher had not the array key, so they fixed that case with a default array key value as empty-string. Instead, you have the array key, and it's NULL, so the default array-key is unused and just confusing.

Does this fix your problem, keeping your change as-is, but also rollbacking idx() as before, removing the '' default?

src/workflow/ArcanistWorkflow.php
481–482

Can you also try simplifing this in this way (keeping phutil_nonempty_string() below)?

Updating D25383: Removing useless fallback string definition when calling idx

bob marked an inline comment as done.Aug 11 2023, 10:36
This revision is now accepted and ready to land.Aug 11 2023, 16:29