From 6e7c7ae8c8a4f43da8149a95fe46afd91fb78b5d Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Fri, 23 Feb 2018 22:23:05 +0100 Subject: [PATCH 1/7] modelValidator: Add more strict Sudzy email validation next to is_email check MAILPOET-1288 --- lib/Models/ModelValidator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Models/ModelValidator.php b/lib/Models/ModelValidator.php index 389dd2d7a6..2791c1ea0b 100644 --- a/lib/Models/ModelValidator.php +++ b/lib/Models/ModelValidator.php @@ -32,7 +32,7 @@ class ModelValidator extends \Sudzy\Engine { function validateEmail($email) { $permitted_length = (strlen($email) >= self::EMAIL_MIN_LENGTH && strlen($email) <= self::EMAIL_MAX_LENGTH); - $valid_email = (is_email($email) !== false); + $valid_email = is_email($email) !== false && parent::_isEmail($email, null); return ($permitted_length && $valid_email); } From afed408297e2405c3ca3292175343af37003bba3 Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Sat, 24 Feb 2018 19:35:29 +0100 Subject: [PATCH 2/7] Synchronize segments removes updated/inserted subscribers with invalid emails MAILPOET-1288 --- lib/Segments/WP.php | 40 ++++++++++++++++++++++++++++++---- tests/unit/Segments/WPTest.php | 14 +++++++++--- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/lib/Segments/WP.php b/lib/Segments/WP.php index d793258a9b..0f20a2f3c3 100644 --- a/lib/Segments/WP.php +++ b/lib/Segments/WP.php @@ -1,6 +1,7 @@ validateEmail($updated_email['email']); + })); + if(!$invalid_wp_user_ids) { + return; + } + \ORM::for_table(Subscriber::$_table)->whereIn('wp_user_id', $invalid_wp_user_ids)->delete_many(); + } + private static function updateSubscribersEmails() { global $wpdb; $subscribers_table = Subscriber::$_table; + $changed_email_user_ids = \ORM::for_table(Subscriber::$_table)->raw_query(sprintf( + 'SELECT %1$s.wp_user_id as id, wu.user_email as email FROM %1$s + INNER JOIN %2$s AS wu ON %1$s.wp_user_id = wu.id + WHERE wu.user_email != %1$s.email + ', $subscribers_table, $wpdb->users))->findArray(); + Subscriber::raw_execute(sprintf(' UPDATE IGNORE %1$s - JOIN %2$s as wu ON %1$s.wp_user_id = wu.id + INNER JOIN %2$s as wu ON %1$s.wp_user_id = wu.id SET %1$s.email = wu.user_email - WHERE %1$s.wp_user_id IS NOT NULL AND wu.user_email != "" + WHERE wu.user_email != %1$s.email AND wu.user_email != "" ', $subscribers_table, $wpdb->users)); + return $changed_email_user_ids; } private static function insertSubscribers() { global $wpdb; $subscribers_table = Subscriber::$_table; + + $inserterd_user_ids = \ORM::for_table($wpdb->users)->raw_query(sprintf( + 'SELECT %2$s.id, %2$s.user_email as email FROM %2$s + LEFT JOIN %1$s AS mps ON mps.wp_user_id = %2$s.id + WHERE mps.wp_user_id IS NULL AND %2$s.user_email != "" + ', $subscribers_table, $wpdb->users))->findArray(); + Subscriber::raw_execute(sprintf(' INSERT IGNORE INTO %1$s(wp_user_id, email, status, created_at) SELECT wu.id, wu.user_email, "subscribed", CURRENT_TIMESTAMP() FROM %2$s wu @@ -113,6 +143,8 @@ class WP { WHERE mps.wp_user_id IS NULL AND wu.user_email != "" ON DUPLICATE KEY UPDATE wp_user_id = wu.id ', $subscribers_table, $wpdb->users)); + + return $inserterd_user_ids; } private static function updateFirstNames() { diff --git a/tests/unit/Segments/WPTest.php b/tests/unit/Segments/WPTest.php index b61b13dfeb..eb26f2701c 100644 --- a/tests/unit/Segments/WPTest.php +++ b/tests/unit/Segments/WPTest.php @@ -54,13 +54,21 @@ class WPTest extends \MailPoetTest { expect($subscriber->email)->equals('user-sync-test-xx@email.com'); } - function testItDoesNotSynchronizeEmptyEmailsForExistingUsers() { + function testRemovesUsersWithEmptyEmailsFromSunscribersDuringSynchronization() { $id = $this->insertUser(); WP::synchronizeUsers(); $this->updateWPUserEmail($id, ''); WP::synchronizeUsers(); - $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); - expect($subscriber->email)->notEmpty(); + expect(Subscriber::where('wp_user_id', $id)->count())->equals(0); + $this->deleteWPUser($id); + } + + function testRemovesUsersWithInvalidEmailsFromSunscribersDuringSynchronization() { + $id = $this->insertUser(); + WP::synchronizeUsers(); + $this->updateWPUserEmail($id, 'ivalid.@email.com'); + WP::synchronizeUsers(); + expect(Subscriber::where('wp_user_id', $id)->count())->equals(0); $this->deleteWPUser($id); } From 5c10a664441831ec8fd01319f08c19244e44d7eb Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Sun, 25 Feb 2018 11:01:17 +0100 Subject: [PATCH 3/7] Subscribers import: Email validation unified with subscribers sync MAILPOET-1288 --- lib/Subscribers/ImportExport/Import/Import.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Subscribers/ImportExport/Import/Import.php b/lib/Subscribers/ImportExport/Import/Import.php index 4451b44e1f..ff24d1aaf2 100644 --- a/lib/Subscribers/ImportExport/Import/Import.php +++ b/lib/Subscribers/ImportExport/Import/Import.php @@ -3,6 +3,7 @@ namespace MailPoet\Subscribers\ImportExport\Import; use MailPoet\Form\Block\Date; use MailPoet\Models\CustomField; +use MailPoet\Models\ModelValidator; use MailPoet\Models\Newsletter; use MailPoet\Models\Subscriber; use MailPoet\Models\SubscriberCustomField; @@ -148,12 +149,13 @@ class Import { function validateSubscribersData($subscribers_data, $validation_rules) { $invalid_records = array(); + $validator = new ModelValidator(); 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)) { + function($index, $email) use(&$invalid_records, $validator) { + if(!$validator->validateEmail($email)) { $invalid_records[] = $index; } return strtolower($email); From c164066522a26811f31f5fdc624704d27b1c0e5a Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Thu, 1 Mar 2018 09:16:33 +0100 Subject: [PATCH 4/7] Check valid filled email addresses in plugin settings MAILPOET-1288 --- views/settings.html | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/views/settings.html b/views/settings.html index 3e2c681c69..b8ec62b4fc 100644 --- a/views/settings.html +++ b/views/settings.html @@ -59,11 +59,24 @@ diff --git a/views/settings.html b/views/settings.html index b8ec62b4fc..5bf90ddc4c 100644 --- a/views/settings.html +++ b/views/settings.html @@ -59,8 +59,6 @@ <% endblock %> From 8bc0ad48c039081bb6efc60b997b22b7d9dde43d Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Thu, 1 Mar 2018 10:13:52 +0100 Subject: [PATCH 6/7] Email valiation added to reply-to on newsletter send screen --- assets/js/src/newsletters/send/notification.jsx | 3 +++ assets/js/src/newsletters/send/standard.jsx | 3 +++ assets/js/src/newsletters/send/welcome.jsx | 3 +++ 3 files changed, 9 insertions(+) diff --git a/assets/js/src/newsletters/send/notification.jsx b/assets/js/src/newsletters/send/notification.jsx index 3a04d8ef44..79855b4453 100644 --- a/assets/js/src/newsletters/send/notification.jsx +++ b/assets/js/src/newsletters/send/notification.jsx @@ -92,6 +92,9 @@ define( name: 'reply_to_address', type: 'text', placeholder: MailPoet.I18n.t('replyToAddressPlaceholder'), + validation: { + 'data-parsley-type': 'email', + }, }, ], }, diff --git a/assets/js/src/newsletters/send/standard.jsx b/assets/js/src/newsletters/send/standard.jsx index e61dd9a839..84e5d008de 100644 --- a/assets/js/src/newsletters/send/standard.jsx +++ b/assets/js/src/newsletters/send/standard.jsx @@ -398,6 +398,9 @@ define( name: 'reply_to_address', type: 'text', placeholder: MailPoet.I18n.t('replyToAddressPlaceholder'), + validation: { + 'data-parsley-type': 'email', + }, }, ], }, diff --git a/assets/js/src/newsletters/send/welcome.jsx b/assets/js/src/newsletters/send/welcome.jsx index ab37fbfb49..e92f43f4d5 100644 --- a/assets/js/src/newsletters/send/welcome.jsx +++ b/assets/js/src/newsletters/send/welcome.jsx @@ -65,6 +65,9 @@ define( name: 'reply_to_address', type: 'text', placeholder: MailPoet.I18n.t('replyToAddressPlaceholder'), + validation: { + 'data-parsley-type': 'email', + }, }, ], }, From aceb9bb03183ff9c8b5868191ca28cef72c5740f Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Mon, 5 Mar 2018 07:20:04 +0100 Subject: [PATCH 7/7] Hardened UI email address check MAILPOET-1288 --- views/layout.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/views/layout.html b/views/layout.html index f95db7b5d6..d29b584ff1 100644 --- a/views/layout.html +++ b/views/layout.html @@ -48,8 +48,8 @@ jQuery('.toplevel_page_mailpoet-newsletters.menu-top-last') var mailpoet_premium_version = <%= json_encode(mailpoet_premium_version()) %>; var mailpoet_analytics_enabled = <%= is_analytics_enabled() | json_encode %>; var mailpoet_analytics_data = <%= json_encode(get_analytics_data()) %>; - // RFC 5322 standard; http://emailregex.com/ - var mailpoet_email_regex = /^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/; + // RFC 5322 standard; http://emailregex.com/ combined with https://google.github.io/closure-library/api/goog.format.EmailAddress.html#isValid + var mailpoet_email_regex = /(?=^[+a-zA-Z0-9_.!#$%&'*\/=?^`{|}~-]+@([a-zA-Z0-9-]+\.)+[a-zA-Z0-9]{2,63}$)(?=^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,})))/;