.../EventSubscriber/FinishResponseSubscriber.php | 4 +- .../contact/src/Access/ContactPageAccess.php | 2 +- .../contact/src/Controller/ContactController.php | 67 +--------------------- core/modules/contact/src/MessageForm.php | 38 +++++++++++- .../contact/src/Tests/ContactPersonalTest.php | 4 +- .../contact/src/Tests/ContactSitewideTest.php | 3 +- core/modules/simpletest/src/WebTestBase.php | 13 ++++- 7 files changed, 57 insertions(+), 74 deletions(-) diff --git a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php index 0e508a2..863cf29 100644 --- a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php @@ -167,7 +167,7 @@ public function onRespond(FilterResponseEvent $event) { protected function updateDrupalCacheHeaders(Response $response, CacheableInterface $cacheable_access_result) { // X-Drupal-Cache-Tags $cache_tags = $cacheable_access_result->getCacheTags(); - if ($response->headers->has('X-Drupal-Cache-Tags')) { + if (strlen($response->headers->get('X-Drupal-Cache-Tags')) > 0) { $existing_cache_tags = explode(' ', $response->headers->get('X-Drupal-Cache-Tags')); $cache_tags = Cache::mergeTags($existing_cache_tags, $cache_tags); } @@ -175,7 +175,7 @@ protected function updateDrupalCacheHeaders(Response $response, CacheableInterfa // X-Drupal-Cache-Contexts $cache_contexts = $cacheable_access_result->getCacheContexts(); - if ($response->headers->has('X-Drupal-Cache-Contexts')) { + if (strlen($response->headers->get('X-Drupal-Cache-Contexts')) > 0) { $existing_cache_contexts = explode(' ', $response->headers->get('X-Drupal-Cache-Contexts')); $cache_contexts = Cache::mergeContexts($existing_cache_contexts, $cache_contexts); } diff --git a/core/modules/contact/src/Access/ContactPageAccess.php b/core/modules/contact/src/Access/ContactPageAccess.php index 507404a..0d6ffcb 100644 --- a/core/modules/contact/src/Access/ContactPageAccess.php +++ b/core/modules/contact/src/Access/ContactPageAccess.php @@ -62,7 +62,7 @@ public function access(UserInterface $user, AccountInterface $account) { // Anonymous users cannot have contact forms. if ($contact_account->isAnonymous()) { - return AccessResult::forbidden(); + return AccessResult::forbidden()->cachePerPermissions(); } // Users may not contact themselves. diff --git a/core/modules/contact/src/Controller/ContactController.php b/core/modules/contact/src/Controller/ContactController.php index f3d6825..4cc1815 100644 --- a/core/modules/contact/src/Controller/ContactController.php +++ b/core/modules/contact/src/Controller/ContactController.php @@ -9,8 +9,6 @@ use Drupal\Core\Cache\Cache; use Drupal\Core\Controller\ControllerBase; -use Drupal\Core\Datetime\DateFormatter; -use Drupal\Core\Flood\FloodInterface; use Drupal\contact\ContactFormInterface; use Drupal\user\UserInterface; use Drupal\Component\Utility\SafeMarkup; @@ -24,43 +22,6 @@ class ContactController extends ControllerBase { /** - * The flood service. - * - * @var \Drupal\Core\Flood\FloodInterface - */ - protected $flood; - - /** - * The date formatter service. - * - * @var \Drupal\Core\Datetime\DateFormatter - */ - protected $dateFormatter; - - /** - * Constructs a ContactController object. - * - * @param \Drupal\Core\Flood\FloodInterface $flood - * The flood service. - * @param \Drupal\Core\Datetime\DateFormatter $date_formatter - * The date service. - */ - public function __construct(FloodInterface $flood, DateFormatter $date_formatter) { - $this->flood = $flood; - $this->dateFormatter = $date_formatter; - } - - /** - * {@inheritdoc} - */ - public static function create(ContainerInterface $container) { - return new static( - $container->get('flood'), - $container->get('date.formatter') - ); - } - - /** * Presents the site-wide contact form. * * @param \Drupal\contact\ContactFormInterface $contact_form @@ -74,10 +35,6 @@ public static function create(ContainerInterface $container) { * contact form. */ public function contactSitePage(ContactFormInterface $contact_form = NULL) { - // Check if flood control has been activated for sending emails. - if (!$this->currentUser()->hasPermission('administer contact forms')) { - $this->contactFloodControl(); - } $config = $this->config('contact.settings'); // Use the default form if no form has been passed. @@ -106,6 +63,7 @@ public function contactSitePage(ContactFormInterface $contact_form = NULL) { $form = $this->entityFormBuilder()->getForm($message); $form['#title'] = SafeMarkup::checkPlain($contact_form->label()); + $form['#cache']['contexts'][] = 'user.permissions'; $form['#cache']['tags'] = Cache::mergeTags(isset($form['#cache']['tags']) ? $form['#cache']['tags'] : [], $config->getCacheTags()); return $form; } @@ -129,11 +87,6 @@ public function contactPersonalPage(UserInterface $user) { throw new NotFoundHttpException(); } - // Check if flood control has been activated for sending emails. - if (!$this->currentUser()->hasPermission('administer contact forms') && !$this->currentUser()->hasPermission('administer users')) { - $this->contactFloodControl(); - } - $message = $this->entityManager()->getStorage('contact_message')->create(array( 'contact_form' => 'personal', 'recipient' => $user->id(), @@ -141,24 +94,8 @@ public function contactPersonalPage(UserInterface $user) { $form = $this->entityFormBuilder()->getForm($message); $form['#title'] = $this->t('Contact @username', array('@username' => $user->getUsername())); + $form['#cache']['contexts'][] = 'user.permissions'; return $form; } - /** - * Throws an exception if the current user triggers flood control. - * - * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException - */ - protected function contactFloodControl() { - $limit = $this->config('contact.settings')->get('flood.limit'); - $interval = $this->config('contact.settings')->get('flood.interval'); - if (!$this->flood->isAllowed('contact', $limit, $interval)) { - drupal_set_message($this->t('You cannot send more than %limit messages in @interval. Try again later.', array( - '%limit' => $limit, - '@interval' => $this->dateFormatter->formatInterval($interval), - )), 'error'); - throw new AccessDeniedHttpException(); - } - } - } diff --git a/core/modules/contact/src/MessageForm.php b/core/modules/contact/src/MessageForm.php index 4384e9e..78de6dd 100644 --- a/core/modules/contact/src/MessageForm.php +++ b/core/modules/contact/src/MessageForm.php @@ -8,6 +8,7 @@ namespace Drupal\contact; use Drupal\Component\Utility\SafeMarkup; +use Drupal\Core\Datetime\DateFormatter; use Drupal\Core\Entity\ContentEntityForm; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Flood\FloodInterface; @@ -50,6 +51,13 @@ class MessageForm extends ContentEntityForm { protected $mailHandler; /** + * The date formatter service. + * + * @var \Drupal\Core\Datetime\DateFormatter + */ + protected $dateFormatter; + + /** * Constructs a MessageForm object. * * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager @@ -60,12 +68,15 @@ class MessageForm extends ContentEntityForm { * The language manager service. * @param \Drupal\contact\MailHandlerInterface $mail_handler * The contact mail handler service. + * @param \Drupal\Core\Datetime\DateFormatter $date_formatter + * The date service. */ - public function __construct(EntityManagerInterface $entity_manager, FloodInterface $flood, LanguageManagerInterface $language_manager, MailHandlerInterface $mail_handler) { + public function __construct(EntityManagerInterface $entity_manager, FloodInterface $flood, LanguageManagerInterface $language_manager, MailHandlerInterface $mail_handler, DateFormatter $date_formatter) { parent::__construct($entity_manager); $this->flood = $flood; $this->languageManager = $language_manager; $this->mailHandler = $mail_handler; + $this->dateFormatter = $date_formatter; } /** @@ -76,7 +87,8 @@ public static function create(ContainerInterface $container) { $container->get('entity.manager'), $container->get('flood'), $container->get('language_manager'), - $container->get('contact.mail_handler') + $container->get('contact.mail_handler'), + $container->get('date.formatter') ); } @@ -175,6 +187,28 @@ public function preview(array $form, FormStateInterface $form_state) { /** * {@inheritdoc} */ + public function validate(array $form, FormStateInterface $form_state) { + parent::validate($form, $form_state); + + $message = $this->entity; + + // Check if flood control has been activated for sending emails. + if (!$this->currentUser()->hasPermission('administer contact forms') && (!$message->isPersonal() || !$this->currentUser()->hasPermission('administer users'))) { + $limit = $this->config('contact.settings')->get('flood.limit'); + $interval = $this->config('contact.settings')->get('flood.interval'); + + if (!$this->flood->isAllowed('contact', $limit, $interval)) { + $form_state->setErrorByName('', $this->t('You cannot send more than %limit messages in @interval. Try again later.', array( + '%limit' => $limit, + '@interval' => $this->dateFormatter->formatInterval($interval), + ))); + } + } + } + + /** + * {@inheritdoc} + */ public function save(array $form, FormStateInterface $form_state) { $message = $this->entity; $user = $this->currentUser(); diff --git a/core/modules/contact/src/Tests/ContactPersonalTest.php b/core/modules/contact/src/Tests/ContactPersonalTest.php index 6b7638f..a586580 100644 --- a/core/modules/contact/src/Tests/ContactPersonalTest.php +++ b/core/modules/contact/src/Tests/ContactPersonalTest.php @@ -152,11 +152,13 @@ function testPersonalContactAccess() { // Test that anonymous users can access admin user's contact form. $this->drupalGet('user/' . $this->adminUser->id() . '/contact'); $this->assertResponse(200); + $this->assertCacheContext('user.permissions'); // Revoke the personal contact permission for the anonymous user. user_role_revoke_permissions(RoleInterface::ANONYMOUS_ID, array('access user contact forms')); $this->drupalGet('user/' . $this->contactUser->id() . '/contact'); $this->assertResponse(403); + $this->assertCacheContext('user.permissions'); $this->drupalGet('user/' . $this->adminUser->id() . '/contact'); $this->assertResponse(403); @@ -235,7 +237,7 @@ function testPersonalContactFlood() { } // Submit contact form one over limit. - $this->drupalGet('user/' . $this->contactUser->id(). '/contact'); + $this->submitPersonalContact($this->contactUser); $this->assertRaw(t('You cannot send more than %number messages in @interval. Try again later.', array('%number' => $flood_limit, '@interval' => \Drupal::service('date.formatter')->formatInterval($this->config('contact.settings')->get('flood.interval')))), 'Normal user denied access to flooded contact form.'); // Test that the admin user can still access the contact form even though diff --git a/core/modules/contact/src/Tests/ContactSitewideTest.php b/core/modules/contact/src/Tests/ContactSitewideTest.php index 793388a..df2816e 100644 --- a/core/modules/contact/src/Tests/ContactSitewideTest.php +++ b/core/modules/contact/src/Tests/ContactSitewideTest.php @@ -230,8 +230,7 @@ function testSiteWideContact() { $this->assertText(t('Your message has been sent.')); } // Submit contact form one over limit. - $this->drupalGet('contact'); - $this->assertResponse(403); + $this->submitContact($this->randomMachineName(16), $recipients[0], $this->randomMachineName(16), $id, $this->randomMachineName(64)); $this->assertRaw(t('You cannot send more than %number messages in @interval. Try again later.', array('%number' => $this->config('contact.settings')->get('flood.limit'), '@interval' => \Drupal::service('date.formatter')->formatInterval(600)))); // Test listing controller. diff --git a/core/modules/simpletest/src/WebTestBase.php b/core/modules/simpletest/src/WebTestBase.php index 29271ee..28f7200 100644 --- a/core/modules/simpletest/src/WebTestBase.php +++ b/core/modules/simpletest/src/WebTestBase.php @@ -2741,6 +2741,17 @@ protected function buildUrl($path, array $options = array()) { } /** + * Asserts whether an expected cache context was present in the last response. + * + * @param string $expected_cache_context + * The expected cache context. + */ + protected function assertCacheContext($expected_cache_context) { + $cache_contexts = explode(' ', $this->drupalGetHeader('X-Drupal-Cache-Contexts')); + $this->assertTrue(in_array($expected_cache_context, $cache_contexts), "'" . $expected_cache_context . "' is present in the X-Drupal-Cache-Contexts header."); + } + + /** * Asserts whether an expected cache tag was present in the last response. * * @param string $expected_cache_tag @@ -2748,7 +2759,7 @@ protected function buildUrl($path, array $options = array()) { */ protected function assertCacheTag($expected_cache_tag) { $cache_tags = explode(' ', $this->drupalGetHeader('X-Drupal-Cache-Tags')); - $this->assertTrue(in_array($expected_cache_tag, $cache_tags)); + $this->assertTrue(in_array($expected_cache_tag, $cache_tags), "'" . $expected_cache_tag . "' is present in the X-Drupal-Cache-Tags header."); } }