Fix PHP 8.1 "strlen(null)" exceptions which block rendering page to import ICS calendar file
ClosedPublic
Actions

Authored by aklapper on May 6 2023, 22:52.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15362: PHP 8.1 "strlen(null)" exceptions block rendering page to import ICS calendar file

Commits

rP8d2b481bb5a8: Fix PHP 8.1 "strlen(null)" exceptions which block rendering page to import ICS…

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.

Closes T15362

Test Plan

Applied these two changes and /calendar/import/edit/ correctly rendered the "Create Import" page in the web browser, with the expected error about the missing ICS file (instead of an exception stacktrace).

Diff Detail

Repository

rP Phorge

Branch

importICScal (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 372
Build 372: arc lint + arc unit

Event Timeline

aklapper created this revision.May 6 2023, 22:52

Owners added a reviewer: O1: Blessed Committers.May 6 2023, 22:52

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptMay 6 2023, 22:52

aklapper requested review of this revision.May 6 2023, 22:52

Harbormaster completed remote builds in B372: Diff 677.May 6 2023, 22:52

I tested both values locally with phlog() and using some forms as the test plan suggests.

The $new_value just assumes null, or strings like PHID-FILE-w4n7l42i3gcq5x6rvwcy for a file upload. The $filename is also always a string or null.

The function phutil_nonempty_string() will report any alien type, and that is OK here.

This revision is now accepted and ready to land.May 8 2023, 20:08

Closed by commit rP8d2b481bb5a8: Fix PHP 8.1 "strlen(null)" exceptions which block rendering page to import ICS…. · Explain WhyMay 8 2023, 21:13

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rP8d2b481bb5a8: Fix PHP 8.1 "strlen(null)" exceptions which block rendering page to import ICS….