Merge pull request #8516 from mlutfy/master-crm18756-2

CRM-18756: TrackableURLOpen: fix SQL parameter escaping convention.
civicrm · Aug 7, 2016 · 7a5a698 · 7a5a698
2 parents f0d24a4 + b4fb973
commit 7a5a698
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 18 deletions.
diff --git a/CRM/Mailing/Event/BAO/TrackableURLOpen.php b/CRM/Mailing/Event/BAO/TrackableURLOpen.php
@@ -53,41 +53,57 @@ public function __construct() {
    *   The redirection url, or base url on failure.
    */
   public static function track($queue_id, $url_id) {
-
-    $search = new CRM_Mailing_BAO_TrackableURL();
-
     // To find the url, we also join on the queue and job tables.  This
     // prevents foreign key violations.
-    $job = CRM_Mailing_BAO_MailingJob::getTableName();
-    $eq = CRM_Mailing_Event_BAO_Queue::getTableName();
-    $turl = CRM_Mailing_BAO_TrackableURL::getTableName();
+    $job = CRM_Utils_Type::escape(CRM_Mailing_BAO_MailingJob::getTableName(), 'MysqlColumnNameOrAlias');
+    $eq = CRM_Utils_Type::escape(CRM_Mailing_Event_BAO_Queue::getTableName(), 'MysqlColumnNameOrAlias');
+    $turl = CRM_Utils_Type::escape(CRM_Mailing_BAO_TrackableURL::getTableName(), 'MysqlColumnNameOrAlias');
 
     if (!$queue_id) {
-      $search->query("SELECT $turl.url as url from $turl
-                    WHERE $turl.id = " . CRM_Utils_Type::escape($url_id, 'Integer')
+      $search = CRM_Core_DAO::executeQuery(
+        "SELECT url
+           FROM $turl
+          WHERE $turl.id = %1",
+        array(
+          1 => array($url_id, 'Integer'),
+        )
       );
+
       if (!$search->fetch()) {
         return CRM_Utils_System::baseURL();
       }
+
       return $search->url;
     }
 
-    $search->query("SELECT $turl.url as url from $turl
-                    INNER JOIN $job ON $turl.mailing_id = $job.mailing_id
-                    INNER JOIN $eq ON $job.id = $eq.job_id
-                    WHERE $eq.id = " . CRM_Utils_Type::escape($queue_id, 'Integer') . " AND $turl.id = " . CRM_Utils_Type::escape($url_id, 'Integer')
+    $search = CRM_Core_DAO::executeQuery(
+      "SELECT $turl.url as url
+         FROM $turl
+        INNER JOIN $job ON $turl.mailing_id = $job.mailing_id
+        INNER JOIN $eq ON $job.id = $eq.job_id
+        WHERE $eq.id = %1 AND $turl.id = %2",
+      array(
+        1 => array($queue_id, 'Integer'),
+        2 => array($url_id, 'Integer'),
+      )
     );
 
     if (!$search->fetch()) {
       // Can't find either the URL or the queue. If we can find the URL then
       // return the URL without tracking.  Otherwise return the base URL.
-
-      $search->query("SELECT $turl.url as url from $turl
-                    WHERE $turl.id = " . CRM_Utils_Type::escape($url_id, 'Integer')
+      $search = CRM_Core_DAO::executeQuery(
+        "SELECT $turl.url as url
+           FROM $turl
+          WHERE $turl.id = %1",
+        array(
+          1 => array($url_id, 'Integer'),
+        )
       );
+
       if (!$search->fetch()) {
         return CRM_Utils_System::baseURL();
       }
+
       return $search->url;
     }
 

diff --git a/tests/phpunit/api/v3/MailingTest.php b/tests/phpunit/api/v3/MailingTest.php
@@ -53,8 +53,8 @@ public function setUp() {
     $this->_email = 'test@test.test';
     $this->_params = array(
       'subject' => 'Hello {contact.display_name}',
-      'body_text' => "This is {contact.display_name}.\n{domain.address}{action.optOutUrl}",
-      'body_html' => "<p>This is {contact.display_name}.</p><p>{domain.address}{action.optOutUrl}</p>",
+      'body_text' => "This is {contact.display_name}.\nhttps://civicrm.org\n{domain.address}{action.optOutUrl}",
+      'body_html' => "<p>This is {contact.display_name}.</p><p><a href='https://civicrm.org/'>CiviCRM.org</a></p><p>{domain.address}{action.optOutUrl}</p>",
       'name' => 'mailing name',
       'created_id' => $this->_contactID,
       'header_id' => '',
@@ -658,6 +658,35 @@ public function createDraftMailing($params = array()) {
     return $createResult['id'];
   }
 
-  //----------- civicrm_mailing_create ----------
+  /**
+   * Test to make sure that if the event queue hashes have been archived,
+   * we can still have working click-trough URLs working (CRM-17959).
+   */
+  public function testUrlWithMissingTrackingHash() {
+    $mail = $this->callAPIAndDocument('mailing', 'create', $this->_params + array('scheduled_date' => 'now'), __FUNCTION__, __FILE__);
+    $jobs = $this->callAPISuccess('mailing_job', 'get', array('mailing_id' => $mail['id']));
+    $this->assertEquals(1, $jobs['count']);
+
+    $params = array('mailing_id' => $mail['id'], 'test_email' => 'alice@example.org', 'test_group' => NULL);
+    $deliveredInfo = $this->callAPISuccess($this->_entity, 'send_test', $params);
+
+    $sql = "SELECT turl.id as url_id, turl.url, q.id as queue_id
+      FROM civicrm_mailing_trackable_url as turl
+      INNER JOIN civicrm_mailing_job as j ON turl.mailing_id = j.mailing_id
+      INNER JOIN civicrm_mailing_event_queue q ON j.id = q.job_id
+      ORDER BY turl.id DESC LIMIT 1";
+
+    $dao = CRM_Core_DAO::executeQuery($sql);
+    $this->assertTrue($dao->fetch());
+
+    $url = CRM_Mailing_Event_BAO_TrackableURLOpen::track($dao->queue_id, $dao->url_id);
+    $this->assertContains('https://civicrm.org', $url);
+
+    // Now delete the event queue hashes and see if the tracking still works.
+    CRM_Core_DAO::executeQuery('DELETE FROM civicrm_mailing_event_queue');
+
+    $url = CRM_Mailing_Event_BAO_TrackableURLOpen::track($dao->queue_id, $dao->url_id);
+    $this->assertContains('https://civicrm.org', $url);
+  }
 
 }