Page MenuHomePhorge
Differential D25170

Fix PHP 8.1 "strlen(null)" and "array_slice(null)" exceptions which block typeahead completion proposals
ClosedPublic
Actions

Authored by aklapper on May 1 2023, 18:09.

Details

Reviewers
valerio.bozzolan
Group Reviewers
O1: Blessed Committers
Maniphest Tasks
T15321: PHP 8.1 "strlen(null)" and "array_slice(null)" exceptions block typeahead completion proposals
Commits
rPaea1d4737954: Fix PHP 8.1 "strlen(null)" and "array_slice(null)" exceptions which block…
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.

array_slice() no longer accepts passing null as a parameter.
This behavior is deprecated since PHP 8.1. Adding an if clause; not using a
Null Coalescing Operator (PHP 7+) as Phorge currently still supports PHP 5.5.

Closes T15321

Test Plan

Applied these two changes on top of D25147. Afterwards, typeahead autocompletion
proposal dropdowns for the three fields "Assigned To", "Subscribers", "Tags" got
displayed in web browser.

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

aklapper created this revision.May 1 2023, 18:09
Owners added a reviewer: O1: Blessed Committers.May 1 2023, 18:09
Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptMay 1 2023, 18:09
aklapper requested review of this revision.May 1 2023, 18:09
Harbormaster completed remote builds in B324: Diff 602.May 1 2023, 18:09
valerio.bozzolan accepted this revision.May 1 2023, 20:07

Thanks also for this patch.

Tested locally. No implosions.

It would be nice to understand where the parameters are populated, thought. But, this is fortunately probably out of the scope here, since getStr() should do the needed part important to us.

sgtm

src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php
42

✅ I was honestly not able to completely test the non-null values of $raw_parameters so I have not any idea about the intended semantic usage, but the getStr() method is supposed to return strings, so this seems very OK to me.

The method phutil_nonempty_string() will report any alien type, and this OK there.

src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php
210–215

✅ FINALLY the IDEAL usage of a simple if() in PHP. Since the value "0" is assumed as false for weird reasons related to PHP, but here it's converted to the numerical 0 and it has LOT of sense.

Thanks for this.

This revision is now accepted and ready to land.May 1 2023, 20:07
Closed by commit rPaea1d4737954: Fix PHP 8.1 "strlen(null)" and "array_slice(null)" exceptions which block…. · Explain WhyMay 1 2023, 20:29
This revision was automatically updated to reflect the committed changes.
aklapper added a commit: rPaea1d4737954: Fix PHP 8.1 "strlen(null)" and "array_slice(null)" exceptions which block….

Revision Contents
Changeset List

Diff 611

View Options

src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php

Loading...
View Options

src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php

Loading...
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0