Fix PHP 8.1 "strlen(null)" exceptions importing ICS file into calendar
ClosedPublic
Actions

Authored by aklapper on Aug 18 2023, 16:40.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15620: PHP 8.1 "strlen(null)" exceptions importing ICS file into calendar

Commits

rP9b105c8e9e54: Fix PHP 8.1 "strlen(null)" exceptions importing ICS file into calendar

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=df6c315ace5f), phorge(head=importICSCalendar, ref.master=3cc5ee6a33df, ref.importICSCalendar=3bd396120123)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/calendar/import/PhabricatorCalendarImportEngine.php:459]

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=df6c315ace5f), phorge(head=importICSCalendar, ref.master=3cc5ee6a33df, ref.importICSCalendar=3bd396120123)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/calendar/import/PhabricatorCalendarImportEngine.php:450]

Closes T15620

Test Plan

Revert rP02a4f8b0c8f1279fc0040ad8077942fd8b0d948b not to run into T15619
Try to import an ICS file via /calendar/import/edit/
See that page /calendar/import/4/ renders correctly in web browser and shows Log Messages and Imported Events as expected

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.Aug 18 2023, 16:40

Owners added a reviewer: O1: Blessed Committers.Aug 18 2023, 16:40

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptAug 18 2023, 16:40

aklapper requested review of this revision.Aug 18 2023, 16:40

Harbormaster completed remote builds in B785: Diff 1306.Aug 18 2023, 16:40

Thanks! Tested locally. This seems separated from the mentioned regression. Indeed the recurrence ID and the instance ISO seems that must be strings, or NULL, and any other value will cause a violent nuclear implosion as usual, and Phorge somehow likes it.

Now let's work on T15619

lgtm