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

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

Details

Reviewers

speck

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

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

Commits

rP76ef9db8a5ed: Fix PHP 8.1 "strlen(null)" exception about Staging URI on Diffusion repo…

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

aklapper created this revision.Jun 10 2023, 14:15

Owners added a reviewer: O1: Blessed Committers.Jun 10 2023, 14:15

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptJun 10 2023, 14:15

aklapper requested review of this revision.Jun 10 2023, 14:15

Harbormaster completed remote builds in B526: Diff 914.Jun 10 2023, 14:15

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

Harbormaster completed remote builds in B527: Diff 915.Jun 10 2023, 14:23

aklapper edited the summary of this revision. (Show Details)Jun 10 2023, 14:23

Thanks!

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

Closed by commit rP76ef9db8a5ed: Fix PHP 8.1 "strlen(null)" exception about Staging URI on Diffusion repo…. · Explain WhyJun 10 2023, 16:34

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rP76ef9db8a5ed: Fix PHP 8.1 "strlen(null)" exception about Staging URI on Diffusion repo….