Skip to content
This repository was archived by the owner on Jan 21, 2026. It is now read-only.

Commit 4b3c384

Browse files
Steve Campbellcampbsb
authored andcommitted
Fix DifferentialCommitMessageField renderFieldValue PHP 8.1 strlen(null) error
Summary: arc diff throws strlen(null) error from DifferentialCommitMessageField renderFieldValue when calling a Phorge server running PHP 8.1 Add unit test, which required a new DifferentialTestCommitMessageField class so as to be able to test the abstract DifferentialCommitMessageField class methods. Fixes T15530 Test Plan: Make a change in a git repo with remote a Phorge server running PHP 8.1 Run: ``` arc diff ``` See exception thrown as per T15530 Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15530 Differential Revision: https://we.phorge.it/D25334
1 parent 9c8b9a6 commit 4b3c384

File tree

5 files changed

+31
-2
lines changed

5 files changed

+31
-2
lines changed

src/__phutil_library_map__.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,7 @@
714714
'DifferentialTabReplacementTestCase' => 'applications/differential/parser/__tests__/DifferentialTabReplacementTestCase.php',
715715
'DifferentialTagsCommitMessageField' => 'applications/differential/field/DifferentialTagsCommitMessageField.php',
716716
'DifferentialTasksCommitMessageField' => 'applications/differential/field/DifferentialTasksCommitMessageField.php',
717+
'DifferentialTestCommitMessageField' => 'applications/differential/field/DifferentialTestCommitMessageField.php',
717718
'DifferentialTestPlanCommitMessageField' => 'applications/differential/field/DifferentialTestPlanCommitMessageField.php',
718719
'DifferentialTestPlanField' => 'applications/differential/customfield/DifferentialTestPlanField.php',
719720
'DifferentialTitleCommitMessageField' => 'applications/differential/field/DifferentialTitleCommitMessageField.php',
@@ -6760,6 +6761,7 @@
67606761
'DifferentialTabReplacementTestCase' => 'PhabricatorTestCase',
67616762
'DifferentialTagsCommitMessageField' => 'DifferentialCommitMessageField',
67626763
'DifferentialTasksCommitMessageField' => 'DifferentialCommitMessageField',
6764+
'DifferentialTestCommitMessageField' => 'DifferentialCommitMessageField',
67636765
'DifferentialTestPlanCommitMessageField' => 'DifferentialCommitMessageField',
67646766
'DifferentialTestPlanField' => 'DifferentialCoreCustomField',
67656767
'DifferentialTitleCommitMessageField' => 'DifferentialCommitMessageField',

src/applications/differential/field/DifferentialCommitMessageField.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public function readFieldValueFromObject(DifferentialRevision $revision) {
6060
}
6161

6262
public function renderFieldValue($value) {
63-
if (!strlen($value)) {
63+
if (!phutil_nonempty_string($value)) {
6464
return null;
6565
}
6666

src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public function readFieldValueFromConduit($value) {
7272
}
7373

7474
public function renderFieldValue($value) {
75-
if (!strlen($value)) {
75+
if (!phutil_nonempty_string($value)) {
7676
return null;
7777
}
7878

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
/**
4+
* This class should only be used for unit tests
5+
*/
6+
final class DifferentialTestCommitMessageField
7+
extends DifferentialCommitMessageField {
8+
public function getFieldName() { return 'Test'; }
9+
public function getFieldOrder() { return 1; }
10+
}

src/applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,21 @@ public function testRevisionCommitMessageFieldParsing() {
2828
unset($env);
2929
}
3030

31+
public function testRenderFieldValue() {
32+
$test_object = new DifferentialTestCommitMessageField();
33+
$this->assertEqual('foo', $test_object->renderFieldValue('foo'),
34+
'Normal strings should be rendered unaltered');
35+
36+
$this->assertEqual(null, $test_object->renderFieldValue(''),
37+
'Empty strings should be returned as null');
38+
39+
$this->assertEqual(null, $test_object->renderFieldValue(null),
40+
'null values strings should be returned as null');
41+
42+
$test_object = new DifferentialRevisionIDCommitMessageField();
43+
$expected = 'http://phabricator.example.com/D123';
44+
$this->assertEqual($expected, $test_object->renderFieldValue('123'));
45+
$this->assertEqual(null, $test_object->renderFieldValue(null));
46+
}
47+
3148
}

0 commit comments

Comments
 (0)