From 4ae39d2fb82c7f12d0e4f5ddc63a4c6fa269fb05 Mon Sep 17 00:00:00 2001
From: webchick <drupal@webchick.net>
Date: Mon, 2 Feb 2015 13:58:16 -0800
Subject: [PATCH] Issue #2417645 by tim.plunkett, effulgentsia, pwolanin:
Change destination query string handling to break dependence on system path
---
core/includes/common.inc | 12 ++++---
.../RedirectResponseSubscriber.php | 34 +++++++++++++------
.../src/BlockContentListBuilder.php | 2 +-
.../BlockContentAddLocalAction.php | 4 ++-
core/modules/comment/src/CommentManager.php | 16 ++++-----
core/modules/menu_ui/src/MenuForm.php | 8 +++--
core/modules/user/src/Tests/UserAdminTest.php | 2 +-
core/modules/views_ui/src/ViewEditForm.php | 2 +-
8 files changed, 51 insertions(+), 29 deletions(-)
diff --git a/core/includes/common.inc b/core/includes/common.inc
index 79f16db246e3..5d88545c1f7a 100644
--- a/core/includes/common.inc
+++ b/core/includes/common.inc
@@ -210,11 +210,15 @@ function drupal_get_html_head($render = TRUE) {
* previous request, that destination is returned. As such, a destination can
* persist across multiple pages.
*
- * @return
+ * @return array
* An associative array containing the key:
- * - destination: The path provided via the destination query string or, if
- * not available, the current path.
+ * - destination: The value of the current request's 'destination' query
+ * parameter, if present. This can be either a relative or absolute URL.
+ * However, for security, redirection to external URLs is not performed.
+ * If the query parameter isn't present, then the URL of the current
+ * request is returned.
*
+ * @see \Drupal\Core\EventSubscriber\RedirectResponseSubscriber::checkRedirectUrl()
* @ingroup form_api
*/
function drupal_get_destination() {
@@ -229,7 +233,7 @@ function drupal_get_destination() {
$destination = array('destination' => $query->get('destination'));
}
else {
- $path = \Drupal::routeMatch()->getRouteName() ? Url::fromRouteMatch(\Drupal::routeMatch())->getInternalPath() : '';
+ $path = \Drupal::url('<current>');
$query = UrlHelper::buildQuery(UrlHelper::filterQueryParameters($query->all()));
if ($query != '') {
$path .= '?' . $query;
diff --git a/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
index bcab53841860..1a468f1812e1 100644
--- a/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
+++ b/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
@@ -52,7 +52,8 @@ public function checkRedirectUrl(FilterResponseEvent $event) {
if ($response instanceOf RedirectResponse) {
$options = array();
- $destination = $event->getRequest()->query->get('destination');
+ $request = $event->getRequest();
+ $destination = $request->query->get('destination');
// A destination from \Drupal::request()->query always overrides the
// current RedirectResponse. We do not allow absolute URLs to be passed
// via \Drupal::request()->query, as this can be an attack vector, with
@@ -61,15 +62,28 @@ public function checkRedirectUrl(FilterResponseEvent $event) {
// base path) are allowed.
if ($destination) {
if (!UrlHelper::isExternal($destination)) {
- $destination = UrlHelper::parse($destination);
-
- $path = $destination['path'];
- $options['query'] = $destination['query'];
- $options['fragment'] = $destination['fragment'];
- // The 'Location' HTTP header contain an absolute URL.
- $options['absolute'] = TRUE;
-
- $response->setTargetUrl($this->urlGenerator->generateFromPath($path, $options));
+ // The destination query parameter can be a relative URL in the sense
+ // of not including the scheme and host, but its path is expected to
+ // be absolute (start with a '/'). For such a case, prepend the
+ // scheme and host, because the 'Location' header must be absolute.
+ if (strpos($destination, '/') === 0) {
+ $destination = $request->getSchemeAndHttpHost() . $destination;
+ }
+ else {
+ // Legacy destination query parameters can be relative paths that
+ // have not yet been converted to URLs (outbound path processors
+ // and other URL handling still needs to be performed).
+ // @todo As generateFromPath() is deprecated, remove this in
+ // https://www.drupal.org/node/2418219.
+ $destination = UrlHelper::parse($destination);
+ $path = $destination['path'];
+ $options['query'] = $destination['query'];
+ $options['fragment'] = $destination['fragment'];
+ // The 'Location' HTTP header must always be absolute.
+ $options['absolute'] = TRUE;
+ $destination = $this->urlGenerator->generateFromPath($path, $options);
+ }
+ $response->setTargetUrl($destination);
}
elseif (UrlHelper::externalIsLocal($destination, $this->requestContext->getCompleteBaseUrl())) {
$response->setTargetUrl($destination);
diff --git a/core/modules/block_content/src/BlockContentListBuilder.php b/core/modules/block_content/src/BlockContentListBuilder.php
index 731f2dac9688..793428da2438 100644
--- a/core/modules/block_content/src/BlockContentListBuilder.php
+++ b/core/modules/block_content/src/BlockContentListBuilder.php
@@ -39,7 +39,7 @@ public function buildRow(EntityInterface $entity) {
public function getDefaultOperations(EntityInterface $entity) {
$operations = parent::getDefaultOperations($entity);
if (isset($operations['edit'])) {
- $operations['edit']['query']['destination'] = 'admin/structure/block/block-content';
+ $operations['edit']['query']['destination'] = $entity->url('collection');
}
return $operations;
}
diff --git a/core/modules/block_content/src/Plugin/Menu/LocalAction/BlockContentAddLocalAction.php b/core/modules/block_content/src/Plugin/Menu/LocalAction/BlockContentAddLocalAction.php
index 86921ec19d40..62e0c3cf0e88 100644
--- a/core/modules/block_content/src/Plugin/Menu/LocalAction/BlockContentAddLocalAction.php
+++ b/core/modules/block_content/src/Plugin/Menu/LocalAction/BlockContentAddLocalAction.php
@@ -9,11 +9,13 @@
use Drupal\Core\Menu\LocalActionDefault;
use Drupal\Core\Routing\RouteMatchInterface;
+use Drupal\Core\Routing\UrlGeneratorTrait;
/**
* Modifies the 'Add custom block' local action.
*/
class BlockContentAddLocalAction extends LocalActionDefault {
+ use UrlGeneratorTrait;
/**
* {@inheritdoc}
@@ -26,7 +28,7 @@ public function getOptions(RouteMatchInterface $route_match) {
}
// Adds a destination on custom block listing.
if ($route_match->getRouteName() == 'entity.block_content.collection') {
- $options['query']['destination'] = 'admin/structure/block/block-content';
+ $options['query']['destination'] = $this->url('<current>');
}
return $options;
}
diff --git a/core/modules/comment/src/CommentManager.php b/core/modules/comment/src/CommentManager.php
index ef884e1e1a26..e1a6f5fb62c4 100644
--- a/core/modules/comment/src/CommentManager.php
+++ b/core/modules/comment/src/CommentManager.php
@@ -16,6 +16,7 @@
use Drupal\Core\Entity\Query\QueryFactory;
use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\Routing\UrlGeneratorInterface;
+use Drupal\Core\Routing\UrlGeneratorTrait;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\StringTranslation\StringTranslationTrait;
use Drupal\Core\StringTranslation\TranslationInterface;
@@ -27,6 +28,7 @@
*/
class CommentManager implements CommentManagerInterface {
use StringTranslationTrait;
+ use UrlGeneratorTrait;
/**
* The entity manager service.
@@ -56,13 +58,6 @@ class CommentManager implements CommentManagerInterface {
*/
protected $userConfig;
- /**
- * The url generator service.
- *
- * @var \Drupal\Core\Routing\UrlGeneratorInterface
- */
- protected $urlGenerator;
-
/**
* The module handler service.
*
@@ -260,7 +255,12 @@ public function forbiddenMessage(EntityInterface $entity, $field_name) {
// We cannot use drupal_get_destination() because these links
// sometimes appear on /node and taxonomy listing pages.
if ($entity->get($field_name)->getFieldDefinition()->getSetting('form_location') == CommentItemInterface::FORM_SEPARATE_PAGE) {
- $destination = array('destination' => 'comment/reply/' . $entity->getEntityTypeId() . '/' . $entity->id() . '/' . $field_name . '#comment-form');
+ $comment_reply_parameters = [
+ 'entity_type' => $entity->getEntityTypeId(),
+ 'entity' => $entity->id(),
+ 'field_name' => $field_name,
+ ];
+ $destination = array('destination' => $this->url('comment.reply', $comment_reply_parameters, array('fragment' => 'comment-form')));
}
else {
$destination = array('destination' => $entity->url('canonical', array('fragment' => 'comment-form')));
diff --git a/core/modules/menu_ui/src/MenuForm.php b/core/modules/menu_ui/src/MenuForm.php
index 987858e832f8..fd38c02c7107 100644
--- a/core/modules/menu_ui/src/MenuForm.php
+++ b/core/modules/menu_ui/src/MenuForm.php
@@ -275,9 +275,11 @@ protected function buildOverviewForm(array &$form, FormStateInterface $form_stat
),
);
- $destination = $this->getUrlGenerator()->getPathFromRoute('entity.menu.edit_form', array('menu' => $this->entity->id()));
- $url = $destination = $this->url('entity.menu.add_link_form', array('menu' => $this->entity->id()), array('query' => array('destination' => $destination)));
- $form['links']['#empty'] = $this->t('There are no menu links yet. <a href="@url">Add link</a>.', array('@url' => $url));
+ $form['links']['#empty'] = $this->t('There are no menu links yet. <a href="@url">Add link</a>.', [
+ '@url' => $this->url('entity.menu.add_link_form', ['menu' => $this->entity->id()], [
+ 'query' => ['destination' => $this->entity->url('edit-form')],
+ ]),
+ ]);
$links = $this->buildOverviewTreeForm($tree, $delta);
foreach (Element::children($links) as $id) {
if (isset($links[$id]['#item'])) {
diff --git a/core/modules/user/src/Tests/UserAdminTest.php b/core/modules/user/src/Tests/UserAdminTest.php
index 135585dee9eb..de4749ed15e5 100644
--- a/core/modules/user/src/Tests/UserAdminTest.php
+++ b/core/modules/user/src/Tests/UserAdminTest.php
@@ -52,7 +52,7 @@ function testUserAdmin() {
$this->assertText($admin_user->getUsername(), 'Found Admin user on admin users page');
// Test for existence of edit link in table.
- $link = $user_a->link(t('Edit'), 'edit-form', array('query' => array('destination' => 'admin/people')));
+ $link = $user_a->link(t('Edit'), 'edit-form', array('query' => array('destination' => $user_a->url('collection'))));
$this->assertRaw($link, 'Found user A edit link on admin users page');
// Test exposed filter elements.
diff --git a/core/modules/views_ui/src/ViewEditForm.php b/core/modules/views_ui/src/ViewEditForm.php
index 363474dd47ea..37fc363ea159 100644
--- a/core/modules/views_ui/src/ViewEditForm.php
+++ b/core/modules/views_ui/src/ViewEditForm.php
@@ -727,7 +727,7 @@ public function renderDisplayTop(ViewUI $view) {
$element['extra_actions']['#links']['revert'] = array(
'title' => $this->t('Revert view'),
'href' => "admin/structure/views/view/{$view->id()}/revert",
- 'query' => array('destination' => "admin/structure/views/view/{$view->id()}"),
+ 'query' => array('destination' => $view->url('edit-form')),
);
}
else {
--
GitLab