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
F3318682: D25383.1743290350.diff
Fri, Mar 28, 23:19
F3306303: D25383.1743126774.diff
Thu, Mar 27, 01:52
F3304613: D25383.1743102421.diff
Wed, Mar 26, 19:07
F3304185: D25383.1743092942.diff
Wed, Mar 26, 16:29
F3299024: D25383.1743014333.diff
Tue, Mar 25, 18:38
F3293610: D25383.1742916994.diff
Mon, Mar 24, 15:36
F3291147: D25383.1742877016.diff
Mon, Mar 24, 04:30
F3289498: D25383.1742848515.diff
Sun, Mar 23, 20:35

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