Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exception about Staging URI on Diffusion repo History page
ClosedPublic

Authored by aklapper on Jun 10 2023, 14:15.

Details

Summary

strlen() was used in Phabricator to check if a generic value is a non-empty string.
Passing null to strlen() is deprecated since PHP 8.1. Thus first check that the string is not null.

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=b325304b6e52), phorge(head=master, ref.master=83edbffd521d)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryStagingURITransaction.php:20]

This is quite similar to D25277.

Closes T15458

Test Plan

After applying this change, editing the Staging URI by saving its already empty value, and going to the repo History page, the page /diffusion/1/manage/history/ renders without an exception, showing R1 being Inactive and user set as the staging area for this repository. [sic!]

Diff Detail

Repository
rP Phorge
Branch
diffusionStagingHistory (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 526
Build 526: arc lint + arc unit

Event Timeline

speck requested changes to this revision.Jun 10 2023, 14:18

Could you make these null-and-strlen checks instead? Notice that within the strlen check it uses “renderOldValue” instead of the old value directly, suggesting it might not be a string

This revision now requires changes to proceed.Jun 10 2023, 14:18

Use null checks instead as we're not sure it's always a string

This revision is now accepted and ready to land.Jun 10 2023, 14:36