From 080b7aca97dca7f093660112343be18e83824c9c Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Wed, 12 Feb 2020 19:20:53 -0500
Subject: [PATCH] APIv3 - Handle input errors

---
 Civi/API/Kernel.php                                       | 8 +++++---
 Civi/API/Request.php                                      | 4 ++--
 api/api.php                                               | 2 +-
 api/v3/Contact.php                                        | 3 +++
 api/v3/utils.php                                          | 2 +-
 .../Civi/API/Subscriber/WhitelistSubscriberTest.php       | 3 ---
 tests/phpunit/api/v3/UtilsTest.php                        | 1 +
 7 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/Civi/API/Kernel.php b/Civi/API/Kernel.php
index f7a1d5c728..188e2c3acb 100644
--- a/Civi/API/Kernel.php
+++ b/Civi/API/Kernel.php
@@ -75,14 +75,16 @@ class Kernel {
    * @throws \API_Exception
    */
   public function runSafe($entity, $action, $params) {
-    $apiRequest = Request::create($entity, $action, $params);
-
+    $apiRequest = [];
     try {
+      $apiRequest = Request::create($entity, $action, $params);
       $apiResponse = $this->runRequest($apiRequest);
       return $this->formatResult($apiRequest, $apiResponse);
     }
     catch (\Exception $e) {
-      $this->dispatcher->dispatch(Events::EXCEPTION, new ExceptionEvent($e, NULL, $apiRequest, $this));
+      if ($apiRequest) {
+        $this->dispatcher->dispatch(Events::EXCEPTION, new ExceptionEvent($e, NULL, $apiRequest, $this));
+      }
 
       if ($e instanceof \PEAR_Exception) {
         $err = $this->formatPearException($e, $apiRequest);
diff --git a/Civi/API/Request.php b/Civi/API/Request.php
index ccc970c117..7a053e37c2 100644
--- a/Civi/API/Request.php
+++ b/Civi/API/Request.php
@@ -31,7 +31,7 @@ class Request {
    * @return \Civi\Api4\Generic\AbstractAction|array
    */
   public static function create(string $entity, string $action, array $params) {
-    switch ($params['version']) {
+    switch ($params['version'] ?? NULL) {
       case 3:
         return [
           'id' => self::$nextId++,
@@ -61,7 +61,7 @@ class Request {
         return $apiRequest;
 
       default:
-        throw new \Civi\API\Exception\NotImplementedException("CiviCRM API version {$params['version']} not found.");
+        throw new \Civi\API\Exception\NotImplementedException("Unknown api version");
     }
   }
 
diff --git a/api/api.php b/api/api.php
index 7f7c5167ce..4f6c4585c9 100644
--- a/api/api.php
+++ b/api/api.php
@@ -19,7 +19,7 @@
  *
  * @return array|int
  */
-function civicrm_api(string $entity = NULL, string $action, array $params, $extra = NULL) {
+function civicrm_api(string $entity, string $action, array $params, $extra = NULL) {
   return \Civi::service('civi_api_kernel')->runSafe($entity, $action, $params, $extra);
 }
 
diff --git a/api/v3/Contact.php b/api/v3/Contact.php
index 680583cf1e..00c0bebe1d 100644
--- a/api/v3/Contact.php
+++ b/api/v3/Contact.php
@@ -81,6 +81,9 @@ function civicrm_api3_contact_create($params) {
 
   if (empty($params['contact_type']) && $contactID) {
     $params['contact_type'] = CRM_Contact_BAO_Contact::getContactType($contactID);
+    if (!$params['contact_type']) {
+      throw new API_Exception('Contact id ' . $contactID . ' not found.');
+    }
   }
 
   if (!isset($params['contact_sub_type']) && $contactID) {
diff --git a/api/v3/utils.php b/api/v3/utils.php
index fb3c509cb9..5b7ed12668 100644
--- a/api/v3/utils.php
+++ b/api/v3/utils.php
@@ -171,7 +171,7 @@ function civicrm_api3_create_success($values = 1, $params = [], $entity = NULL,
     }
   }
 
-  if (is_array($params) && !empty($params['debug'])) {
+  if (is_array($params) && $entity && !empty($params['debug'])) {
     if (is_string($action) && $action !== 'getfields') {
       $apiFields = civicrm_api($entity, 'getfields', ['version' => 3, 'action' => $action] + $params);
     }
diff --git a/tests/phpunit/Civi/API/Subscriber/WhitelistSubscriberTest.php b/tests/phpunit/Civi/API/Subscriber/WhitelistSubscriberTest.php
index d7fcd2dc62..2ff64e595d 100644
--- a/tests/phpunit/Civi/API/Subscriber/WhitelistSubscriberTest.php
+++ b/tests/phpunit/Civi/API/Subscriber/WhitelistSubscriberTest.php
@@ -163,8 +163,6 @@ class WhitelistSubscriberTest extends \CiviUnitTestCase {
               0 => ['id' => 1, 'provider' => 'cosmo spacely'],
               1 => ['id' => 5, 'provider' => 'george jetson'],
             ],
-            // This is silly:
-            'undefined_fields' => ['entity_id', 'entity_table', 'widget_id', 'api.has_parent'],
           ],
         ],
       ],
@@ -384,7 +382,6 @@ class WhitelistSubscriberTest extends \CiviUnitTestCase {
     $dispatcher->addSubscriber(new WhitelistSubscriber($whitelist));
     $dispatcher->addSubscriber(new ChainSubscriber());
 
-    $apiRequest['params']['debug'] = 1;
     $apiRequest['params']['check_permissions'] = 'whitelist';
     $result = $kernel->runSafe($apiRequest['entity'], $apiRequest['action'], $apiRequest['params']);
 
diff --git a/tests/phpunit/api/v3/UtilsTest.php b/tests/phpunit/api/v3/UtilsTest.php
index e6f5a52530..9988575b95 100644
--- a/tests/phpunit/api/v3/UtilsTest.php
+++ b/tests/phpunit/api/v3/UtilsTest.php
@@ -95,6 +95,7 @@ class api_v3_UtilsTest extends CiviUnitTestCase {
    *   TRUE or FALSE depending on the outcome of the authorization check
    */
   public function runPermissionCheck($entity, $action, $params, $throws = FALSE) {
+    $params['version'] = 3;
     $dispatcher = new \Symfony\Component\EventDispatcher\EventDispatcher();
     $dispatcher->addSubscriber(new \Civi\API\Subscriber\PermissionCheck());
     $kernel = new \Civi\API\Kernel($dispatcher);
-- 
2.25.1