Commit 4ae39d2f authored by webchick's avatar webchick

Issue #2417645 by tim.plunkett, effulgentsia, pwolanin: Change destination...

Issue #2417645 by tim.plunkett, effulgentsia, pwolanin: Change destination query string handling to break dependence on system path
parent b01c7c45
......@@ -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;
......
......@@ -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);
......
......@@ -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;
}
......
......@@ -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;
}
......
......@@ -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')));
......
......@@ -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'])) {
......
......@@ -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.
......
......@@ -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 {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment