Page MenuHomePhorge

fix PHP 8 "strlen(null)" when moving paths with missing options
ClosedPublic

Authored by jeanguyomarch on Dec 2 2023, 09:51.
Tags
None
Referenced Files
F1392630: D25484.id1556.diff
Tue, Feb 20, 18:20
F1392629: D25484.id1555.diff
Tue, Feb 20, 18:20
F1392628: D25484.id.diff
Tue, Feb 20, 18:20
Unknown Object (File)
Mon, Feb 19, 08:50
Unknown Object (File)
Sat, Feb 17, 22:41
Unknown Object (File)
Sat, Feb 17, 06:22
Unknown Object (File)
Fri, Feb 16, 04:47
Unknown Object (File)
Wed, Jan 31, 20:16

Details

Summary

Running ./bin/repository move-paths without specifying --from nor --to
leads to the PHP 8 error about strlen() not accepting null parameters.

Test Plan

Running ./bin/repository move-paths without --from/--to (shows a proper error)

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Add maniphest task to the commit message

speck subscribed.

This seems fine, though I wonder if we should introduce a phutil_empty_string() or phutil_is_empty_string function to avoid the double-negative logic. I think this reads more easily:

if (phutil_empty_string($from)) {
  throw new Exception()
}

than this:

if (!phutil_nonempty_string($from)) {
  throw new Exception()
}
This revision is now accepted and ready to land.Dec 2 2023, 19:10

In the future it may be nice to also have a getArgStr() that always return a string, never NULL. So, we can just if ( $to === '' )

@jeanguyomarch: Hi, would you like to arc land your accepted patch?