From: Seamus Lee Date: Sat, 29 Feb 2020 22:32:21 +0000 (+1100) Subject: [MOSS] CIV-01-014 Validate status_id and campaign_type_id for camapginSummary functio... X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=4286fa4566a4a9049f991b18935fa8765c3481d6;p=civicrm-core.git [MOSS] CIV-01-014 Validate status_id and campaign_type_id for camapginSummary function and the source_record_id and activity_type_id for Activity delete function --- diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index b3457a2e3a..fb2b1c0d80 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -149,6 +149,7 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { } $transaction = new CRM_Core_Transaction(); + $sqlWhereParams = $where = []; if (isset($params['source_record_id']) && is_array($params['source_record_id'])) { $sourceRecordIds = implode(',', $params['source_record_id']); } @@ -156,18 +157,19 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { $sourceRecordIds = $params['source_record_id'] ?? NULL; } + if ($sourceRecordIds) { + $where[] = 'source_record_id IN ( %1 )'; + $sqlWhereParams[1] = [$sourceRecordIds, 'CommaSeparatedIntegers']; + } $result = NULL; if (!$moveToTrash) { if (!isset($params['id'])) { - if (is_array($params['activity_type_id'])) { - $activityTypes = implode(',', $params['activity_type_id']); - } - else { - $activityTypes = $params['activity_type_id']; + if (!empty($params['activity_type_id'])) { + $where[] = 'activity_type_id IN ( %2 )'; + $sqlWhereParams[2] = [implode(',', (array) $params['activity_type_id']), 'CommaSeparatedIntegers']; } - - $query = "DELETE FROM civicrm_activity WHERE source_record_id IN ({$sourceRecordIds}) AND activity_type_id IN ( {$activityTypes} )"; - $dao = CRM_Core_DAO::executeQuery($query); + $query = "DELETE FROM civicrm_activity WHERE " . implode(' AND ', $where); + $dao = CRM_Core_DAO::executeQuery($query, $sqlWhereParams); } else { $activity = new CRM_Activity_DAO_Activity(); @@ -178,8 +180,8 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { $activity->case_id = CRM_Case_BAO_Case::getCaseIdByActivityId($activity->id); // CRM-13994 delete activity entity_tag - $query = "DELETE FROM civicrm_entity_tag WHERE entity_table = 'civicrm_activity' AND entity_id = {$activity->id}"; - $dao = CRM_Core_DAO::executeQuery($query); + $query = "DELETE FROM civicrm_entity_tag WHERE entity_table = 'civicrm_activity' AND entity_id = %1"; + $dao = CRM_Core_DAO::executeQuery($query, [1 => [$activity->id, 'Positive']]); } } else { diff --git a/CRM/Campaign/BAO/Campaign.php b/CRM/Campaign/BAO/Campaign.php index 538eec0499..4ceeec2ea8 100644 --- a/CRM/Campaign/BAO/Campaign.php +++ b/CRM/Campaign/BAO/Campaign.php @@ -416,18 +416,12 @@ INNER JOIN civicrm_option_group grp ON ( campaign_type.option_group_id = grp.id $queryParams[6] = ['%' . trim($params['description']) . '%', 'String']; } if (!empty($params['campaign_type_id'])) { - $typeId = $params['campaign_type_id']; - if (is_array($params['campaign_type_id'])) { - $typeId = implode(' , ', $params['campaign_type_id']); - } - $where[] = "( campaign.campaign_type_id IN ( {$typeId} ) )"; + $where[] = "( campaign.campaign_type_id IN ( %7 ) )"; + $queryParams[7] = [implode(',', (array) $params['campaign_type_id']), 'CommaSeparatedIntegers']; } if (!empty($params['status_id'])) { - $statusId = $params['status_id']; - if (is_array($params['status_id'])) { - $statusId = implode(' , ', $params['status_id']); - } - $where[] = "( campaign.status_id IN ( {$statusId} ) )"; + $where[] = "( campaign.status_id IN ( %8 ) )"; + $queryParams[8] = [implode(',', (array) $params['status_id']), 'CommaSeparatedIntegers']; } if (array_key_exists('is_active', $params)) { $active = "( campaign.is_active = 1 )"; diff --git a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php index 2c4a1a8092..1606ab1f71 100644 --- a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php +++ b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php @@ -241,6 +241,24 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'id', 'contact_id', 'Database check for created activity target.' ); + + $paramOptions = ['0))+and+0+--+-f', ['0))+and+0+--+-f']]; + $paramField = ['source_record_id', 'activity_type_id']; + foreach ($paramField as $field) { + foreach ($paramOptions as $paramOption) { + $params = [ + $field => $paramOption, + ]; + try { + CRM_Activity_BAO_Activity::deleteActivity($params); + } + catch (Exception $e) { + if ($e->getMessage() === 'DB Error: syntax error') { + $this->fail('Delete Activity function did not validate field: ' . $field); + } + } + } + } $params = [ 'source_contact_id' => $contactId, 'source_record_id' => $contactId, diff --git a/tests/phpunit/CRM/Campaign/BAO/CampaignTest.php b/tests/phpunit/CRM/Campaign/BAO/CampaignTest.php new file mode 100644 index 0000000000..6166309a56 --- /dev/null +++ b/tests/phpunit/CRM/Campaign/BAO/CampaignTest.php @@ -0,0 +1,42 @@ +createLoggedInUser(); + $contact = $this->individualCreate(); + $this->callAPISuccess('Campaign', 'create', [ + 'title' => 'CiviCRM Unit Test Campaign', + 'campaign_type_id' => 'Direct Mail', + 'status_id' => 'In Progress', + ]); + try { + CRM_Campaign_BAO_Campaign::getCampaignSummary(['status_id' => '0))+and+0+--+-f']); + $this->fail('Campaign Summary should have validated the status_id'); + } + catch (Exception $e) { + if ($e->getMessage() === 'DB Error: syntax error') { + $this->fail('Campaign Summary should have validated the status_id'); + } + } + $this->assertEquals(1, CRM_Campaign_BAO_Campaign::getCampaignSummary(['status_id' => 2], TRUE)); + $this->assertEquals(1, CRM_Campaign_BAO_Campaign::getCampaignSummary(['status_id' => [2, 3]], TRUE)); + } + +}