core/lib/Drupal/Core/Access/AccessManager.php | 5 +- core/modules/link/src/LinkItemInterface.php | 7 +-- .../link/src/Plugin/Field/FieldType/LinkItem.php | 17 +----- .../src/Plugin/Field/FieldWidget/LinkWidget.php | 69 +++++++++++++++++----- .../Validation/Constraint/LinkTypeConstraint.php | 33 +++++++---- .../src/Entity/MenuLinkContent.php | 5 +- .../src/Form/MenuLinkContentForm.php | 28 --------- core/modules/menu_ui/src/MenuForm.php | 2 +- .../menu_ui/src/Tests/MenuCacheTagsTest.php | 4 +- .../install/migrate.migration.d6_menu_links.yml | 5 +- .../src/Plugin/migrate/process/d6/UserPathUri.php | 35 +++++++++++ .../src/Tests/d6/MigrateMenuLinkTest.php | 4 +- .../rdf/src/Tests/Field/LinkFieldRdfaTest.php | 4 +- .../shortcut/src/Tests/ShortcutCacheTagsTest.php | 2 +- .../src/Tests/ShortcutTranslationUITest.php | 2 +- 15 files changed, 138 insertions(+), 84 deletions(-) diff --git a/core/lib/Drupal/Core/Access/AccessManager.php b/core/lib/Drupal/Core/Access/AccessManager.php index a59f697..3cd659d 100644 --- a/core/lib/Drupal/Core/Access/AccessManager.php +++ b/core/lib/Drupal/Core/Access/AccessManager.php @@ -87,8 +87,9 @@ public function checkNamedRoute($route_name, array $parameters = array(), Accoun try { $route = $this->routeProvider->getRouteByName($route_name, $parameters); - // ParamConverterManager relies on the route object being available - // from the parameters array. + // ParamConverterManager relies on the route name and object being + // available from the parameters array. + $parameters[RouteObjectInterface::ROUTE_NAME] = $route_name; $parameters[RouteObjectInterface::ROUTE_OBJECT] = $route; $upcasted_parameters = $this->paramConverterManager->convert($parameters + $route->getDefaults()); diff --git a/core/modules/link/src/LinkItemInterface.php b/core/modules/link/src/LinkItemInterface.php index cd3bce2..f6a48f0 100644 --- a/core/modules/link/src/LinkItemInterface.php +++ b/core/modules/link/src/LinkItemInterface.php @@ -40,11 +40,8 @@ public function isExternal(); /** * Gets the URL object. * - * @return \Drupal\Core\Url|false - * Returns an Url object if any of the following are true: - * - The URI is external. - * - The URI is internal and valid. - * Otherwise, FALSE is returned. + * @return \Drupal\Core\Url + * Returns an Url object. */ public function getUrl(); diff --git a/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php b/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php index 8af3c46..66433c7 100644 --- a/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php +++ b/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php @@ -15,6 +15,7 @@ use Drupal\Core\Form\FormStateInterface; use Drupal\Core\TypedData\DataDefinition; use Drupal\Core\TypedData\MapDataDefinition; +use Drupal\Core\Url; use Drupal\link\LinkItemInterface; /** @@ -164,21 +165,9 @@ public static function mainPropertyName() { /** * {@inheritdoc} - * - * @todo Remove the $access_check parameter and replace all logic in the - * function body with a call to Url::fromUri() in - * https://www.drupal.org/node/2416987. */ - public function getUrl($access_check = FALSE) { - $uri = $this->uri; - $scheme = parse_url($uri, PHP_URL_SCHEME); - if ($scheme === 'user-path') { - $uri_reference = explode(':', $uri, 2)[1]; - } - else { - $uri_reference = $uri; - } - return $access_check ? \Drupal::pathValidator()->getUrlIfValid($uri_reference) : \Drupal::pathValidator()->getUrlIfValidWithoutAccessCheck($uri_reference); + public function getUrl() { + return Url::fromUri($this->uri); } /** diff --git a/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php index 3230a41..08af5d3 100644 --- a/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php +++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php @@ -12,6 +12,7 @@ use Drupal\Core\Field\FieldItemListInterface; use Drupal\Core\Field\WidgetBase; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\Url; use Drupal\link\LinkItemInterface; use Symfony\Component\Validator\ConstraintViolation; use Symfony\Component\Validator\ConstraintViolationInterface; @@ -60,6 +61,32 @@ protected function getUriAsDisplayableString($uri) { } /** + * This link widget disallows saving URIs inaccessible to the current user. + */ + public function elementValidate($element, FormStateInterface $form_state, $form) { + $uri = $this->getUserEnteredStringAsUri($element['#value']); + + // If the URI is empty or not well-formed, the link field type's validation + // constraint will detect it. + // @see \Drupal\link\Plugin\Validation\Constraint\LinkTypeConstraint::validate() + if (!empty($uri) && parse_url($uri)) { + $url = Url::fromUri($uri); + + // Disallow unrouted internal URLs (i.e. disallow 'base:' URIs). + $disallowed = !$url->isRouted() && !$url->isExternal(); + // Disallow URLs if the current user doesn't have the 'link to any page' + // permission nor can access this URI. + $disallowed = $disallowed || (!\Drupal::currentUser()->hasPermission('link to any page') && !$url->access()); + // Disallow external URLs using untrusted protocols. + $disallowed = $disallowed || ($url->isExternal() && !in_array(parse_url($uri, PHP_URL_SCHEME), \Drupal::config('system.filter')->get('protocols'))); + + if ($disallowed) { + $form_state->setError($element, $this->t('The URL %uri is not valid.', ['%uri' => $this->getUriAsDisplayableString($uri)])); + } + } + } + + /** * {@inheritdoc} */ public function formElement(FieldItemListInterface $items, $delta, array $element, array &$form, FormStateInterface $form_state) { @@ -73,9 +100,8 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen // The current field value could have been entered by a different user. // However, if it is inaccessible to the current user, do not display it // to them. - // @todo Revisit this access requirement in - // https://www.drupal.org/node/2416987. - '#default_value' => $item->getUrl(TRUE) ? $this->getUriAsDisplayableString($item->uri) : NULL, + '#default_value' => (!$item->isEmpty() && (\Drupal::currentUser()->hasPermission('link to any page') || $item->getUrl()->access())) ? $this->getUriAsDisplayableString($item->uri) : NULL, + '#element_validate' => array(array($this, 'elementValidate')), '#maxlength' => 2048, '#required' => $element['#required'], ); @@ -219,19 +245,34 @@ public function validateTitle(&$element, FormStateInterface $form_state, $form) } /** + * Gets the user-entered string as a URI. + * + * Schemeless URIs are treated as 'user-path:' URIs. + * + * @param string $uri + * The user-entered string. + * + * @return string + * The URI, if a non-empty $uri was passed. + */ + protected function getUserEnteredStringAsUri($uri) { + if (!empty($uri)) { + // Users can enter relative URLs, but we need a valid URI, so add an + // explicit scheme when necessary. + if (parse_url($uri, PHP_URL_SCHEME) === NULL) { + $uri = 'user-path:' . $uri; + } + } + return $uri; + } + + /** * {@inheritdoc} */ public function massageFormValues(array $values, array $form, FormStateInterface $form_state) { foreach ($values as &$value) { - if (!empty($value['uri'])) { - // Users can enter relative URLs, but we need a valid URI, so add an - // explicit scheme when necessary. - if (parse_url($value['uri'], PHP_URL_SCHEME) === NULL) { - $value['uri'] = 'user-path:' . $value['uri']; - } - - $value += ['options' => []]; - } + $value['uri'] = $this->getUserEnteredStringAsUri($value['uri']); + $value += ['options' => []]; } return $values; } @@ -240,7 +281,7 @@ public function massageFormValues(array $values, array $form, FormStateInterface /** * {@inheritdoc} * - * Override the '%url' message parameter, to ensure that 'user-path:' URIs + * Override the '%uri' message parameter, to ensure that 'user-path:' URIs * show a validation error message that doesn't mention that scheme. */ public function flagErrors(FieldItemListInterface $items, ConstraintViolationListInterface $violations, array $form, FormStateInterface $form_state) { @@ -248,7 +289,7 @@ public function flagErrors(FieldItemListInterface $items, ConstraintViolationLis foreach ($violations as $offset => $violation) { $parameters = $violation->getMessageParameters(); if (isset($parameters['%url'])) { - $parameters['%url'] = $this->getUriAsDisplayableString($parameters['%url']); + $parameters['%uri'] = $this->getUriAsDisplayableString($parameters['%uri']); $violations->set($offset, new ConstraintViolation( $this->t($violation->getMessageTemplate(), $parameters), $violation->getMessageTemplate(), diff --git a/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php b/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php index 22b1942..ca5ca43 100644 --- a/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php +++ b/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php @@ -22,7 +22,7 @@ */ class LinkTypeConstraint extends Constraint implements ConstraintValidatorInterface { - public $message = 'The URL %url is not valid.'; + public $message = 'The URL %uri is not valid.'; /** * @var \Symfony\Component\Validator\ExecutionContextInterface @@ -48,23 +48,34 @@ public function validatedBy() { */ public function validate($value, Constraint $constraint) { if (isset($value)) { - $url_is_valid = FALSE; + $uri_is_valid = TRUE; + /** @var $link_item \Drupal\link\LinkItemInterface */ $link_item = $value; $link_type = $link_item->getFieldDefinition()->getSetting('link_type'); - $url = $link_item->getUrl(TRUE); - if ($url) { - $url_is_valid = TRUE; - if ($url->isExternal() && !($link_type & LinkItemInterface::LINK_EXTERNAL)) { - $url_is_valid = FALSE; + // Try to resolve the given URI to a URL. It may fail if it's schemeless. + try { + $url = $link_item->getUrl(); + } + catch (\InvalidArgumentException $e) { + $uri_is_valid = FALSE; + } + + // If the link field doesn't support both internal and external links, + // check whether the URL (a resolved URI) is in fact violating either + // restriction. + if ($uri_is_valid && !($link_type & LinkItemInterface::LINK_GENERIC)) { + if (!($link_type & LinkItemInterface::LINK_EXTERNAL) && $url->isExternal()) { + $uri_is_valid = FALSE; } - if (!$url->isExternal() && !($link_type & LinkItemInterface::LINK_INTERNAL)) { - $url_is_valid = FALSE; + if (!($link_type & LinkItemInterface::LINK_INTERNAL) && !$url->isExternal()) { + $uri_is_valid = FALSE; } } - if (!$url_is_valid) { - $this->context->addViolation($this->message, array('%url' => $link_item->uri)); + + if (!$uri_is_valid) { + $this->context->addViolation($this->message, array('%uri' => $link_item->uri)); } } } diff --git a/core/modules/menu_link_content/src/Entity/MenuLinkContent.php b/core/modules/menu_link_content/src/Entity/MenuLinkContent.php index 420ebca..b67db69 100644 --- a/core/modules/menu_link_content/src/Entity/MenuLinkContent.php +++ b/core/modules/menu_link_content/src/Entity/MenuLinkContent.php @@ -149,7 +149,10 @@ protected function getPluginDefinition() { $definition['menu_name'] = $this->getMenuName(); if ($url_object = $this->getUrlObject()) { - if ($url_object->isExternal()) { + $definition['url'] = NULL; + $definition['route_name'] = NULL; + $definition['route_parameters'] = []; + if (!$url_object->isRouted()) { $definition['url'] = $url_object->getUri(); } else { diff --git a/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php b/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php index d4180ed..a0e9a28 100644 --- a/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php +++ b/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php @@ -244,15 +244,6 @@ protected function actions(array $form, FormStateInterface $form_state) { /** * {@inheritdoc} */ - public function validate(array $form, FormStateInterface $form_state) { - $this->doValidate($form, $form_state); - - parent::validate($form, $form_state); - } - - /** - * {@inheritdoc} - */ public function buildEntity(array $form, FormStateInterface $form_state) { /** @var \Drupal\menu_link_content\MenuLinkContentInterface $entity */ $entity = parent::buildEntity($form, $form_state); @@ -287,23 +278,4 @@ public function save(array $form, FormStateInterface $form_state) { } } - /** - * Validates the form, both on the menu link edit and content menu link form. - * - * $form is not currently used, but passed here to match the normal form - * validation method signature. - * - * @param array $form - * A nested array form elements comprising the form. - * @param \Drupal\Core\Form\FormStateInterface $form_state - * The current state of the form. - */ - protected function doValidate(array $form, FormStateInterface $form_state) { - $extracted = $this->pathValidator->getUrlIfValid($form_state->getValue(['link', 0, 'uri'])); - - if (!$extracted) { - $form_state->setErrorByName('link][0][uri', $this->t("The path '@link_path' is either invalid or you do not have access to it.", array('@link_path' => $form_state->getValue(['link', 0, 'uri'])))); - } - } - } diff --git a/core/modules/menu_ui/src/MenuForm.php b/core/modules/menu_ui/src/MenuForm.php index b5aef7b..987858e 100644 --- a/core/modules/menu_ui/src/MenuForm.php +++ b/core/modules/menu_ui/src/MenuForm.php @@ -346,7 +346,7 @@ protected function buildOverviewTreeForm($tree, $delta) { if (!$link->isEnabled()) { $form[$id]['title']['#markup'] .= ' (' . $this->t('disabled') . ')'; } - elseif (($url = $link->getUrlObject()) && !$url->isExternal() && $url->getRouteName() == 'user.page') { + elseif (($url = $link->getUrlObject()) && $url->isRouted() && $url->getRouteName() == 'user.page') { $form[$id]['title']['#markup'] .= ' (' . $this->t('logged in users only') . ')'; } diff --git a/core/modules/menu_ui/src/Tests/MenuCacheTagsTest.php b/core/modules/menu_ui/src/Tests/MenuCacheTagsTest.php index d1d05c4..7282ca2 100644 --- a/core/modules/menu_ui/src/Tests/MenuCacheTagsTest.php +++ b/core/modules/menu_ui/src/Tests/MenuCacheTagsTest.php @@ -82,7 +82,9 @@ public function testMenuBlock() { 'parent' => '', 'title' => 'Alpaca', 'menu_name' => 'llama', - 'route_name' => '', + 'link' => [[ + 'uri' => 'user-path:', + ]], 'bundle' => 'menu_name', )); $menu_link_2->save(); diff --git a/core/modules/migrate_drupal/config/install/migrate.migration.d6_menu_links.yml b/core/modules/migrate_drupal/config/install/migrate.migration.d6_menu_links.yml index 5708f02..41b7fa3 100644 --- a/core/modules/migrate_drupal/config/install/migrate.migration.d6_menu_links.yml +++ b/core/modules/migrate_drupal/config/install/migrate.migration.d6_menu_links.yml @@ -22,7 +22,10 @@ process: plugin: migration migration: d6_menu source: menu_name - 'link/uri': link_path + 'link/uri': + plugin: userpath_uri + source: + - link_path 'link/options': options external: external weight: weight diff --git a/core/modules/migrate_drupal/src/Plugin/migrate/process/d6/UserPathUri.php b/core/modules/migrate_drupal/src/Plugin/migrate/process/d6/UserPathUri.php new file mode 100644 index 0000000..d2512d0 --- /dev/null +++ b/core/modules/migrate_drupal/src/Plugin/migrate/process/d6/UserPathUri.php @@ -0,0 +1,35 @@ +assertIdentical($menu_link->isEnabled(), TRUE); $this->assertIdentical($menu_link->isExpanded(), FALSE); $this->assertIdentical($menu_link->link->options, ['attributes' => ['title' => 'Test menu link 1']]); - $this->assertIdentical($menu_link->link->uri, 'user/login'); + $this->assertIdentical($menu_link->link->uri, 'user-path:user/login'); $this->assertIdentical($menu_link->getWeight(), 15); $menu_link = entity_load('menu_link_content', 139); @@ -67,7 +67,7 @@ public function testMenuLinks() { $this->assertIdentical($menu_link->isEnabled(), TRUE); $this->assertIdentical($menu_link->isExpanded(), TRUE); $this->assertIdentical($menu_link->link->options, ['query' => ['foo' => 'bar'], 'attributes' => ['title' => ['Test menu link 2']]]); - $this->assertIdentical($menu_link->link->uri, 'admin'); + $this->assertIdentical($menu_link->link->uri, 'user-path:admin'); $this->assertIdentical($menu_link->getWeight(), 12); $menu_link = entity_load('menu_link_content', 140); diff --git a/core/modules/rdf/src/Tests/Field/LinkFieldRdfaTest.php b/core/modules/rdf/src/Tests/Field/LinkFieldRdfaTest.php index 0c3e257..db77161 100644 --- a/core/modules/rdf/src/Tests/Field/LinkFieldRdfaTest.php +++ b/core/modules/rdf/src/Tests/Field/LinkFieldRdfaTest.php @@ -84,9 +84,9 @@ public function testAllFormattersInternal() { */ public function testAllFormattersFront() { // Set up test values. - $this->testValue = ''; + $this->testValue = 'user-path:'; $this->entity = entity_create('entity_test', array()); - $this->entity->{$this->fieldName}->uri = ''; + $this->entity->{$this->fieldName}->uri = 'user-path:'; // Set up the expected result. $expected_rdf = array( diff --git a/core/modules/shortcut/src/Tests/ShortcutCacheTagsTest.php b/core/modules/shortcut/src/Tests/ShortcutCacheTagsTest.php index e116214..46e37cb 100644 --- a/core/modules/shortcut/src/Tests/ShortcutCacheTagsTest.php +++ b/core/modules/shortcut/src/Tests/ShortcutCacheTagsTest.php @@ -46,7 +46,7 @@ protected function createEntity() { 'shortcut_set' => 'default', 'title' => t('Llama'), 'weight' => 0, - 'link' => ['uri' => 'admin'], + 'link' => [['uri' => 'user-path:admin']], )); $shortcut->save(); diff --git a/core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php b/core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php index 2944e1f..07b8bec 100644 --- a/core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php +++ b/core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php @@ -50,7 +50,7 @@ protected function getTranslatorPermissions() { * {@inheritdoc} */ protected function createEntity($values, $langcode, $bundle_name = NULL) { - $values['link']['uri'] = 'user'; + $values['link']['uri'] = 'user-path:user'; return parent::createEntity($values, $langcode, $bundle_name); }