From 7f566fb672c7ceda32c1ab74b5b54edb9d51724c Mon Sep 17 00:00:00 2001 From: Vlad Date: Sun, 4 Jun 2017 17:38:28 -0400 Subject: [PATCH] Adds client-side check for invalid characters in email addresses Adds server-side validation of email addresses using WP's is_email() --- .../js/src/subscribers/importExport/import.js | 4 ++ .../ImportExport/Import/Import.php | 29 +++++++++-- .../ImportExport/Import/ImportTest.php | 50 ++++++++++++++++++- 3 files changed, 77 insertions(+), 6 deletions(-) diff --git a/assets/js/src/subscribers/importExport/import.js b/assets/js/src/subscribers/importExport/import.js index 322a1a2bf4..e59a719a79 100644 --- a/assets/js/src/subscribers/importExport/import.js +++ b/assets/js/src/subscribers/importExport/import.js @@ -323,6 +323,10 @@ define( // is the email in 'mailto:email' format? email = test[1].trim(); } + // test for valid characters using WP's rule (https://core.trac.wordpress.org/browser/tags/4.7.3/src/wp-includes/formatting.php#L2902) + if (!/^[a-zA-Z0-9!#$%&\'*+\/=?^_`{|}~\.-@]+$/.test(email) ) { + return false; + } return email; }; diff --git a/lib/Subscribers/ImportExport/Import/Import.php b/lib/Subscribers/ImportExport/Import/Import.php index beb2837d33..71dfb7ea18 100644 --- a/lib/Subscribers/ImportExport/Import/Import.php +++ b/lib/Subscribers/ImportExport/Import/Import.php @@ -16,10 +16,14 @@ class Import { public $update_subscribers; public $subscribers_fields; public $subscribers_custom_fields; + public $subscribers_fields_validation_rules; public $subscribers_count; public $created_at; public $updated_at; public $required_subscribers_fields; + private $default_subscribers_data_validation_rules = array( + 'email' => 'email' + ); const DB_QUERY_CHUNK_SIZE = 100; public function __construct($data) { @@ -36,9 +40,10 @@ class Import { $this->subscribers_custom_fields = $this->getCustomSubscribersFields( array_keys($data['columns']) ); - $this->subscribers_fields_validation_rules = $this->getSubscriberDataValidationRules( - $data['columns'] + $this->default_subscribers_fields_validation_rules = array( + 'email' => 'email' ); + $this->subscribers_fields_validation_rules = $this->getSubscriberDataValidationRules($data['columns']); $this->subscribers_count = count(reset($this->subscribers_data)); $this->created_at = date('Y-m-d H:i:s', (int)$data['timestamp']); $this->updated_at = date('Y-m-d H:i:s', (int)$data['timestamp'] + 1); @@ -74,7 +79,7 @@ class Import { $field['validation_rule'] : false; } - return $validation_rules; + return array_replace($validation_rules, $this->default_subscribers_data_validation_rules); } function process() { @@ -83,7 +88,9 @@ class Import { $this->subscribers_data, $this->subscribers_fields_validation_rules ); - + if(!$subscribers_data) { + throw new \Exception(__('No valid subscribers were founds.', 'mailpoet')); + } // permanently trash deleted subscribers $this->deleteExistingTrashedSubscribers($subscribers_data); @@ -149,6 +156,16 @@ class Import { $invalid_records = array(); foreach($subscribers_data as $column => &$data) { $validation_rule = $validation_rules[$column]; + if($validation_rule === 'email') { + $data = array_map( + function($index, $email) use(&$invalid_records) { + if(!is_email($email)) { + $invalid_records[] = $index; + } + return $email; + }, array_keys($data), $data + ); + } // if this is a custom column if(in_array($column, $this->subscribers_custom_fields)) { $custom_field = CustomField::findOne($column); @@ -162,7 +179,8 @@ class Import { $invalid_records[] = $index; } return $date; - }, array_keys($data), $data); + }, array_keys($data), $data + ); } } } @@ -172,6 +190,7 @@ class Import { $data = array_values($data); } } + if(empty($subscribers_data['email'])) return false; return $subscribers_data; } diff --git a/tests/unit/Subscribers/ImportExport/Import/ImportTest.php b/tests/unit/Subscribers/ImportExport/Import/ImportTest.php index 8d46dec362..62b6861fd6 100644 --- a/tests/unit/Subscribers/ImportExport/Import/ImportTest.php +++ b/tests/unit/Subscribers/ImportExport/Import/ImportTest.php @@ -91,6 +91,55 @@ class ImportTest extends MailPoetTest { } } + function testItValidatesSubscribersEmail() { + $validation_rules = array('email' => 'email'); + + // invalid email is removed from data object + $data['email'] = array( + 'àdam@smîth.com', + 'jane@doe.com' + ); + $result = $this->import->validateSubscribersData($data, $validation_rules); + expect($result['email'])->count(1); + expect($result['email'][0])->equals('jane@doe.com'); + + // valid email passes validation + $data['email'] = array( + 'adam@smith.com', + 'jane@doe.com' + ); + $result = $this->import->validateSubscribersData($data, $validation_rules); + expect($result)->equals($data); + } + + function testItThrowsErrorWhenNoValidSubscribersAreFoundDuringImport() { + $data = array( + 'subscribers' => array( + array( + 'Adam', + 'Smith', + 'àdam@smîth.com', + 'France' + ) + ), + 'columns' => array( + 'first_name' => array('index' => 0), + 'last_name' => array('index' => 1), + 'email' => array('index' => 2) + ), + 'segments' => array(), + 'timestamp' => time(), + 'updateSubscribers' => true + ); + $import = new Import($data); + try { + $import->process(); + self::fail('No valid subscribers found exception not thrown.'); + } catch(Exception $e) { + expect($e->getMessage())->equals('No valid subscribers were founds.'); + } + } + function testItTransformsSubscribers() { $custom_field = $this->subscribers_custom_fields[0]; expect($this->import->subscribers_data['first_name'][0]) @@ -439,7 +488,6 @@ class ImportTest extends MailPoetTest { expect($updated_subscriber->status)->equals('unsubscribed'); } - function testItRunsImport() { $result = $this->import->process(); expect($result['created'])->equals(2);