1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 16:22:43 +01:00

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
This commit is contained in:
Steve Campbell 2023-08-11 10:38:08 +01:00 committed by sten
parent 9c8b9a6bbf
commit 4b3c384856
5 changed files with 31 additions and 2 deletions

View file

@ -714,6 +714,7 @@ phutil_register_library_map(array(
'DifferentialTabReplacementTestCase' => 'applications/differential/parser/__tests__/DifferentialTabReplacementTestCase.php',
'DifferentialTagsCommitMessageField' => 'applications/differential/field/DifferentialTagsCommitMessageField.php',
'DifferentialTasksCommitMessageField' => 'applications/differential/field/DifferentialTasksCommitMessageField.php',
'DifferentialTestCommitMessageField' => 'applications/differential/field/DifferentialTestCommitMessageField.php',
'DifferentialTestPlanCommitMessageField' => 'applications/differential/field/DifferentialTestPlanCommitMessageField.php',
'DifferentialTestPlanField' => 'applications/differential/customfield/DifferentialTestPlanField.php',
'DifferentialTitleCommitMessageField' => 'applications/differential/field/DifferentialTitleCommitMessageField.php',
@ -6760,6 +6761,7 @@ phutil_register_library_map(array(
'DifferentialTabReplacementTestCase' => 'PhabricatorTestCase',
'DifferentialTagsCommitMessageField' => 'DifferentialCommitMessageField',
'DifferentialTasksCommitMessageField' => 'DifferentialCommitMessageField',
'DifferentialTestCommitMessageField' => 'DifferentialCommitMessageField',
'DifferentialTestPlanCommitMessageField' => 'DifferentialCommitMessageField',
'DifferentialTestPlanField' => 'DifferentialCoreCustomField',
'DifferentialTitleCommitMessageField' => 'DifferentialCommitMessageField',

View file

@ -60,7 +60,7 @@ abstract class DifferentialCommitMessageField
}
public function renderFieldValue($value) {
if (!strlen($value)) {
if (!phutil_nonempty_string($value)) {
return null;
}

View file

@ -72,7 +72,7 @@ final class DifferentialRevisionIDCommitMessageField
}
public function renderFieldValue($value) {
if (!strlen($value)) {
if (!phutil_nonempty_string($value)) {
return null;
}

View file

@ -0,0 +1,10 @@
<?php
/**
* This class should only be used for unit tests
*/
final class DifferentialTestCommitMessageField
extends DifferentialCommitMessageField {
public function getFieldName() { return 'Test'; }
public function getFieldOrder() { return 1; }
}

View file

@ -28,4 +28,21 @@ final class DifferentialCommitMessageFieldTestCase
unset($env);
}
public function testRenderFieldValue() {
$test_object = new DifferentialTestCommitMessageField();
$this->assertEqual('foo', $test_object->renderFieldValue('foo'),
'Normal strings should be rendered unaltered');
$this->assertEqual(null, $test_object->renderFieldValue(''),
'Empty strings should be returned as null');
$this->assertEqual(null, $test_object->renderFieldValue(null),
'null values strings should be returned as null');
$test_object = new DifferentialRevisionIDCommitMessageField();
$expected = 'http://phabricator.example.com/D123';
$this->assertEqual($expected, $test_object->renderFieldValue('123'));
$this->assertEqual(null, $test_object->renderFieldValue(null));
}
}