Addressing PHP8 incompatibilities - Conduit

neandrake · neandrake · commit dd94e2efcfb7 · 2023-05-30T11:58:43.000-04:00
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
diff --git a/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php
@@ -55,6 +55,9 @@ protected function defineReturnType() {
   protected function execute(ConduitAPIRequest $request) {
     $viewer = $request->getUser();
     $change_data = $request->getValue('changes');
+    if ($change_data === null) {
+      throw new Exception(pht('Field "changes" must be non-empty.'));
+    }
 
     $changes = array();
     foreach ($change_data as $dict) {
diff --git a/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php
@@ -26,6 +26,9 @@ protected function defineReturnType() {
   protected function execute(ConduitAPIRequest $request) {
     $viewer = $request->getUser();
     $raw_diff = $request->getValue('diff');
+    if ($raw_diff === null || !strlen($raw_diff)) {
+      throw new Exception(pht('Field "raw_diff" must be non-empty.'));
+    }
 
     $repository_phid = $request->getValue('repositoryPHID');
     if ($repository_phid) {
diff --git a/src/applications/differential/conduit/DifferentialParseCommitMessageConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialParseCommitMessageConduitAPIMethod.php
@@ -33,6 +33,9 @@ protected function execute(ConduitAPIRequest $request) {
     }
 
     $corpus = $request->getValue('corpus');
+    if ($corpus === null || !strlen($corpus)) {
+      throw new Exception(pht('Field "corpus" must be non-empty.'));
+    }
     $field_map = $parser->parseFields($corpus);
 
     $errors = $parser->getErrors();
diff --git a/src/applications/differential/conduit/DifferentialSetDiffPropertyConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialSetDiffPropertyConduitAPIMethod.php
@@ -30,9 +30,22 @@ protected function defineErrorTypes() {
   }
 
   protected function execute(ConduitAPIRequest $request) {
+    $data = $request->getValue('data');
+    if ($data === null || !strlen($data)) {
+      throw new Exception(pht('Field "data" must be non-empty.'));
+    }
+
     $diff_id = $request->getValue('diff_id');
+    if ($diff_id === null) {
+      throw new Exception(pht('Field "diff_id" must be non-null.'));
+    }
+
     $name = $request->getValue('name');
-    $data = json_decode($request->getValue('data'), true);
+    if ($name === null || !strlen($name)) {
+      throw new Exception(pht('Field "name" must be non-empty.'));
+    }
+
+    $data = json_decode($data, true);
 
     self::updateDiffProperty($diff_id, $name, $data);
   }
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -218,7 +218,7 @@ protected function expandTransaction(
 
           // No "$", to allow for branches like T123_demo.
           $match = null;
-          if (preg_match('/^T(\d+)/i', $branch, $match)) {
+          if ($branch !== null && preg_match('/^T(\d+)/i', $branch, $match)) {
             $task_id = $match[1];
             $tasks = id(new ManiphestTaskQuery())
               ->setViewer($this->getActor())
diff --git a/src/applications/files/conduit/FileUploadConduitAPIMethod.php b/src/applications/files/conduit/FileUploadConduitAPIMethod.php
@@ -31,6 +31,9 @@ protected function execute(ConduitAPIRequest $request) {
     $view_policy = $request->getValue('viewPolicy');
 
     $data = $request->getValue('data_base64');
+    if ($data === null) {
+      throw new Exception(pht('Field "data_base64" must be non-empty.'));
+    }
     $data = $this->decodeBase64($data);
 
     $params = array(
diff --git a/src/applications/harbormaster/conduit/HarbormasterSendMessageConduitAPIMethod.php b/src/applications/harbormaster/conduit/HarbormasterSendMessageConduitAPIMethod.php
@@ -515,15 +515,15 @@ protected function execute(ConduitAPIRequest $request) {
       }
     }
 
-    if (!strlen($receiver_name)) {
+    if ($receiver_name === null || !strlen($receiver_name)) {
       throw new Exception(
         pht(
           'Call omits required "receiver" parameter. Specify the PHID '.
           'of the object you want to send a message to.'));
     }
 
     $message_type = $request->getValue('type');
-    if (!strlen($message_type)) {
+    if ($message_type === null || !strlen($message_type)) {
       throw new Exception(
         pht(
           'Call omits required "type" parameter. Specify the type of '.
diff --git a/src/applications/harbormaster/editor/HarbormasterBuildPlanEditEngine.php b/src/applications/harbormaster/editor/HarbormasterBuildPlanEditEngine.php
@@ -103,7 +103,7 @@ protected function buildCustomEditFields($object) {
         $key);
       $behavior_option = $object->getPlanProperty($storage_key);
 
-      if (!strlen($behavior_option)) {
+      if ($behavior_option === null || !strlen($behavior_option)) {
         $behavior_option = $behavior->getPlanOption($object)->getKey();
       }
 
diff --git a/src/applications/paste/conduit/PasteCreateConduitAPIMethod.php b/src/applications/paste/conduit/PasteCreateConduitAPIMethod.php
@@ -43,7 +43,7 @@ protected function execute(ConduitAPIRequest $request) {
     $title    = $request->getValue('title');
     $language = $request->getValue('language');
 
-    if (!strlen($content)) {
+    if ($content === null || !strlen($content)) {
       throw new ConduitException('ERR-NO-PASTE');
     }
 
diff --git a/src/applications/phriction/conduit/PhrictionCreateConduitAPIMethod.php b/src/applications/phriction/conduit/PhrictionCreateConduitAPIMethod.php
@@ -25,9 +25,10 @@ protected function defineReturnType() {
 
   protected function execute(ConduitAPIRequest $request) {
     $slug = $request->getValue('slug');
-    if (!strlen($slug)) {
-      throw new Exception(pht('No such document.'));
+    if ($slug === null || !strlen($slug)) {
+      throw new Exception(pht('Field "slug" must be non-empty.'));
     }
+
     $doc = id(new PhrictionDocumentQuery())
       ->setViewer($request->getUser())
       ->withSlugs(array(PhabricatorSlug::normalize($slug)))
diff --git a/src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php b/src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php
@@ -25,6 +25,9 @@ protected function defineReturnType() {
 
   protected function execute(ConduitAPIRequest $request) {
     $slug = $request->getValue('slug');
+    if ($slug === null || !strlen($slug)) {
+      throw new Exception(pht('Field "slug" must be non-empty.'));
+    }
 
     $doc = id(new PhrictionDocumentQuery())
       ->setViewer($request->getUser())
diff --git a/src/applications/phriction/conduit/PhrictionHistoryConduitAPIMethod.php b/src/applications/phriction/conduit/PhrictionHistoryConduitAPIMethod.php
@@ -38,6 +38,10 @@ protected function defineErrorTypes() {
 
   protected function execute(ConduitAPIRequest $request) {
     $slug = $request->getValue('slug');
+    if ($slug === null || !strlen($slug)) {
+      throw new Exception(pht('Field "slug" must be non-empty.'));
+    }
+
     $doc = id(new PhrictionDocumentQuery())
       ->setViewer($request->getUser())
       ->withSlugs(array(PhabricatorSlug::normalize($slug)))
diff --git a/src/applications/phriction/conduit/PhrictionInfoConduitAPIMethod.php b/src/applications/phriction/conduit/PhrictionInfoConduitAPIMethod.php
@@ -38,6 +38,9 @@ protected function defineErrorTypes() {
 
   protected function execute(ConduitAPIRequest $request) {
     $slug = $request->getValue('slug');
+    if ($slug === null || !strlen($slug)) {
+      throw new Exception(pht('Field "slug" must be non-empty.'));
+    }
 
     $document = id(new PhrictionDocumentQuery())
       ->setViewer($request->getUser())
diff --git a/src/applications/project/conduit/ProjectCreateConduitAPIMethod.php b/src/applications/project/conduit/ProjectCreateConduitAPIMethod.php
@@ -43,12 +43,21 @@ protected function execute(ConduitAPIRequest $request) {
 
     $project = PhabricatorProject::initializeNewProject($user);
     $type_name = PhabricatorProjectNameTransaction::TRANSACTIONTYPE;
+
+    $name = $request->getValue('name');
+    if ($name === null || !strlen(name)) {
+      throw new Exception(pht('Field "name" must be non-empty.'));
+    }
+
     $members = $request->getValue('members');
+    if ($members === null) {
+      $members = array();
+    }
     $xactions = array();
 
     $xactions[] = id(new PhabricatorProjectTransaction())
       ->setTransactionType($type_name)
-      ->setNewValue($request->getValue('name'));
+      ->setNewValue($name);
 
     if ($request->getValue('icon')) {
       $xactions[] = id(new PhabricatorProjectTransaction())
diff --git a/src/applications/remarkup/conduit/RemarkupProcessConduitAPIMethod.php b/src/applications/remarkup/conduit/RemarkupProcessConduitAPIMethod.php
@@ -41,7 +41,7 @@ protected function execute(ConduitAPIRequest $request) {
 
     $engine_class = idx($this->getEngineContexts(), $context);
     if (!$engine_class) {
-      throw new ConduitException('ERR-INVALID_ENGINE');
+      throw new ConduitException('ERR-INVALID-ENGINE');
     }
 
     $engine = PhabricatorMarkupEngine::$engine_class();
diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php b/src/applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php
@@ -95,7 +95,7 @@ private function buildEditTypesDocumentationPages(
       $section[] = $type->getConduitDescription();
 
       $type_documentation = $type->getConduitDocumentation();
-      if (strlen($type_documentation)) {
+      if ($type_documentation !== null && strlen($type_documentation)) {
         $section[] = $type_documentation;
       }
 
diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php
@@ -24,7 +24,7 @@ public function buildOrderIndex() {
 
   public function getValueForStorage() {
     $value = $this->getFieldValue();
-    if (strlen($value)) {
+    if ($value !== null && strlen($value)) {
       return $value;
     } else {
       return null;
diff --git a/src/infrastructure/markup/blockrule/PhutilRemarkupTableBlockRule.php b/src/infrastructure/markup/blockrule/PhutilRemarkupTableBlockRule.php
@@ -114,7 +114,7 @@ private function newTable(PhutilDOMNode $table) {
         if ($cell->isContentNode()) {
           $content = $node->getContent();
 
-          if (!strlen(trim($content))) {
+          if ($content === null || !strlen(trim($content))) {
             continue;
           }
 

Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,9 @@ protected function execute(ConduitAPIRequest $request) {`
`33`	`33`	`}`
`34`	`34`
`35`	`35`	`$corpus = $request->getValue('corpus');`
	`36`	`+ if ($corpus === null \|\| !strlen($corpus)) {`
	`37`	`+ throw new Exception(pht('Field "corpus" must be non-empty.'));`
	`38`	`+ }`
`36`	`39`	`$field_map = $parser->parseFields($corpus);`
`37`	`40`
`38`	`41`	`$errors = $parser->getErrors();`
Original file line number	Diff line number	Diff line change
`@@ -515,15 +515,15 @@ protected function execute(ConduitAPIRequest $request) {`
`515`	`515`	`}`
`516`	`516`	`}`
`517`	`517`
`518`		`- if (!strlen($receiver_name)) {`
	`518`	`+ if ($receiver_name === null \|\| !strlen($receiver_name)) {`
`519`	`519`	`throw new Exception(`
`520`	`520`	`pht(`
`521`	`521`	`'Call omits required "receiver" parameter. Specify the PHID '.`
`522`	`522`	`'of the object you want to send a message to.'));`
`523`	`523`	`}`
`524`	`524`
`525`	`525`	`$message_type = $request->getValue('type');`
`526`		`- if (!strlen($message_type)) {`
	`526`	`+ if ($message_type === null \|\| !strlen($message_type)) {`
`527`	`527`	`throw new Exception(`
`528`	`528`	`pht(`
`529`	`529`	`'Call omits required "type" parameter. Specify the type of '.`
Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,7 @@ protected function buildCustomEditFields($object) {`
`103`	`103`	`$key);`
`104`	`104`	`$behavior_option = $object->getPlanProperty($storage_key);`
`105`	`105`
`106`		`- if (!strlen($behavior_option)) {`
	`106`	`+ if ($behavior_option === null \|\| !strlen($behavior_option)) {`
`107`	`107`	`$behavior_option = $behavior->getPlanOption($object)->getKey();`
`108`	`108`	`}`
`109`	`109`
Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ protected function execute(ConduitAPIRequest $request) {`
`43`	`43`	`$title = $request->getValue('title');`
`44`	`44`	`$language = $request->getValue('language');`
`45`	`45`
`46`		`- if (!strlen($content)) {`
	`46`	`+ if ($content === null \|\| !strlen($content)) {`
`47`	`47`	`throw new ConduitException('ERR-NO-PASTE');`
`48`	`48`	`}`
`49`	`49`