From 7788aebe83a5078111e5c069f519a9f85d3a66a0 Mon Sep 17 00:00:00 2001 From: wxa Date: Thu, 11 Jul 2019 13:42:47 +0300 Subject: [PATCH] Fix minor PR remarks [MAILPOET-2015] Reuse updateCaptcha() function Inject Captcha class using DI Add no-cache headers to captcha image Fix an error when accessing the captcha page directly Edit the line in settings regarding missing dependencies --- assets/js/src/public.js | 20 ++++++++++--------- lib/API/JSON/v1/Setup.php | 6 ++++-- lib/AdminPages/Pages/Settings.php | 9 +++++++-- lib/Config/MP2Migrator.php | 4 +++- lib/Config/Populator.php | 12 ++++++++--- lib/Subscription/Captcha.php | 8 ++++++++ lib/Subscription/Pages.php | 3 +++ .../Workers/SendingQueue/SendingQueueTest.php | 3 ++- .../Workers/SendingQueue/Tasks/MailerTest.php | 3 ++- .../integration/Newsletter/ShortcodesTest.php | 3 ++- tests/integration/Subscription/UrlTest.php | 3 ++- views/settings/advanced.html | 2 +- 12 files changed, 54 insertions(+), 22 deletions(-) diff --git a/assets/js/src/public.js b/assets/js/src/public.js index 7353e480c3..372b0ea86f 100644 --- a/assets/js/src/public.js +++ b/assets/js/src/public.js @@ -22,6 +22,15 @@ jQuery(function ($) { // eslint-disable-line func-names return (window.location.hostname === link.hostname); } + function updateCaptcha(e) { + var captcha = $('img.mailpoet_captcha'); + var captchaSrc = captcha.attr('src'); + var hashPos = captchaSrc.indexOf('#'); + var newSrc = hashPos > 0 ? captchaSrc.substring(0, hashPos) : captchaSrc; + captcha.attr('src', newSrc + '#' + new Date().getTime()); + if (e) e.preventDefault(); + } + $(function () { // eslint-disable-line func-names // setup form validation $('form.mailpoet_form').each(function () { // eslint-disable-line func-names @@ -68,7 +77,7 @@ jQuery(function ($) { // eslint-disable-line func-names window.top.location.href = response.meta.redirect_url; } else { if (response.meta && response.meta.refresh_captcha) { - $('.mailpoet_captcha_update').click(); + updateCaptcha(); } form.find('.mailpoet_validate_error').html( response.errors.map(function buildErrorMessage(error) { @@ -125,13 +134,6 @@ jQuery(function ($) { // eslint-disable-line func-names }); }); - $('.mailpoet_captcha_update').click(function updateCaptcha(e) { - var captcha = $('img.mailpoet_captcha'); - var captchaSrc = captcha.attr('src'); - var hashPos = captchaSrc.indexOf('#'); - var newSrc = hashPos > 0 ? captchaSrc.substring(0, hashPos) : captchaSrc; - captcha.attr('src', newSrc + '#' + new Date().getTime()); - e.preventDefault(); - }); + $('.mailpoet_captcha_update').on('click', updateCaptcha); }); }); diff --git a/lib/API/JSON/v1/Setup.php b/lib/API/JSON/v1/Setup.php index 96b1eeffbc..3930574a56 100644 --- a/lib/API/JSON/v1/Setup.php +++ b/lib/API/JSON/v1/Setup.php @@ -6,8 +6,9 @@ use MailPoet\API\JSON\Endpoint as APIEndpoint; use MailPoet\WP\Functions as WPFunctions; use MailPoet\Config\AccessControl; use MailPoet\Config\Activator; -use MailPoet\Settings\SettingsController; use MailPoet\Config\Populator; +use MailPoet\Settings\SettingsController; +use MailPoet\Subscription\Captcha; if (!defined('ABSPATH')) exit; @@ -24,7 +25,8 @@ class Setup extends APIEndpoint { function reset() { try { $settings = new SettingsController(); - $activator = new Activator($settings, new Populator($settings, $this->wp)); + $captcha = new Captcha(); + $activator = new Activator($settings, new Populator($settings, $this->wp, $captcha)); $activator->deactivate(); $activator->activate(); $this->wp->doAction('mailpoet_setup_reset'); diff --git a/lib/AdminPages/Pages/Settings.php b/lib/AdminPages/Pages/Settings.php index ede2900cc2..ac880d2b05 100644 --- a/lib/AdminPages/Pages/Settings.php +++ b/lib/AdminPages/Pages/Settings.php @@ -37,6 +37,9 @@ class Settings { /** @var ServicesChecker */ private $services_checker; + /** @var Captcha */ + private $captcha; + /** @var FeaturesController */ private $features_controller; @@ -50,7 +53,8 @@ class Settings { WPFunctions $wp, ServicesChecker $services_checker, FeaturesController $features_controller, - Installation $installation + Installation $installation, + Captcha $captcha ) { $this->page_renderer = $page_renderer; $this->settings = $settings; @@ -59,6 +63,7 @@ class Settings { $this->services_checker = $services_checker; $this->features_controller = $features_controller; $this->installation = $installation; + $this->captcha = $captcha; } function render() { @@ -90,7 +95,7 @@ class Settings { 'web' => Hosts::getWebHosts(), 'smtp' => Hosts::getSMTPHosts(), ], - 'built_in_captcha_supported' => (new Captcha)->isSupported(), + 'built_in_captcha_supported' => $this->captcha->isSupported(), ]; $data['is_new_user'] = $this->installation->isNewInstallation(); diff --git a/lib/Config/MP2Migrator.php b/lib/Config/MP2Migrator.php index 9ea5a0ed45..657d181a8d 100644 --- a/lib/Config/MP2Migrator.php +++ b/lib/Config/MP2Migrator.php @@ -10,6 +10,7 @@ use MailPoet\Models\Subscriber; use MailPoet\Models\SubscriberCustomField; use MailPoet\Models\SubscriberSegment; use MailPoet\Settings\SettingsController; +use MailPoet\Subscription\Captcha; use MailPoet\Util\Notices\AfterMigrationNotice; use MailPoet\Util\ProgressBar; use MailPoet\WP\Functions as WPFunctions; @@ -217,7 +218,8 @@ class MP2Migrator { */ private function eraseMP3Data() { $settings = new SettingsController(); - $activator = new Activator($settings, new Populator($settings, WPFunctions::get())); + $captcha = new Captcha(); + $activator = new Activator($settings, new Populator($settings, WPFunctions::get(), $captcha)); $activator->deactivate(); $activator->activate(); diff --git a/lib/Config/Populator.php b/lib/Config/Populator.php index 8391a41a34..fc363e63ea 100644 --- a/lib/Config/Populator.php +++ b/lib/Config/Populator.php @@ -38,11 +38,18 @@ class Populator { private $settings; /** @var WPFunctions */ private $wp; + /** @var Captcha */ + private $captcha; const TEMPLATES_NAMESPACE = '\MailPoet\Config\PopulatorData\Templates\\'; - function __construct(SettingsController $settings, WPFunctions $wp) { + function __construct( + SettingsController $settings, + WPFunctions $wp, + Captcha $captcha + ) { $this->settings = $settings; $this->wp = $wp; + $this->captcha = $captcha; $this->prefix = Env::$db_prefix; $this->models = [ 'newsletter_option_fields', @@ -226,11 +233,10 @@ class Populator { $captcha = $this->settings->fetch('captcha'); $re_captcha = $this->settings->fetch('re_captcha'); if (empty($captcha)) { - $subscription_captcha = new Captcha; $captcha_type = Captcha::TYPE_DISABLED; if (!empty($re_captcha['enabled'])) { $captcha_type = Captcha::TYPE_RECAPTCHA; - } elseif ($subscription_captcha->isSupported()) { + } elseif ($this->captcha->isSupported()) { $captcha_type = Captcha::TYPE_BUILTIN; } $this->settings->set('captcha', [ diff --git a/lib/Subscription/Captcha.php b/lib/Subscription/Captcha.php index c0fb1ec0ef..c217a477b5 100644 --- a/lib/Subscription/Captcha.php +++ b/lib/Subscription/Captcha.php @@ -90,6 +90,14 @@ class Captcha { return $builder->get(); } + header("Expires: Sat, 01 Jan 2019 01:00:00 GMT"); // time in the past + header("Last-Modified: " . gmdate("D, d M Y H:i:s") . " GMT"); + header("Cache-Control: no-store, no-cache, must-revalidate"); + header("Cache-Control: post-check=0, pre-check=0", false); + header("Pragma: no-cache"); + header('X-Cache-Enabled: False'); + header('X-LiteSpeed-Cache-Control: no-cache'); + header('Content-Type: image/jpeg'); $builder->output(); exit; diff --git a/lib/Subscription/Pages.php b/lib/Subscription/Pages.php index 15322c3b01..c16ba35bf2 100644 --- a/lib/Subscription/Pages.php +++ b/lib/Subscription/Pages.php @@ -292,6 +292,9 @@ class Pages { $form_id = isset($_SESSION[Captcha::SESSION_FORM_KEY]['form_id']) ? (int)$_SESSION[Captcha::SESSION_FORM_KEY]['form_id'] : 0; $form_model = FormModel::findOne($form_id); + if (!$form_model instanceof FormModel) { + return false; + } $form_model = $form_model->asArray(); $form_html = '
ID); $this->settings = new SettingsController(); - $populator = new Populator($this->settings, WPFunctions::get()); + $populator = new Populator($this->settings, WPFunctions::get(), new Captcha); $populator->up(); $this->subscriber = Subscriber::create(); $this->subscriber->email = 'john@doe.com'; diff --git a/tests/integration/Cron/Workers/SendingQueue/Tasks/MailerTest.php b/tests/integration/Cron/Workers/SendingQueue/Tasks/MailerTest.php index f6f33959f4..054bb0d4e4 100644 --- a/tests/integration/Cron/Workers/SendingQueue/Tasks/MailerTest.php +++ b/tests/integration/Cron/Workers/SendingQueue/Tasks/MailerTest.php @@ -9,6 +9,7 @@ use MailPoet\Mailer\Mailer; use MailPoet\Models\Setting; use MailPoet\Models\Subscriber; use MailPoet\Settings\SettingsController; +use MailPoet\Subscription\Captcha; use MailPoet\WP\Functions as WPFunctions; if (!defined('ABSPATH')) exit; @@ -25,7 +26,7 @@ class MailerTest extends \MailPoetTest { $wp_users = get_users(); wp_set_current_user($wp_users[0]->ID); $this->settings = new SettingsController(); - $populator = new Populator($this->settings, WPFunctions::get()); + $populator = new Populator($this->settings, WPFunctions::get(), new Captcha); $populator->up(); $this->mailer_task = new MailerTask(); $this->sender = $this->settings->get('sender'); diff --git a/tests/integration/Newsletter/ShortcodesTest.php b/tests/integration/Newsletter/ShortcodesTest.php index 14736c4d8f..2874b17fde 100644 --- a/tests/integration/Newsletter/ShortcodesTest.php +++ b/tests/integration/Newsletter/ShortcodesTest.php @@ -10,6 +10,7 @@ use MailPoet\Models\SubscriberCustomField; use MailPoet\Newsletter\Shortcodes\Categories\Date; use MailPoet\Newsletter\Url as NewsletterUrl; use MailPoet\Settings\SettingsController; +use MailPoet\Subscription\Captcha; use MailPoet\Subscription\Url as SubscriptionUrl; use MailPoet\WP\Functions as WPFunctions; @@ -25,7 +26,7 @@ class ShortcodesTest extends \MailPoetTest { function _before() { parent::_before(); $this->settings = new SettingsController(); - $populator = new Populator($this->settings, WPFunctions::get()); + $populator = new Populator($this->settings, WPFunctions::get(), new Captcha); $populator->up(); $this->WP_user = $this->_createWPUser(); $this->WP_post = $this->_createWPPost(); diff --git a/tests/integration/Subscription/UrlTest.php b/tests/integration/Subscription/UrlTest.php index e9e16ca889..5a9cd9e390 100644 --- a/tests/integration/Subscription/UrlTest.php +++ b/tests/integration/Subscription/UrlTest.php @@ -7,12 +7,13 @@ use MailPoet\Models\Subscriber; use MailPoet\Models\Setting; use MailPoet\Config\Populator; use MailPoet\Settings\SettingsController; +use MailPoet\Subscription\Captcha; use MailPoet\WP\Functions as WPFunctions; class UrlTest extends \MailPoetTest { function _before() { parent::_before(); - $populator = new Populator(new SettingsController, WPFunctions::get()); + $populator = new Populator(new SettingsController, WPFunctions::get(), new Captcha); $populator->up(); } diff --git a/views/settings/advanced.html b/views/settings/advanced.html index f3e0579aab..927490511a 100644 --- a/views/settings/advanced.html +++ b/views/settings/advanced.html @@ -297,7 +297,7 @@ <% endif %> /><%= __('Built-in captcha (default)') %> <% if(not(built_in_captcha_supported)) %> - <%= __('(disabled because GD and FreeType extensions are missing)') %> + <%= __('(disabled because GD or FreeType extension is missing)') %> <% endif %>