Skip to content

Commit e8ea7a4

Browse files
Steve Campbellcampbsb
Steve Campbell
authored andcommitted
Fix PHP 8.1 issue in DifferentialChangeset getOldStatePathVector()
Summary: The DifferentialChangeset getOldStatePathVector() method assumes oldFile and filename are set. This worked under PHP <= 8.0, but fails for PHP >= 8.1 with error messsage ``` strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated ``` Fixes T15517 Test Plan: Create a diff in which a new file is added. This file will have oldFile NULL and filename a string. View the diff https://my.phorge.site/D1234 Reviewers: O1 Blessed Committers, Matthew Reviewed By: O1 Blessed Committers, Matthew Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15517 Differential Revision: https://we.phorge.it/D25323
1 parent 98dfac5 commit e8ea7a4

File tree

3 files changed

+24
-4
lines changed

3 files changed

+24
-4
lines changed

src/__phutil_library_map__.php

+2
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,7 @@
488488
'DifferentialChangesetRenderer' => 'applications/differential/render/DifferentialChangesetRenderer.php',
489489
'DifferentialChangesetSearchConduitAPIMethod' => 'applications/differential/conduit/DifferentialChangesetSearchConduitAPIMethod.php',
490490
'DifferentialChangesetSearchEngine' => 'applications/differential/query/DifferentialChangesetSearchEngine.php',
491+
'DifferentialChangesetTestCase' => 'applications/differential/storage/__tests__/DifferentialChangesetTestCase.php',
491492
'DifferentialChangesetTestRenderer' => 'applications/differential/render/DifferentialChangesetTestRenderer.php',
492493
'DifferentialChangesetTwoUpRenderer' => 'applications/differential/render/DifferentialChangesetTwoUpRenderer.php',
493494
'DifferentialChangesetTwoUpTestRenderer' => 'applications/differential/render/DifferentialChangesetTwoUpTestRenderer.php',
@@ -6501,6 +6502,7 @@
65016502
'DifferentialChangesetRenderer' => 'Phobject',
65026503
'DifferentialChangesetSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
65036504
'DifferentialChangesetSearchEngine' => 'PhabricatorApplicationSearchEngine',
6505+
'DifferentialChangesetTestCase' => 'PhabricatorTestCase',
65046506
'DifferentialChangesetTestRenderer' => 'DifferentialChangesetRenderer',
65056507
'DifferentialChangesetTwoUpRenderer' => 'DifferentialChangesetHTMLRenderer',
65066508
'DifferentialChangesetTwoUpTestRenderer' => 'DifferentialChangesetTestRenderer',

src/applications/differential/storage/DifferentialChangeset.php

+6-4
Original file line numberDiff line numberDiff line change
@@ -325,14 +325,16 @@ public function getDiff() {
325325

326326
public function getOldStatePathVector() {
327327
$path = $this->getOldFile();
328-
if (!strlen($path)) {
328+
if (!phutil_nonempty_string($path)) {
329329
$path = $this->getFilename();
330330
}
331331

332-
$path = trim($path, '/');
333-
$path = explode('/', $path);
332+
if (!phutil_nonempty_string($path)) {
333+
return null;
334+
}
334335

335-
return $path;
336+
$path = trim($path, '/');
337+
return explode('/', $path);
336338
}
337339

338340
public function getNewStatePathVector() {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
final class DifferentialChangesetTestCase extends PhabricatorTestCase {
4+
5+
public function testPhp81() {
6+
$diff_change_set = new DifferentialChangeset();
7+
try {
8+
$old_state_vector = $diff_change_set->getOldStatePathVector();
9+
$this->assertTrue(true, 'getOldStatePathVector did not throw an error');
10+
} catch (Throwable $ex) {
11+
$this->assertTrue(false,
12+
'getOldStatePathVector threw an exception:'.$ex->getMessage());
13+
}
14+
}
15+
16+
}

0 commit comments

Comments
 (0)