Fix PHP 8.1 "strlen(null)" exceptions trying to View Branches of a Diffusion repository
ClosedPublic
Actions

Authored by aklapper on May 23 2023, 21:45.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15414: PHP 8.1 "strlen(null)" exceptions trying to View Branches of a Diffusion repository

Commits

rP0cbcb110b03b: Fix PHP 8.1 "strlen(null)" exceptions trying to View Branches of a Diffusion…

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=e4fd31ec024e), phorge(head=D25241, ref.master=b1edfea09bad, ref.D25241=b1edfea09bad)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionBranchTableController.php:29]

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=e4fd31ec024e), phorge(head=D25241, ref.master=b1edfea09bad, ref.D25241=b1edfea09bad)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php:33]

Closes T15414

Test Plan

Applied these two changes and at least got to (FilesystemException) Filesystem path "/var/repo/1/" does not exist. as an error message, instead of being stuck at (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated.

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.May 23 2023, 21:45

Owners added a reviewer: O1: Blessed Committers.May 23 2023, 21:45

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptMay 23 2023, 21:45

Harbormaster completed remote builds in B449: Diff 797.May 23 2023, 21:45

aklapper requested review of this revision.May 23 2023, 21:45

sgtm

src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php
23	✅ ↑ The `contains` parameter must be a `string`
32–33	✅ It seems this is a Conduit API parameter that only accepts strings https://we.phorge.it/conduit/method/diffusion.branchquery/ This is useful to generate stuff like this: git branch --verbose --no-abbrev --contains $contains -- From `git branch --help`: With --contains, shows only the branches that contain the named commit (in other words, the branches whose tip commits are descendants of the named commit), --no-contains inverts it. With --merged, only branches merged into the named commit (i.e. the branches whose tip commits are reachable from the named commit) will be listed. With --no-merged only branches not merged into the named commit will be listed. If the <commit> argument is missing it defaults to HEAD (i.e. the tip of the current branch). So, this indeed must be a string.
src/applications/diffusion/controller/DiffusionBranchTableController.php
29	✅ The variable must be null or a string /** * Get the symbolic commit associated with this request. * * A symbolic commit may be a commit hash, an abbreviated commit hash, a * branch name, a tag name, or an expression like "HEAD^^^". The symbolic * commit may also be absent. * * This method always returns the symbol present in the original request, * in unmodified form. * * See also @{method:getStableCommit}. * * @return string\|null Symbolic commit, if one was present in the request. */ public function getSymbolicCommit() { return $this->symbolicCommit; }