Merge pull request #662 from mailpoet/security_issue_634

PHP Object injection in front Router

assets/js/src/subscribers/importExport/import.js

@@ -7,7 +7,6 @@ define(
       'handlebars',
       'papaparse',
       'asyncqueue',
-      'xss',
       'moment',
       'select2'
     ],
@@ -19,7 +18,6 @@ define(
       Handlebars,
       Papa,
       AsyncQueue,
-      xss,
       Moment
     ) {
       if (!jQuery('#mailpoet_subscribers_import').length) {
@@ -337,9 +335,9 @@ define(
               complete: function (CSV) {
                 for (var rowCount in CSV.data) {
                   var rowData = CSV.data[rowCount].map(function (el) {
-                        return filterXSS(el.trim());
-                      }),
-                      rowColumnCount = rowData.length;
+                    return el.trim();
+                  });
+                  var rowColumnCount = rowData.length;
                   // set the number of row elements based on the first non-empty row
                   if (columnCount === null) {
                     columnCount = rowColumnCount;
@@ -582,7 +580,8 @@ define(
               }).done(function(response) {
                 mailpoetSegments.push({
                   'id': response.data.id,
-                  'name': response.data.name
+                  'name': response.data.name,
+                  'subscriberCount': 0
                 });
 
                 var selected_values = segmentSelectElement.val();
@@ -669,8 +668,15 @@ define(
                 return options.fn(displayedColumns);
               });
 
+          // sanitize unsafe data
+          Handlebars.registerHelper('sanitize_data', function(data) {
+            return (data instanceof Handlebars.SafeString) ?
+              data :
+              new Handlebars.SafeString(Handlebars.Utils.escapeExpression(data));
+          });
+
           // start array index from 1
-          Handlebars.registerHelper('show_real_index', function (index) {
+          Handlebars.registerHelper('calculate_index', function (index) {
             var index = parseInt(index);
             // display filler data (e.g., ellipsis) if we've reached the maximum number of rows and
             // subscribers count is greater than the maximum number of rows we're displaying
@@ -879,7 +885,9 @@ define(
                       jQuery(matchedColumn.element).data('validation-rule', validationRule);
                       break;
                     }
-                    if (validationRule === 'datetime') validationRule = Moment.ISO_8601;
+                    if (validationRule === 'datetime') {
+                      validationRule = Moment.ISO_8601;
+                    }
                   }
                 }
                 jQuery.map(subscribersClone.subscribers, function (data, index) {
@@ -888,18 +896,22 @@ define(
                   var date = Moment(rowData, testedFormat, true);
                   // validate date
                   if (date.isValid()) {
-                    data[matchedColumn.index] +=
-                        '<span class="mailpoet_data_match" title="'
+                    data[matchedColumn.index] = new Handlebars.SafeString(
+                      Handlebars.Utils.escapeExpression(data[matchedColumn.index])
+                      + '<span class="mailpoet_data_match" title="'
                       + MailPoet.I18n.t('verifyDateMatch') + '">'
                       + MailPoet.Date.format(date)
-                        + '</span>';
+                      + '</span>'
+                    );
                   }
                   else {
-                    data[matchedColumn.index] +=
-                      '<span class="mailpoet_data_match mailpoet_import_error" title="'
+                    data[matchedColumn.index] = new Handlebars.SafeString(
+                      Handlebars.Utils.escapeExpression(data[matchedColumn.index])
+                      + '<span class="mailpoet_data_match mailpoet_import_error" title="'
                       + MailPoet.I18n.t('noDateFieldMatch') + '">'
-                      + MailPoet.I18n.t('dateMatchError')
-                      + '</span>';
+                      + (new Handlebars.SafeString(MailPoet.I18n.t('dateMatchError')))
+                      + '</span>'
+                    );
                     preventNextStep = true;
                   };
                 });

lib/Cron/Workers/SendingQueue/Tasks/Posts.php

@@ -20,10 +20,10 @@ class Posts {
       $newsletter->parent_id :
       $newsletter->id;
     foreach($matched_posts_ids as $post_id) {
-      $newletter_post = NewsletterPost::create();
-      $newletter_post->newsletter_id = $newsletter_id;
-      $newletter_post->post_id = $post_id;
-      $newletter_post->save();
+      $newsletter_post = NewsletterPost::create();
+      $newsletter_post->newsletter_id = $newsletter_id;
+      $newsletter_post->post_id = $post_id;
+      $newsletter_post->save();
     }
     return true;
   }

lib/Mailer/Methods/AmazonSES.php

@@ -17,11 +17,19 @@ class AmazonSES {
   public $reply_to;
   public $date;
   public $date_without_time;
+  const SES_REGIONS = array(
+    'US East (N. Virginia)' => 'us-east-1',
+    'US West (Oregon)' => 'us-west-2',
+    'EU (Ireland)' => 'eu-west-1'
+  );
 
   function __construct($region, $access_key, $secret_key, $sender, $reply_to) {
     $this->aws_access_key = $access_key;
     $this->aws_secret_key = $secret_key;
-    $this->aws_region = $region;
+    $this->aws_region = (in_array($region, self::SES_REGIONS)) ? $region : false;
+    if(!$this->aws_region) {
+      throw new \Exception(__('Unsupported Amazon SES region.', 'mailpoet'));
+    }
     $this->aws_endpoint = sprintf('email.%s.amazonaws.com', $this->aws_region);
     $this->aws_signing_algorithm = 'AWS4-HMAC-SHA256';
     $this->aws_service = 'ses';

lib/Router/Router.php

@@ -46,10 +46,7 @@ class Router {
   }
 
   static function decodeRequestData($data) {
-    $data = base64_decode($data);
-    if(is_serialized($data)) {
-      $data = unserialize($data);
-    }
+    $data = json_decode(base64_decode($data), true);
     if(!is_array($data)) {
       $data = array();
     }
@@ -57,7 +54,7 @@ class Router {
   }
 
   static function encodeRequestData($data) {
-    return rtrim(base64_encode(serialize($data)), '=');
+    return rtrim(base64_encode(json_encode($data)), '=');
   }
 
   static function buildRequest($endpoint, $action, $data) {

lib/Settings/Hosts.php

@@ -1,6 +1,8 @@
 <?php
 namespace MailPoet\Settings;
 
+use MailPoet\Mailer\Methods\AmazonSES;
+
 class Hosts {
   private static $_smtp = array(
     'AmazonSES' => array(
@@ -12,11 +14,7 @@ class Hosts {
         'access_key',
         'secret_key'
       ),
-      'regions' => array(
-        'US East (N. Virginia)' => 'us-east-1',
-        'US West (Oregon)' => 'us-west-2',
-        'EU (Ireland)' => 'eu-west-1'
-      )
+      'regions' => AmazonSES::SES_REGIONS
     ),
     'SendGrid' => array(
       'name' => 'SendGrid',

lib/Subscribers/ImportExport/Import/Import.php

@@ -21,6 +21,7 @@ class Import {
   public $updated_at;
 
   public function __construct($data) {
+    $this->validateData($data);
     $this->subscribers_data = $this->transformSubscribersData(
       $data['subscribers'],
       $data['columns']
@@ -41,6 +42,23 @@ class Import {
     $this->updated_at = date('Y-m-d H:i:s', (int)$data['timestamp'] + 1);
   }
 
+  function validateData($data) {
+    $required_data_fields = array(
+      'subscribers',
+      'columns',
+      'segments',
+      'timestamp',
+      'updateSubscribers'
+    );
+    // 1. data should contain all required fields
+    // 2. column names should only contain alphanumeric & underscore characters
+    if(count(array_intersect_key(array_flip($required_data_fields), $data)) !== count($required_data_fields) ||
+       preg_grep('/[^a-zA-Z0-9_]/', array_keys($data['columns']))
+    ) {
+      throw new \Exception(__('Missing or invalid subscriber data.', 'mailpoet'));
+    }
+  }
+
   function getSubscriberFieldsValidationRules($subscriber_fields) {
     $validation_rules = array();
     foreach($subscriber_fields as $column => $field) {
@@ -89,8 +107,8 @@ class Import {
           $this->synchronizeWPUsers($wp_users);
         }
       }
-    } catch(\PDOException $e) {
-      throw new \Exception($e->getMessage());
+    } catch(\Exception $e) {
+      throw new \Exception(__('Unable to save imported subscribers.', 'mailpoet'));
     }
     $import_factory = new ImportExportFactory('import');
     $segments = $import_factory->getSegments();
@@ -364,6 +382,11 @@ class Import {
     $subscribers_data,
     $subscriber_custom_fields
   ) {
+    // check if custom fields exist in the database
+    $subscriber_custom_fields = Helpers::flattenArray(
+      CustomField::whereIn('id', $subscriber_custom_fields)->select('id')->findArray()
+    );
+    if(!$subscriber_custom_fields) return;
     $subscribers = array_map(
       function($column) use ($db_subscribers, $subscribers_data) {
         $count = range(0, count($subscribers_data[$column]) - 1);

lib/Subscribers/ImportExport/Import/MailChimp.php

@@ -4,8 +4,14 @@ namespace MailPoet\Subscribers\ImportExport\Import;
 use MailPoet\Util\Helpers;
 
 class MailChimp {
-  function __construct($APIKey, $lists = false) {
-    $this->api_key = $this->getAPIKey($APIKey);
+  public $api_key;
+  public $max_post_size;
+  public $data_center;
+  public $export_url;
+  const API_KEY_REGEX = '/[a-zA-Z0-9]{32}-[a-zA-Z0-9]{2,3}$/';
+
+  function __construct($api_key, $lists = false) {
+    $this->api_key = $this->getAPIKey($api_key);
     $this->max_post_size = Helpers::getMaxPostSize('bytes');
     $this->data_center = $this->getDataCenter($this->api_key);
     $this->lists_url = 'https://%s.api.mailchimp.com/2.0/lists/list?apikey=%s';
@@ -108,15 +114,14 @@ class MailChimp {
     );
   }
 
-  function getDataCenter($APIKey) {
-    if(!preg_match('/-[a-zA-Z0-9]{3,}/', $APIKey)) return false;
-    // double parantheses: http://phpsadness.com/sad/51
-    $key_parts = explode('-', $APIKey);
-    return end($key_parts);
+  function getDataCenter($api_key) {
+    if(!$api_key) return false;
+    $api_key_parts = explode('-', $api_key);
+    return end($api_key_parts);
   }
 
-  function getAPIKey($APIKey) {
-    return (preg_match('/[a-zA-Z0-9]{32}-[a-zA-Z0-9]{3,}/', $APIKey)) ? $APIKey : false;
+  function getAPIKey($api_key) {
+    return (preg_match(self::API_KEY_REGEX, $api_key)) ? $api_key : false;
   }
 
   function throwException($error) {

lib/Util/CSS.php

@@ -29,51 +29,13 @@ use csstidy;
 */
 
 class CSS {
-
-  private $cssFiles = array();
   private $parsed_css = array();
 
-  /*
-  * Retrieves a CSS stylesheet and caches it before returning it.
-  */
-  public function getCSS($url)
-  {
-    if(!isset($cssFiles[$url]))
-    {
-      $cssFiles[$url] = file_get_contents($url);
-    }
-    return $cssFiles[$url];
-  }
-
-  /*
-  * Take a list of absolute URLs pointing to CSS stylesheets,
-  * retrieve the CSS, parse it, sort the rules by increasing order of specificity,
-  * cache the rules, return them.
-  */
-  public function getCSSFromFiles($urls)
-  {
-    $key = implode('::', $urls);
-    if(!isset($this->parsed_css[$key]))
-    {
-      $texts = array();
-      foreach($urls as $url)
-      {
-        $texts[] = $this->getCSS($url);
-      }
-      $text = implode("\n\n", $texts);
-      $this->parsed_css[$key] = $text;
-    }
-    return $this->parsed_css[$key];
-  }
-
-  public static function splitMediaQueries($css)
-  {
-
+  public static function splitMediaQueries($css) {
     $start = 0;
     $queries = '';
 
-    while (($start = strpos($css, "@media", $start)) !== false)
-    {
+    while(($start = strpos($css, "@media", $start)) !== false) {
       // stack to manage brackets
       $s = array();
 
@@ -81,23 +43,18 @@ class CSS {
       $i = strpos($css, "{", $start);
 
       // if $i is false, then there is probably a css syntax error
-      if ($i !== false)
-      {
+      if($i !== false) {
         // push bracket onto stack
         array_push($s, $css[$i]);
 
         // move past first bracket
         $i++;
 
-        while (!empty($s))
-        {
+        while(!empty($s)) {
           // if the character is an opening bracket, push it onto the stack, otherwise pop the stack
-          if ($css[$i] == "{")
-          {
+          if($css[$i] == "{") {
             array_push($s, "{");
-          }
-          elseif ($css[$i] == "}")
-          {
+          } else if($css[$i] == "}") {
             array_pop($s);
           }
 
@@ -113,8 +70,7 @@ class CSS {
     return array($css, $queries);
   }
 
-  public function parseCSS($text)
-  {
+  public function parseCSS($text) {
     $css  = new csstidy();
     $css->settings['compress_colors'] = false;
     $css->parse($text);
@@ -122,12 +78,9 @@ class CSS {
     $rules    = array();
     $position   = 0;
 
-    foreach($css->css as $declarations)
-    {
-      foreach($declarations as $selectors => $properties)
-      {
-        foreach(explode(",", $selectors) as $selector)
-        {
+    foreach($css->css as $declarations) {
+      foreach($declarations as $selectors => $properties) {
+        foreach(explode(",", $selectors) as $selector) {
           $rules[] = array(
             'position'    => $position,
             'specificity'   => self::calculateCSSSpecifity($selector),
@@ -140,23 +93,15 @@ class CSS {
       }
     }
 
-    usort($rules, function($a, $b){
-      if($a['specificity'] > $b['specificity'])
-      {
+    usort($rules, function($a, $b) {
+      if($a['specificity'] > $b['specificity']) {
         return 1;
-      }
-      else if($a['specificity'] < $b['specificity'])
-      {
+      } else if($a['specificity'] < $b['specificity']) {
         return -1;
-      }
-      else
-      {
-        if($a['position'] > $b['position'])
-        {
+      } else {
+        if($a['position'] > $b['position']) {
           return 1;
-        }
-        else
-        {
+        } else {
           return -1;
         }
       }
@@ -176,8 +121,7 @@ class CSS {
    * @license   BSD License
    */
 
-  public static function calculateCSSSpecifity($selector)
-  {
+  public static function calculateCSSSpecifity($selector) {
       // cleanup selector
     $selector = str_replace(array('>', '+'), array(' > ', ' + '), $selector);
 
@@ -207,16 +151,15 @@ class CSS {
   * Turns a CSS style string (like: "border: 1px solid black; color:red")
   * into an array of properties (like: array("border" => "1px solid black", "color" => "red"))
   */
-  public static function styleToArray($str)
-  {
+  public static function styleToArray($str) {
     $array = array();
 
-    if(trim($str) === '')return $array;
+    if(trim($str) === '') return $array;
 
-    foreach(explode(';', $str) as $kv)
-    {
-      if ($kv === '')
+    foreach(explode(';', $str) as $kv) {
+      if($kv === '') {
         continue;
+      }
 
       $key_value = explode(':', $kv);
       $array[trim($key_value[0])] = trim($key_value[1]);
@@ -229,52 +172,14 @@ class CSS {
   * Reverses what styleToArray does, see above.
   * array("border" => "1px solid black", "color" => "red") yields "border: 1px solid black; color:red"
   */
-  public static function arrayToStyle($array)
-  {
+  public static function arrayToStyle($array) {
     $parts = array();
-    foreach($array as $k => $v)
-    {
+    foreach($array as $k => $v) {
       $parts[] = "$k:$v";
     }
     return implode(';', $parts);
   }
 
-  /*
-  * Get an absolute URL from an URL ($relative_url, but relative or not actually!)
-  * that is found on the page with url $page_url.
-  * Determine it as a browser would do. For instance if "<a href='/bob/hello.html'>hi</a>"
-  * (here '/bob/hello.html' is the $relative_url)
-  * is found on a page at $page_url := "http://example.com/stuff/index.html"
-  * then the function returns "http://example.com/bob/hello.html"
-  * because that's where you'd go to if you clicked on the link in your browser.
-  * This is used to find where to download the CSS files from when inlining.
-  */
-  public static function absolutify($page_url, $relative_url)
-  {
-    $parsed_url = parse_url($page_url);
-
-    $absolute_url       = '';
-    $parsed_relative_url  = parse_url($relative_url);
-
-    // If $relative_url has a host it is actually absolute, return it.
-    if(isset($parsed_relative_url['host']))
-    {
-      $absolute_url = $relative_url;
-    }
-    // If $relative_url begins with / then it is a path relative to the $page_url's host
-    else if(preg_match('/^\//', $parsed_relative_url['path']))
-    {
-      $absolute_url = $parsed_url['scheme'].'://'.$parsed_url['host'].$parsed_relative_url['path'];
-    }
-    // No leading slash: append the path of $relative_url to the 'folder' path of $page_url
-    else
-    {
-      $absolute_url = $parsed_url['scheme'].'://'.$parsed_url['host'].dirname($parsed_url['path']).'/'.$parsed_relative_url['path'];
-    }
-
-    return $absolute_url;
-  }
-
   /*
   * The core of the algorithm, takes a URL and returns the HTML found there with the CSS inlined.
   * If you pass $contents then the original HTML is not downloaded and $contents is used instead.
@@ -283,38 +188,24 @@ class CSS {
   function inlineCSS($url, $contents=null)
   {
     // Download the HTML if it was not provided
-    if($contents === null)
-    {
+    if($contents === null) {
       $html = HtmlDomParser::file_get_html($url, false, null, -1, -1, true, true, DEFAULT_TARGET_CHARSET, false, DEFAULT_BR_TEXT, DEFAULT_SPAN_TEXT);
-    }
-    // Else use the data provided!
-    else
-    {
+    } else {
+      // use the data provided!
       $html = HtmlDomParser::str_get_html($contents, true, true, DEFAULT_TARGET_CHARSET, false, DEFAULT_BR_TEXT, DEFAULT_SPAN_TEXT);
     }
 
-    if(!is_object($html))
-    {
+    if(!is_object($html)) {
       return false;
     }
 
-    $css_urls = array();
-
-    // Find all stylesheets and determine their absolute URLs to retrieve them
-    foreach($html->find('link[rel="stylesheet"]') as $style)
-    {
-      $css_urls[] = self::absolutify($url, $style->href);
-      $style->outertext = '';
-    }
-
     $css_blocks = '';
 
     // Find all <style> blocks and cut styles from them (leaving media queries)
-    foreach($html->find('style') as $style)
-    {
+    foreach($html->find('style') as $style) {
       list($_css_to_parse, $_css_to_keep) = self::splitMediaQueries($style->innertext());
       $css_blocks .= $_css_to_parse;
-      if (!empty($_css_to_keep)) {
+      if(!empty($_css_to_keep)) {
         $style->innertext = $_css_to_keep;
       } else {
         $style->outertext = '';
@@ -322,10 +213,7 @@ class CSS {
     }
 
     $raw_css = '';
-    if (!empty($css_urls)) {
-      $raw_css .= $this->getCSSFromFiles($css_urls);
-    }
-    if (!empty($css_blocks)) {
+    if(!empty($css_blocks)) {
       $raw_css .= $css_blocks;
     }
 
@@ -336,10 +224,8 @@ class CSS {
 
     // We loop over each rule by increasing order of specificity, find the nodes matching the selector
     // and apply the CSS properties
-    foreach ($rules as $rule)
-    {
-      foreach($html->find($rule['selector']) as $node)
-      {
+    foreach ($rules as $rule) {
+      foreach($html->find($rule['selector']) as $node) {
         // I'm leaving this for debug purposes, it has proved useful.
         /*
         if($node->already_styled === 'yes')
@@ -357,11 +243,11 @@ class CSS {
         }//*/
 
         // Unserialize the style array, merge the rule's CSS into it...
-        $nodeStyles = self::styleToArray( $node->style );
-        $style = array_merge( $nodeStyles, $rule[ 'properties' ] );
+        $nodeStyles = self::styleToArray($node->style);
+        $style = array_merge($nodeStyles, $rule['properties']);
 
         // !important node styles should take precedence over other styles
-        $style = array_merge( $style, preg_grep( "/important/i", $nodeStyles ) );
+        $style = array_merge($style, preg_grep("/important/i", $nodeStyles));
 
         // And put the CSS back as a string!
         $node->style = self::arrayToStyle($style);
@@ -378,14 +264,10 @@ class CSS {
     // Now a tricky part: do a second pass with only stuff marked !important
     // because !important properties do not care about specificity, except when fighting
     // against another !important property
-    foreach ($rules as $rule)
-    {
-      foreach($rule['properties'] as $key => $value)
-      {
-        if(strpos($value, '!important') !== false)
-        {
-          foreach($html->find($rule['selector']) as $node)
-          {
+    foreach ($rules as $rule) {
+      foreach($rule['properties'] as $key => $value) {
+        if(strpos($value, '!important') !== false) {
+          foreach($html->find($rule['selector']) as $node) {
             $style = self::styleToArray($node->style);
             $style[$key] = $value;
             $node->style = self::arrayToStyle($style);

tests/unit/Mailer/Methods/AmazonSESTest.php

@@ -56,6 +56,21 @@ class AmazonSESTest extends MailPoetTest {
     expect(preg_match('!^\d{8}$!', $this->mailer->date_without_time))->equals(1);
   }
 
+  function testItChecksForValidRegion() {
+    try {
+      $mailer = new AmazonSES(
+        'random_region',
+        $this->settings['access_key'],
+        $this->settings['secret_key'],
+        $this->sender,
+        $this->reply_to
+      );
+      $this->fail('Unsupported region exception was not thrown');
+    } catch(\Exception $e) {
+      expect($e->getMessage())->equals('Unsupported Amazon SES region.');
+    }
+  }
+
   function testItCanGenerateBody() {
     $body = $this->mailer->getBody($this->newsletter, $this->subscriber);
     expect($body['Action'])->equals('SendEmail');

tests/unit/Newsletter/Links/LinksTest.php

@@ -66,10 +66,10 @@ class LinksTest extends MailPoetTest {
     expect($result)
       ->regExp('/<img src="http.*?' . Router::NAME . '&endpoint=track&action=open&data=.*?>/');
 
-    // data was base64encoded, serialized and contains an array of variables
+    // data was properly encoded
     preg_match_all('/data=(?P<data>.*?)"/', $result, $result);
     foreach($result['data'] as $data) {
-      $data = unserialize(base64_decode($data));
+      $data = Router::decodeRequestData($data);
       expect($data['subscriber_id'])->equals($subscriber->id);
       expect($data['queue_id'])->equals($queue->id);
       expect(isset($data['subscriber_token']))->true();
@@ -85,7 +85,7 @@ class LinksTest extends MailPoetTest {
     );
     Links::save(
       $links,
-      $newletter_id = 1,
+      $newsletter_id = 1,
       $queue_id = 1
     );
 

tests/unit/Newsletter/Scheduler/SchedulerTest.php

@@ -0,0 +1,455 @@
+
+<?php
+
+use Carbon\Carbon;
+use MailPoet\Models\Newsletter;
+use MailPoet\Models\NewsletterOption;
+use MailPoet\Models\NewsletterOptionField;
+use MailPoet\Models\NewsletterPost;
+use MailPoet\Models\SendingQueue;
+use MailPoet\Newsletter\Scheduler\Scheduler;
+
+class NewsletterSchedulerTest extends MailPoetTest {
+ function testItSetsConstants() {
+    expect(Scheduler::SECONDS_IN_HOUR)->notEmpty();
+    expect(Scheduler::LAST_WEEKDAY_FORMAT)->notEmpty();
+    expect(Scheduler::WORDPRESS_ALL_ROLES)->notEmpty();
+    expect(Scheduler::INTERVAL_IMMEDIATELY)->notEmpty();
+    expect(Scheduler::INTERVAL_IMMEDIATE)->notEmpty();
+    expect(Scheduler::INTERVAL_DAILY)->notEmpty();
+    expect(Scheduler::INTERVAL_WEEKLY)->notEmpty();
+    expect(Scheduler::INTERVAL_MONTHLY)->notEmpty();
+    expect(Scheduler::INTERVAL_NTHWEEKDAY)->notEmpty();
+  }
+
+  function testItGetsActiveNewslettersFilteredByType() {
+    $newsletter = $this->_createNewsletter($type = Newsletter::TYPE_WELCOME);
+
+    // no newsletters wtih type "notification" should be found
+    expect(Scheduler::getNewsletters(Newsletter::TYPE_NOTIFICATION))->isEmpty();
+
+    // one newsletter with type "welcome" should be found
+    expect(Scheduler::getNewsletters(Newsletter::TYPE_WELCOME))->count(1);
+  }
+
+  function testItCanGetNextRunDate() {
+    // it accepts cron syntax and returns next run date
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+    expect(Scheduler::getNextRunDate('* * * * *'))
+      ->equals($current_time->addMinute()->format('Y-m-d H:i:00'));
+  }
+
+  function testItFormatsDatetimeString() {
+    expect(Scheduler::formatDatetimeString('April 20, 2016 4pm'))
+      ->equals('2016-04-20 16:00:00');
+  }
+
+  function testItCreatesPostNotificationQueueRecord() {
+    $newsletter = $this->_createNewsletter();
+    $newsletter->schedule = '* 5 * * *';
+
+    // new queue record should be created
+    $queue = Scheduler::createPostNotificationQueue($newsletter);
+    expect(SendingQueue::findMany())->count(1);
+    expect($queue->newsletter_id)->equals($newsletter->id);
+    expect($queue->status)->equals(SendingQueue::STATUS_SCHEDULED);
+    expect($queue->scheduled_at)->equals(Scheduler::getNextRunDate('* 5 * * *'));
+
+    // duplicate queue record should not be created
+    Scheduler::createPostNotificationQueue($newsletter);
+    expect(SendingQueue::findMany())->count(1);
+  }
+
+  function testItCreatesWelcomeNotificationQueueRecord() {
+    $newsletter = (object)array(
+      'id' => 1,
+      'afterTimeNumber' => 2
+    );
+
+    // queue is scheduled delivery in 2 hours
+    $newsletter->afterTimeType = 'hours';
+    Scheduler::createWelcomeNotificationQueue($newsletter, $subscriber_id = 1);
+    $queue = SendingQueue::where('newsletter_id', 1)
+      ->findOne();
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+    expect($queue->id)->greaterOrEquals(1);
+    expect(Carbon::parse($queue->scheduled_at)->format('Y-m-d H:i'))
+      ->equals($current_time->addHours(2)->format('Y-m-d H:i'));
+    $this->_after();
+
+    // queue is scheduled for delivery in 2 days
+    $newsletter->afterTimeType = 'days';
+    Scheduler::createWelcomeNotificationQueue($newsletter, $subscriber_id = 1);
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+    $queue = SendingQueue::where('newsletter_id', 1)
+      ->findOne();
+    expect($queue->id)->greaterOrEquals(1);
+    expect(Carbon::parse($queue->scheduled_at)->format('Y-m-d H:i'))
+      ->equals($current_time->addDays(2)->format('Y-m-d H:i'));
+    $this->_after();
+
+    // queue is scheduled for delivery in 2 weeks
+    $newsletter->afterTimeType = 'weeks';
+    Scheduler::createWelcomeNotificationQueue($newsletter, $subscriber_id = 1);
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+    $queue = SendingQueue::where('newsletter_id', 1)
+      ->findOne();
+    expect($queue->id)->greaterOrEquals(1);
+    expect(Carbon::parse($queue->scheduled_at)->format('Y-m-d H:i'))
+      ->equals($current_time->addWeeks(2)->format('Y-m-d H:i'));
+    $this->_after();
+
+    // queue is scheduled for immediate delivery
+    $newsletter->afterTimeType = null;
+    Scheduler::createWelcomeNotificationQueue($newsletter, $subscriber_id = 1);
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+    $queue = SendingQueue::where('newsletter_id', 1)
+      ->findOne();
+    expect($queue->id)->greaterOrEquals(1);
+    expect(Carbon::parse($queue->scheduled_at)->format('Y-m-d H:i'))
+      ->equals($current_time->format('Y-m-d H:i'));
+  }
+
+  function tesIttDoesNotSchedulePostNotificationWhenNotificationWasAlreadySentForPost() {
+    $newsletter = $this->_createNewsletter();
+    $newsletter_post = NewsletterPost::create();
+    $newsletter_post->newsletter_id = $newsletter->id;
+    $newsletter_post->post_id = 10;
+    $newsletter_post->save();
+
+    // queue is not created when notification was already sent for the post
+    Scheduler::schedulePostNotification($post_id = 10);
+    $queue = SendingQueue::where('newsletter_id', $newsletter->id)
+      ->findOne();
+    expect($queue)->false();
+  }
+
+  function testItSchedulesPostNotification() {
+    $newsletter = $this->_createNewsletter();
+    $this->_createNewsletterOptions(
+      $newsletter->id,
+      Newsletter::TYPE_NOTIFICATION,
+      array(
+        'schedule' => '* 5 * * *'
+      )
+    );
+
+    // queue is created and scheduled for delivery one day later at 5 a.m.
+    Scheduler::schedulePostNotification($post_id = 10);
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+    $next_run_date = ($current_time->hour < 5) ?
+      $current_time :
+      $current_time->addDay();
+    $queue = SendingQueue::where('newsletter_id', $newsletter->id)
+      ->findOne();
+    expect($queue->scheduled_at)->equals($next_run_date->format('Y-m-d 05:00:00'));
+  }
+
+  function testItDoesNotSchedulesSubscriberWelcomeNotificationWhenSubscriberIsNotInSegment() {
+    // do not schedule when subscriber is not in segment
+    $newsletter = $this->_createNewsletter(Newsletter::TYPE_WELCOME);
+    Scheduler::scheduleSubscriberWelcomeNotification(
+      $subscriber_id = 10,
+      $segments = array()
+    );
+
+    // queue is not created
+    $queue = SendingQueue::where('newsletter_id', $newsletter->id)
+      ->findOne();
+    expect($queue)->false();
+  }
+
+  function testItSchedulesSubscriberWelcomeNotification() {
+    $newsletter = $this->_createNewsletter(Newsletter::TYPE_WELCOME);
+    $this->_createNewsletterOptions(
+      $newsletter->id,
+      Newsletter::TYPE_WELCOME,
+      array(
+        'event' => 'segment',
+        'segment' => 2,
+        'afterTimeType' => 'days',
+        'afterTimeNumber' => 1
+      )
+    );
+
+    // queue is created and scheduled for delivery one day later
+    Scheduler::scheduleSubscriberWelcomeNotification(
+      $subscriber_id = 10,
+      $segments = array(
+        3,
+        2,
+        1
+      )
+    );
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+    $queue = SendingQueue::where('newsletter_id', $newsletter->id)
+      ->findOne();
+    expect(Carbon::parse($queue->scheduled_at)->format('Y-m-d H:i'))
+      ->equals($current_time->addDay()->format('Y-m-d H:i'));
+  }
+
+  function itDoesNotScheduleAnythingWhenNewsletterDoesNotExist() {
+    // post notification is not scheduled
+    expect(Scheduler::schedulePostNotification($post_id = 10))->false();
+
+    // subscriber welcome notification is not scheduled
+    $result = Scheduler::scheduleSubscriberWelcomeNotification(
+      $subscriber_id = 10,
+      $segments = array()
+    );
+    expect($result)->false();
+
+    // WP user welcome notification is not scheduled
+    $result = Scheduler::scheduleSubscriberWelcomeNotification(
+      $subscriber_id = 10,
+      $segments = array()
+    );
+    expect($result)->false();
+  }
+
+  function testItDoesNotScheduleWPUserWelcomeNotificationWhenRoleHasNotChanged() {
+    $newsletter = $this->_createNewsletter(Newsletter::TYPE_WELCOME);
+    $this->_createNewsletterOptions(
+      $newsletter->id,
+      Newsletter::TYPE_WELCOME,
+      array(
+        'event' => 'user',
+        'role' => 'editor',
+        'afterTimeType' => 'days',
+        'afterTimeNumber' => 1
+      )
+    );
+    Scheduler::scheduleWPUserWelcomeNotification(
+      $subscriber_id = 10,
+      $wp_user = (object)array('roles' => array('editor')),
+      $old_user_data = (object)array('roles' => array('editor'))
+    );
+
+    // queue is not created
+    $queue = SendingQueue::where('newsletter_id', $newsletter->id)
+      ->findOne();
+    expect($queue)->false();
+  }
+
+  function testItDoesNotScheduleWPUserWelcomeNotificationWhenUserRoleDoesNotMatch() {
+    $newsletter = $this->_createNewsletter(Newsletter::TYPE_WELCOME);
+    $this->_createNewsletterOptions(
+      $newsletter->id,
+      Newsletter::TYPE_WELCOME,
+      array(
+        'event' => 'user',
+        'role' => 'editor',
+        'afterTimeType' => 'days',
+        'afterTimeNumber' => 1
+      )
+    );
+    Scheduler::scheduleWPUserWelcomeNotification(
+      $subscriber_id = 10,
+      $wp_user = (object)array('roles' => array('administrator'))
+    );
+
+    // queue is not created
+    $queue = SendingQueue::where('newsletter_id', $newsletter->id)
+      ->findOne();
+    expect($queue)->false();
+  }
+
+  function testItSchedulesWPUserWelcomeNotificationWhenUserRolesMatches() {
+    $newsletter = $this->_createNewsletter(Newsletter::TYPE_WELCOME);
+    $this->_createNewsletterOptions(
+      $newsletter->id,
+      Newsletter::TYPE_WELCOME,
+      array(
+        'event' => 'user',
+        'role' => 'administrator',
+        'afterTimeType' => 'days',
+        'afterTimeNumber' => 1
+      )
+    );
+    Scheduler::scheduleWPUserWelcomeNotification(
+      $subscriber_id = 10,
+      $wp_user = (object)array('roles' => array('administrator'))
+    );
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+
+    // queue is created and scheduled for delivery one day later
+    $queue = SendingQueue::where('newsletter_id', $newsletter->id)
+      ->findOne();
+    expect(Carbon::parse($queue->scheduled_at)->format('Y-m-d H:i'))
+      ->equals($current_time->addDay()->format('Y-m-d H:i'));
+  }
+
+  function testItSchedulesWPUserWelcomeNotificationWhenUserHasAnyRole() {
+    $newsletter = $this->_createNewsletter(Newsletter::TYPE_WELCOME);
+    $this->_createNewsletterOptions(
+      $newsletter->id,
+      Newsletter::TYPE_WELCOME,
+      array(
+        'event' => 'user',
+        'role' => Scheduler::WORDPRESS_ALL_ROLES,
+        'afterTimeType' => 'days',
+        'afterTimeNumber' => 1
+      )
+    );
+    Scheduler::scheduleWPUserWelcomeNotification(
+      $subscriber_id = 10,
+      $wp_user = (object)array('roles' => array('administrator'))
+    );
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+
+    // queue is created and scheduled for delivery one day later
+    $queue = SendingQueue::where('newsletter_id', $newsletter->id)
+      ->findOne();
+    expect(Carbon::parse($queue->scheduled_at)->format('Y-m-d H:i'))
+      ->equals($current_time->addDay()->format('Y-m-d H:i'));
+  }
+
+  function testItProcessesPostNotificationSchedule() {
+    $newsletter_option_field = NewsletterOptionField::create();
+    $newsletter_option_field->name = 'schedule';
+    $newsletter_option_field->newsletter_type = Newsletter::TYPE_WELCOME;
+    $newsletter_option_field->save();
+
+    // daily notification is scheduled at 14:00
+    $newsletter = (object)array(
+      'id' => 1,
+      'intervalType' => Scheduler::INTERVAL_DAILY,
+      'monthDay' => null,
+      'nthWeekDay' => null,
+      'weekDay' => null,
+      'timeOfDay' => 50400 // 14:00
+    );
+    Scheduler::processPostNotificationSchedule($newsletter);
+    $newsletter_option = NewsletterOption::where('newsletter_id', $newsletter->id)
+      ->where('option_field_id', $newsletter_option_field->id)
+      ->findOne();
+    expect(Scheduler::getNextRunDate($newsletter_option->value))
+      ->contains('14:00:00');
+
+    // weekly notification is scheduled every Tuesday at 14:00
+    $newsletter = (object)array(
+      'id' => 1,
+      'intervalType' => Scheduler::INTERVAL_WEEKLY,
+      'monthDay' => null,
+      'nthWeekDay' => null,
+      'weekDay' => Carbon::TUESDAY,
+      'timeOfDay' => 50400 // 14:00
+    );
+    Scheduler::processPostNotificationSchedule($newsletter);
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+    $newsletter_option = NewsletterOption::where('newsletter_id', $newsletter->id)
+      ->where('option_field_id', $newsletter_option_field->id)
+      ->findOne();
+    $next_run_date = ($current_time->dayOfWeek === Carbon::TUESDAY && $current_time->hour < 14) ?
+      $current_time :
+      $current_time->next(Carbon::TUESDAY);
+    expect(Scheduler::getNextRunDate($newsletter_option->value))
+      ->equals($next_run_date->format('Y-m-d 14:00:00'));
+
+    // monthly notification is scheduled every 20th day at 14:00
+    $newsletter = (object)array(
+      'id' => 1,
+      'intervalType' => Scheduler::INTERVAL_MONTHLY,
+      'monthDay' => 19, // 20th (count starts from 0)
+      'nthWeekDay' => null,
+      'weekDay' => null,
+      'timeOfDay' => 50400 // 14:00
+    );
+    Scheduler::processPostNotificationSchedule($newsletter);
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+    $newsletter_option = NewsletterOption::where('newsletter_id', $newsletter->id)
+      ->where('option_field_id', $newsletter_option_field->id)
+      ->findOne();
+    expect(Scheduler::getNextRunDate($newsletter_option->value))
+      ->contains('-19 14:00:00');
+
+    // monthly notification is scheduled every last Saturday at 14:00
+    $newsletter = (object)array(
+      'id' => 1,
+      'intervalType' => Scheduler::INTERVAL_NTHWEEKDAY,
+      'monthDay' => null,
+      'nthWeekDay' => 'L', // L = last
+      'weekDay' => Carbon::SATURDAY,
+      'timeOfDay' => 50400 // 14:00
+    );
+    Scheduler::processPostNotificationSchedule($newsletter);
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+    $next_run_date = (
+      $current_time->day < $current_time->lastOfMonth(Carbon::SATURDAY)->day
+    ) ? $current_time->lastOfMonth(Carbon::SATURDAY)
+      : $current_time->addMonth()->lastOfMonth(Carbon::SATURDAY);
+    $newsletter_option = NewsletterOption::where('newsletter_id', $newsletter->id)
+      ->where('option_field_id', $newsletter_option_field->id)
+      ->findOne();
+    expect(Scheduler::getNextRunDate($newsletter_option->value))
+      ->equals($next_run_date->format('Y-m-d 14:00:00'));
+
+    // notification is scheduled immediately (next minute)
+    $newsletter = (object)array(
+      'id' => 1,
+      'intervalType' => Scheduler::INTERVAL_IMMEDIATELY,
+      'monthDay' => null,
+      'nthWeekDay' => null,
+      'weekDay' => null,
+      'timeOfDay' => null
+    );
+    Scheduler::processPostNotificationSchedule($newsletter);
+    $current_time = Carbon::createFromTimestamp(current_time('timestamp'));
+    $newsletter_option = NewsletterOption::where('newsletter_id', $newsletter->id)
+      ->where('option_field_id', $newsletter_option_field->id)
+      ->findOne();
+    expect(Scheduler::getNextRunDate($newsletter_option->value))
+      ->equals($current_time->addMinute()->format('Y-m-d H:i:00'));
+  }
+
+  function _createQueue(
+    $newsletter_id,
+    $scheduled_at = null,
+    $status = SendingQueue::STATUS_SCHEDULED
+  ) {
+    $queue = SendingQueue::create();
+    $queue->status = $status;
+    $queue->newsletter_id = $newsletter_id;
+    $queue->scheduled_at = $scheduled_at;
+    $queue->save();
+    expect($queue->getErrors())->false();
+    return $queue;
+  }
+
+  function _createNewsletter(
+    $type = Newsletter::TYPE_NOTIFICATION,
+    $status = Newsletter::STATUS_ACTIVE
+  ) {
+    $newsletter = Newsletter::create();
+    $newsletter->type = $type;
+    $newsletter->status = $status;
+    $newsletter->save();
+    expect($newsletter->getErrors())->false();
+    return $newsletter;
+  }
+
+  function _createNewsletterOptions($newsletter_id, $newsletter_type, $options) {
+    foreach($options as $option => $value) {
+      $newsletter_option_field = NewsletterOptionField::create();
+      $newsletter_option_field->name = $option;
+      $newsletter_option_field->newsletter_type = $newsletter_type;
+      $newsletter_option_field->save();
+      expect($newsletter_option_field->getErrors())->false();
+
+      $newsletter_option = NewsletterOption::create();
+      $newsletter_option->option_field_id = $newsletter_option_field->id;
+      $newsletter_option->newsletter_id = $newsletter_id;
+      $newsletter_option->value = $value;
+      $newsletter_option->save();
+      expect($newsletter_option->getErrors())->false();
+    }
+  }
+
+  function _after() {
+    ORM::raw_execute('TRUNCATE ' . Newsletter::$_table);
+    ORM::raw_execute('TRUNCATE ' . NewsletterOption::$_table);
+    ORM::raw_execute('TRUNCATE ' . NewsletterOptionField::$_table);
+    ORM::raw_execute('TRUNCATE ' . NewsletterPost::$_table);
+    ORM::raw_execute('TRUNCATE ' . SendingQueue::$_table);
+  }
+}

tests/unit/Router/RouterTest.php

@@ -14,7 +14,7 @@ class FrontRouterTest extends MailPoetTest {
       Router::NAME => '',
       'endpoint' => 'mock_endpoint',
       'action' => 'test',
-      'data' => base64_encode(serialize(array('data' => 'dummy data')))
+      'data' => base64_encode(json_encode(array('data' => 'dummy data')))
     );
     $this->router = new Router($this->router_data);
   }
@@ -22,7 +22,7 @@ class FrontRouterTest extends MailPoetTest {
   function testItCanGetAPIDataFromGetRequest() {
     $data = array('data' => 'dummy data');
     $url = 'http://example.com/?' . Router::NAME . '&endpoint=view_in_browser&action=view&data='
-      . base64_encode(serialize($data));
+      . base64_encode(json_encode($data));
     parse_str(parse_url($url, PHP_URL_QUERY), $_GET);
     $router = new Router();
     expect($router->api_request)->equals(true);
@@ -100,7 +100,7 @@ class FrontRouterTest extends MailPoetTest {
     $data = array('data' => 'dummy data');
     $result = Router::encodeRequestData($data);
     expect($result)->equals(
-      rtrim(base64_encode(serialize($data)), '=')
+      rtrim(base64_encode(json_encode($data)), '=')
     );
   }
 
@@ -112,14 +112,19 @@ class FrontRouterTest extends MailPoetTest {
 
   function testItCanDecodeRequestData() {
     $data = array('data' => 'dummy data');
-    $encoded_data = rtrim(base64_encode(serialize($data)), '=');
+    $encoded_data = rtrim(base64_encode(json_encode($data)), '=');
     $result = Router::decodeRequestData($encoded_data);
     expect($result)->equals($data);
   }
 
+  function testItCanConvertInvalidRequestDataToArray() {
+    $result = Router::decodeRequestData('some_invalid_data');
+    expect($result)->equals(array());
+  }
+
   function testItCanBuildRequest() {
     $data = array('data' => 'dummy data');
-    $encoded_data = rtrim(base64_encode(serialize($data)), '=');
+    $encoded_data = rtrim(base64_encode(json_encode($data)), '=');
     $result = Router::buildRequest(
       'mock_endpoint',
       'test',

tests/unit/Subscribers/ImportExport/Import/ImportTest.php

@@ -66,6 +66,31 @@ class ImportTest extends MailPoetTest {
     expect($this->import->updated_at)->notEmpty();
   }
 
+  function testItChecksForRequiredDataFields() {
+    $data = $this->data;
+    // exception should be thrown when one or more fields do not exist
+    unset($data['timestamp']);
+    try {
+      $this->import->validateData($data);
+      self::fail('Missing or invalid data exception not thrown.');
+    } catch(Exception $e) {
+      expect($e->getMessage())->equals('Missing or invalid subscriber data.');
+    }
+    // exception should not be thrown when all fields exist
+    $this->import->validateData($this->data);
+  }
+
+  function testItValidatesColumnNames() {
+    $data = $this->data;
+    $data['columns']['test) values ((ExtractValue(1,CONCAT(0x5c, (SELECT version())))))%23'] = true;
+    try {
+      $this->import->validateData($data);
+      self::fail('Missing or invalid data exception not thrown.');
+    } catch(Exception $e) {
+      expect($e->getMessage())->equals('Missing or invalid subscriber data.');
+    }
+  }
+
   function testItCanTransformSubscribers() {
     $custom_field = $this->subscriber_custom_fields[0];
     expect($this->import->subscribers_data['first_name'][0])

tests/unit/Subscribers/ImportExport/Import/MailChimpTest.php

@@ -9,11 +9,22 @@ class MailChimpTest extends MailPoetTest {
     $this->lists = explode(",", getenv('WP_TEST_IMPORT_MAILCHIMP_LISTS'));
   }
 
-  function testItValidatesAPIKey() {
+  function testItCanGetAPIKey() {
     $valid_api_key_format = '12345678901234567890123456789012-ab1';
+    // key must consist of two parts separated by hyphen
+    expect($this->mailchimp->getAPIKey('invalid_api_key_format'))->false();
+    // key must only contain numerals and letters
+    expect($this->mailchimp->getAPIKey('12345678901234567890123456789012-@?1'))->false();
+    // the first part of the key must contain 32 characters,
+    expect($this->mailchimp->getAPIKey('1234567890123456789012345678901-123'))
+      ->false();
+    // the second part must contain 2 or 3 characters
+    expect($this->mailchimp->getAPIKey('12345678901234567890123456789012-1234'))
+      ->false();
+    expect($this->mailchimp->getAPIKey('12345678901234567890123456789012-1'))
+      ->false();
     expect($this->mailchimp->getAPIKey($valid_api_key_format))
       ->equals($valid_api_key_format);
-    expect($this->mailchimp->getAPIKey('invalid_api_key_format'))->false();
   }
 
   function testItCanGetDatacenter() {

tests/unit/Subscription/UrlTest.php

@@ -1,4 +1,5 @@
 <?php
+use MailPoet\Router\Router;
 use \MailPoet\Subscription\Url;
 use \MailPoet\Models\Subscriber;
 use \MailPoet\Models\Setting;
@@ -68,7 +69,7 @@ class UrlTest extends MailPoetTest {
     // extract & decode data from url
     $url_params = parse_url($url);
     parse_str($url_params['query'], $params);
-    $data = unserialize(base64_decode($params['data']));
+    $data = Router::decodeRequestData($params['data']);
 
     expect($data['email'])->contains('john@mailpoet.com');
     expect($data['token'])->notEmpty();

views/subscribers/importExport/import/step2.html

@@ -116,11 +116,11 @@
       {{#subscribers}}
       <tr>
         <td>
-          {{show_real_index @index}}
+          {{calculate_index @index}}
         </td>
         {{#.}}
         <td>
-          {{{this}}}
+          {{sanitize_data this}}
         </td>
         {{/.}}
       </tr>