Skip to content

Commit dd94e2e

Browse files
committedMay 30, 2023
Addressing PHP8 incompatibilities - Conduit
Summary: Navigate through all the conduit pages and address PHP8 incompatibilities. Additionally test out empty form submissions on many of the "create" or "update" APIs and return more contextual errors rather than PHP8 errors. Refs T13588 Test Plan: - Clicked through all the conduit APIs. - Clicked submit on empty forms, or minimally filled-in forms. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley, PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13588 Differential Revision: https://secure.phabricator.com/D21872
1 parent 6015847 commit dd94e2e

18 files changed

+58
-13
lines changed
 

‎src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php

+3
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ protected function defineReturnType() {
5555
protected function execute(ConduitAPIRequest $request) {
5656
$viewer = $request->getUser();
5757
$change_data = $request->getValue('changes');
58+
if ($change_data === null) {
59+
throw new Exception(pht('Field "changes" must be non-empty.'));
60+
}
5861

5962
$changes = array();
6063
foreach ($change_data as $dict) {

‎src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php

+3
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ protected function defineReturnType() {
2626
protected function execute(ConduitAPIRequest $request) {
2727
$viewer = $request->getUser();
2828
$raw_diff = $request->getValue('diff');
29+
if ($raw_diff === null || !strlen($raw_diff)) {
30+
throw new Exception(pht('Field "raw_diff" must be non-empty.'));
31+
}
2932

3033
$repository_phid = $request->getValue('repositoryPHID');
3134
if ($repository_phid) {

‎src/applications/differential/conduit/DifferentialParseCommitMessageConduitAPIMethod.php

+3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ protected function execute(ConduitAPIRequest $request) {
3333
}
3434

3535
$corpus = $request->getValue('corpus');
36+
if ($corpus === null || !strlen($corpus)) {
37+
throw new Exception(pht('Field "corpus" must be non-empty.'));
38+
}
3639
$field_map = $parser->parseFields($corpus);
3740

3841
$errors = $parser->getErrors();

‎src/applications/differential/conduit/DifferentialSetDiffPropertyConduitAPIMethod.php

+14-1
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,22 @@ protected function defineErrorTypes() {
3030
}
3131

3232
protected function execute(ConduitAPIRequest $request) {
33+
$data = $request->getValue('data');
34+
if ($data === null || !strlen($data)) {
35+
throw new Exception(pht('Field "data" must be non-empty.'));
36+
}
37+
3338
$diff_id = $request->getValue('diff_id');
39+
if ($diff_id === null) {
40+
throw new Exception(pht('Field "diff_id" must be non-null.'));
41+
}
42+
3443
$name = $request->getValue('name');
35-
$data = json_decode($request->getValue('data'), true);
44+
if ($name === null || !strlen($name)) {
45+
throw new Exception(pht('Field "name" must be non-empty.'));
46+
}
47+
48+
$data = json_decode($data, true);
3649

3750
self::updateDiffProperty($diff_id, $name, $data);
3851
}

‎src/applications/differential/editor/DifferentialTransactionEditor.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ protected function expandTransaction(
218218

219219
// No "$", to allow for branches like T123_demo.
220220
$match = null;
221-
if (preg_match('/^T(\d+)/i', $branch, $match)) {
221+
if ($branch !== null && preg_match('/^T(\d+)/i', $branch, $match)) {
222222
$task_id = $match[1];
223223
$tasks = id(new ManiphestTaskQuery())
224224
->setViewer($this->getActor())

‎src/applications/files/conduit/FileUploadConduitAPIMethod.php

+3
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ protected function execute(ConduitAPIRequest $request) {
3131
$view_policy = $request->getValue('viewPolicy');
3232

3333
$data = $request->getValue('data_base64');
34+
if ($data === null) {
35+
throw new Exception(pht('Field "data_base64" must be non-empty.'));
36+
}
3437
$data = $this->decodeBase64($data);
3538

3639
$params = array(

‎src/applications/harbormaster/conduit/HarbormasterSendMessageConduitAPIMethod.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -515,15 +515,15 @@ protected function execute(ConduitAPIRequest $request) {
515515
}
516516
}
517517

518-
if (!strlen($receiver_name)) {
518+
if ($receiver_name === null || !strlen($receiver_name)) {
519519
throw new Exception(
520520
pht(
521521
'Call omits required "receiver" parameter. Specify the PHID '.
522522
'of the object you want to send a message to.'));
523523
}
524524

525525
$message_type = $request->getValue('type');
526-
if (!strlen($message_type)) {
526+
if ($message_type === null || !strlen($message_type)) {
527527
throw new Exception(
528528
pht(
529529
'Call omits required "type" parameter. Specify the type of '.

‎src/applications/harbormaster/editor/HarbormasterBuildPlanEditEngine.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ protected function buildCustomEditFields($object) {
103103
$key);
104104
$behavior_option = $object->getPlanProperty($storage_key);
105105

106-
if (!strlen($behavior_option)) {
106+
if ($behavior_option === null || !strlen($behavior_option)) {
107107
$behavior_option = $behavior->getPlanOption($object)->getKey();
108108
}
109109

‎src/applications/paste/conduit/PasteCreateConduitAPIMethod.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ protected function execute(ConduitAPIRequest $request) {
4343
$title = $request->getValue('title');
4444
$language = $request->getValue('language');
4545

46-
if (!strlen($content)) {
46+
if ($content === null || !strlen($content)) {
4747
throw new ConduitException('ERR-NO-PASTE');
4848
}
4949

‎src/applications/phriction/conduit/PhrictionCreateConduitAPIMethod.php

+3-2
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,10 @@ protected function defineReturnType() {
2525

2626
protected function execute(ConduitAPIRequest $request) {
2727
$slug = $request->getValue('slug');
28-
if (!strlen($slug)) {
29-
throw new Exception(pht('No such document.'));
28+
if ($slug === null || !strlen($slug)) {
29+
throw new Exception(pht('Field "slug" must be non-empty.'));
3030
}
31+
3132
$doc = id(new PhrictionDocumentQuery())
3233
->setViewer($request->getUser())
3334
->withSlugs(array(PhabricatorSlug::normalize($slug)))

‎src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php

+3
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ protected function defineReturnType() {
2525

2626
protected function execute(ConduitAPIRequest $request) {
2727
$slug = $request->getValue('slug');
28+
if ($slug === null || !strlen($slug)) {
29+
throw new Exception(pht('Field "slug" must be non-empty.'));
30+
}
2831

2932
$doc = id(new PhrictionDocumentQuery())
3033
->setViewer($request->getUser())

‎src/applications/phriction/conduit/PhrictionHistoryConduitAPIMethod.php

+4
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ protected function defineErrorTypes() {
3838

3939
protected function execute(ConduitAPIRequest $request) {
4040
$slug = $request->getValue('slug');
41+
if ($slug === null || !strlen($slug)) {
42+
throw new Exception(pht('Field "slug" must be non-empty.'));
43+
}
44+
4145
$doc = id(new PhrictionDocumentQuery())
4246
->setViewer($request->getUser())
4347
->withSlugs(array(PhabricatorSlug::normalize($slug)))

‎src/applications/phriction/conduit/PhrictionInfoConduitAPIMethod.php

+3
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ protected function defineErrorTypes() {
3838

3939
protected function execute(ConduitAPIRequest $request) {
4040
$slug = $request->getValue('slug');
41+
if ($slug === null || !strlen($slug)) {
42+
throw new Exception(pht('Field "slug" must be non-empty.'));
43+
}
4144

4245
$document = id(new PhrictionDocumentQuery())
4346
->setViewer($request->getUser())

‎src/applications/project/conduit/ProjectCreateConduitAPIMethod.php

+10-1
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,21 @@ protected function execute(ConduitAPIRequest $request) {
4343

4444
$project = PhabricatorProject::initializeNewProject($user);
4545
$type_name = PhabricatorProjectNameTransaction::TRANSACTIONTYPE;
46+
47+
$name = $request->getValue('name');
48+
if ($name === null || !strlen(name)) {
49+
throw new Exception(pht('Field "name" must be non-empty.'));
50+
}
51+
4652
$members = $request->getValue('members');
53+
if ($members === null) {
54+
$members = array();
55+
}
4756
$xactions = array();
4857

4958
$xactions[] = id(new PhabricatorProjectTransaction())
5059
->setTransactionType($type_name)
51-
->setNewValue($request->getValue('name'));
60+
->setNewValue($name);
5261

5362
if ($request->getValue('icon')) {
5463
$xactions[] = id(new PhabricatorProjectTransaction())

‎src/applications/remarkup/conduit/RemarkupProcessConduitAPIMethod.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ protected function execute(ConduitAPIRequest $request) {
4141

4242
$engine_class = idx($this->getEngineContexts(), $context);
4343
if (!$engine_class) {
44-
throw new ConduitException('ERR-INVALID_ENGINE');
44+
throw new ConduitException('ERR-INVALID-ENGINE');
4545
}
4646

4747
$engine = PhabricatorMarkupEngine::$engine_class();

‎src/applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ private function buildEditTypesDocumentationPages(
9595
$section[] = $type->getConduitDescription();
9696

9797
$type_documentation = $type->getConduitDocumentation();
98-
if (strlen($type_documentation)) {
98+
if ($type_documentation !== null && strlen($type_documentation)) {
9999
$section[] = $type_documentation;
100100
}
101101

‎src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public function buildOrderIndex() {
2424

2525
public function getValueForStorage() {
2626
$value = $this->getFieldValue();
27-
if (strlen($value)) {
27+
if ($value !== null && strlen($value)) {
2828
return $value;
2929
} else {
3030
return null;

‎src/infrastructure/markup/blockrule/PhutilRemarkupTableBlockRule.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ private function newTable(PhutilDOMNode $table) {
114114
if ($cell->isContentNode()) {
115115
$content = $node->getContent();
116116

117-
if (!strlen(trim($content))) {
117+
if ($content === null || !strlen(trim($content))) {
118118
continue;
119119
}
120120

0 commit comments

Comments
 (0)
Failed to load comments.