core/lib/Drupal/Core/Path/PathValidator.php | 12 +++--- core/lib/Drupal/Core/Url.php | 36 +++++++++++++++++- .../Drupal/Core/Utility/UnroutedUrlAssembler.php | 4 -- .../src/Plugin/Field/FieldWidget/LinkWidget.php | 44 ++++++++++++++++++---- .../modules/menu_ui/src/Tests/MenuLanguageTest.php | 2 +- core/modules/menu_ui/src/Tests/MenuTest.php | 15 ++++---- .../rdf/src/Tests/Field/LinkFieldRdfaTest.php | 6 +-- .../shortcut/src/Tests/ShortcutLinksTest.php | 7 ++-- .../Drupal/Tests/Core/Path/PathValidatorTest.php | 16 +++++++- core/tests/Drupal/Tests/Core/UrlTest.php | 28 +++++++++++--- 10 files changed, 127 insertions(+), 43 deletions(-) diff --git a/core/lib/Drupal/Core/Path/PathValidator.php b/core/lib/Drupal/Core/Path/PathValidator.php index b0a69f8..fcf0968 100644 --- a/core/lib/Drupal/Core/Path/PathValidator.php +++ b/core/lib/Drupal/Core/Path/PathValidator.php @@ -106,13 +106,11 @@ protected function getUrl($path, $access_check) { $options['fragment'] = $parsed_url['fragment']; } - if (parse_url($path, PHP_URL_SCHEME) === NULL) { - if (empty($parsed_url['path'])) { - return new Url('', [], $options); - } - elseif ($parsed_url['path'] === '/') { - return new Url('', [], $options); - } + if ($parsed_url['path'] == '') { + return new Url('', [], $options); + } + elseif ($parsed_url['path'] == '') { + return new Url('', [], $options); } elseif (UrlHelper::isExternal($path) && UrlHelper::isValid($path)) { if (empty($parsed_url['path'])) { diff --git a/core/lib/Drupal/Core/Url.php b/core/lib/Drupal/Core/Url.php index e9f943f..f8b901f 100644 --- a/core/lib/Drupal/Core/Url.php +++ b/core/lib/Drupal/Core/Url.php @@ -319,6 +319,30 @@ protected static function fromEntityUri(array $uri_parts, array $options, $uri) /** * Creates a new Url object for 'user-path:' URIs. * + * Important note: the URI minus the scheme can NOT simply be validated by a + * \Drupal\Core\Path\PathValidatorInterface implementation. The semantics of + * the 'user-path:' URI scheme are different: + * - PathValidatorInterface accepts paths without a leading slash (e.g. + * 'node/add') as well as 2 special paths: '' and '', which are + * mapped to the correspondingly named routes. + * - 'user-path:' URIs store paths with a leading slash that represents the + * root — i.e. the front page — (e.g. 'user-path:/node/add'), and doesn't + * have any exceptions. + * + * To clarify, a few examples of path plus corresponding 'user-path:' URI: + * - 'node/add' -> 'user-path:/node/add' + * - 'node/add?foo=bar' -> 'user-path:/node/add?foo=bar' + * - 'node/add#kitten' -> 'user-path:/node/add#kitten' + * - '' -> 'user-path:/' + * - 'foo=bar' -> 'user-path:/?foo=bar' + * - '#kitten' -> 'user-path:/#kitten' + * - '' -> 'user-path:' + * - 'foo=bar' -> 'user-path:?foo=bar' + * - '#kitten' -> 'user-path:#kitten' + * + * Therefore, when using a PathValidatorInterface to validate 'user-path:' + * URIs, we must ensure to not use it in case of the + * * @param array $uri_parts * Parts from an URI of the form user-path:{path} as from parse_url(). * @param array $options @@ -332,7 +356,17 @@ protected static function fromUserPathUri(array $uri_parts, array $options) { // only accept/contain paths without a leading slash, unlike 'user-path:' // URIs, for which the leading slash means "relative to Drupal root" and // "relative to Symfony app root" (just like in Symfony/Drupal 8 routes). - $uri_parts['path'] = ltrim($uri_parts['path'], '/'); + if (empty($uri_parts['path'])) { + $uri_parts['path'] = ''; + } + elseif ($uri_parts['path'] === '/') { + $uri_parts['path'] = ''; + } + else { + // Remove the leading slash. + $uri_parts['path'] = substr($uri_parts['path'], 1); + } + $url = \Drupal::pathValidator() ->getUrlIfValidWithoutAccessCheck($uri_parts['path']) ?: static::fromUri('base:' . $uri_parts['path'], $options); // Allow specifying additional options. diff --git a/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php index 6de2e50..bc21ea8 100644 --- a/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php +++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php @@ -113,10 +113,6 @@ protected function buildLocalUrl($uri, array $options = []) { // https://www.drupal.org/node/2417459 $uri = substr($uri, 5); - // Trim leading slashes. Ensures that it's impossible to generate scheme- - // relative URLs. - $uri = ltrim($uri, '/'); - // Allow (outbound) path processing, if needed. A valid use case is the path // alias overview form: // @see \Drupal\path\Controller\PathController::adminOverview(). diff --git a/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php index f310cdb..ae85ebf 100644 --- a/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php +++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php @@ -53,6 +53,20 @@ protected static function getUriAsDisplayableString($uri) { $scheme = parse_url($uri, PHP_URL_SCHEME); if ($scheme === 'user-path') { $uri_reference = explode(':', $uri, 2)[1]; + // @todo Present the leading slash to the user and hence delete the next + // block in https://www.drupal.org/node/2418017. There, we will also + // remove the ability to enter '' or '', we'll expect '/' + // and '' instead respectively. + $path = parse_url($uri, PHP_URL_PATH); + if ($path === '/') { + $uri_reference = '' . substr($uri_reference, 1); + } + elseif (empty($path)) { + $uri_reference = '' . $uri_reference; + } + else { + $uri_reference = ltrim($uri_reference, '/'); + } } else { $uri_reference = $uri; @@ -76,17 +90,31 @@ protected static function getUserEnteredStringAsUri($string) { // Users can enter relative URLs, but we need a valid URI, so add an // explicit scheme when necessary. if (parse_url($string, PHP_URL_SCHEME) === NULL) { + // @todo Present the leading slash to the user and hence delete the next + // block in https://www.drupal.org/node/2418017. There, we will also + // remove the ability to enter '' or '', we'll expect '/' + // and '' instead respectively. // Users can enter paths that don't start with a leading slash, we // want to normalize them to have a leading slash. However, we don't // want to add a leading slash if it already starts with one, or if it // contains only a querystring or a fragment. Examples: // - 'foo' -> '/foo' - // - '/foo' -> '/foo' - // - '?foo=bar' -> '?foo=bar' - // - '#foo' -> '#foo' - if (!in_array($string[0], ['/', '?', '#'])) { + // - '?foo=bar' -> '/?foo=bar' + // - '#foo' -> '/#foo' + // - '' -> '/' + // - '#foo' -> '/#foo' + // - '' -> '' + // - '#foo' -> '#foo' + if (strpos($string, '') === 0) { + $string = '/' . substr($string, strlen('')); + } + elseif (strpos($string, '') === 0) { + $string = substr($string, strlen('')); + } + elseif (!in_array($string[0], ['/', '?', '#'])) { $string = '/' . $string; } + return 'user-path:' . $string; } } @@ -145,15 +173,15 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen $element['uri']['#type'] = 'textfield'; } - // If the field is configured to allow only internal links, show a useful - // description. + // If the field is configured to allow only internal links, add a useful + // element prefix. if (!$this->supportsExternalLinks()) { - $element['uri']['#description'] = $this->t('This can be an internal Drupal path such as %add-node. Enter %front to link to the front page.', array('%front' => '/', '%add-node' => '/node/add')); + $element['uri']['#field_prefix'] = \Drupal::url('', array(), array('absolute' => TRUE)); } // If the field is configured to allow both internal and external links, // show a useful description. elseif ($this->supportsExternalLinks() && $this->supportsInternalLinks()) { - $element['uri']['#description'] = $this->t('This can be an internal Drupal path such as %add-node or an external URL such as %drupal. Enter %front to link to the front page.', array('%front' => '/', '%add-node' => '/node/add', '%drupal' => 'http://drupal.org')); + $element['uri']['#description'] = $this->t('This can be an internal Drupal path such as %add-node or an external URL such as %drupal. Enter %front to link to the front page.', array('%front' => '', '%add-node' => 'node/add', '%drupal' => 'http://drupal.org')); } $element['title'] = array( diff --git a/core/modules/menu_ui/src/Tests/MenuLanguageTest.php b/core/modules/menu_ui/src/Tests/MenuLanguageTest.php index 59b0458..4d38618 100644 --- a/core/modules/menu_ui/src/Tests/MenuLanguageTest.php +++ b/core/modules/menu_ui/src/Tests/MenuLanguageTest.php @@ -65,7 +65,7 @@ function testMenuLanguage() { $this->assertOptionSelected('edit-langcode', $edit['langcode'], 'The menu language was correctly selected.'); // Test menu link language. - $link_path = '/'; + $link_path = ''; // Add a menu link. $link_title = $this->randomString(); diff --git a/core/modules/menu_ui/src/Tests/MenuTest.php b/core/modules/menu_ui/src/Tests/MenuTest.php index 7d1b60b..f723b5c 100644 --- a/core/modules/menu_ui/src/Tests/MenuTest.php +++ b/core/modules/menu_ui/src/Tests/MenuTest.php @@ -105,8 +105,8 @@ function testMenu() { $this->verifyAccess(403); foreach ($this->items as $item) { - // Paths were set as 'node/$nid'. - $node = Node::load(str_replace('user-path:node/', '', $item->link->uri)); + // Menu link URIs are stored as 'user-path:/node/$nid'. + $node = Node::load(str_replace('user-path:/node/', '', $item->link->uri)); $this->verifyMenuLink($item, $node); } @@ -454,7 +454,7 @@ function doMenuTests() { $this->assertMenuLink($item7->getPluginId(), array('url' => 'http://drupal.org')); // Add menu item. - $item8 = $this->addMenuLink('', '/', $menu_name); + $item8 = $this->addMenuLink('', '', $menu_name); $this->assertMenuLink($item8->getPluginId(), array('route_name' => '')); $this->drupalGet(''); $this->assertResponse(200); @@ -503,8 +503,9 @@ function testMenuQueryAndFragment() { $this->drupalGet('admin/structure/menu/item/' . $item->id() . '/edit'); $this->assertFieldByName('link[0][uri]', $path, 'Path no longer has query or fragment.'); - // Use /#fragment and ensure that saving it does not loose its content. - $path = '/?arg1=value#fragment'; + // Use #fragment and ensure that saving it does not loose its + // content. + $path = '?arg1=value#fragment'; $item = $this->addMenuLink('', $path); $this->drupalGet('admin/structure/menu/item/' . $item->id() . '/edit'); @@ -560,7 +561,7 @@ function testUnpublishedNodeMenuItem() { public function testBlockContextualLinks() { $this->drupalLogin($this->drupalCreateUser(array('administer menu', 'access contextual links', 'administer blocks'))); $custom_menu = $this->addCustomMenu(); - $this->addMenuLink('', '/', $custom_menu->id()); + $this->addMenuLink('', '', $custom_menu->id()); $block = $this->drupalPlaceBlock('system_menu_block:' . $custom_menu->id(), array('label' => 'Custom menu', 'provider' => 'system')); $this->drupalGet('test-page'); @@ -602,7 +603,7 @@ public function testBlockContextualLinks() { * @return \Drupal\menu_link_content\Entity\MenuLinkContent * A menu link entity. */ - function addMenuLink($parent = '', $path = '/', $menu_name = 'tools', $expanded = FALSE, $weight = '0') { + function addMenuLink($parent = '', $path = '', $menu_name = 'tools', $expanded = FALSE, $weight = '0') { // View add menu link page. $this->drupalGet("admin/structure/menu/manage/$menu_name/add"); $this->assertResponse(200); diff --git a/core/modules/rdf/src/Tests/Field/LinkFieldRdfaTest.php b/core/modules/rdf/src/Tests/Field/LinkFieldRdfaTest.php index 0164b61..0c65d64 100644 --- a/core/modules/rdf/src/Tests/Field/LinkFieldRdfaTest.php +++ b/core/modules/rdf/src/Tests/Field/LinkFieldRdfaTest.php @@ -65,14 +65,14 @@ public function testAllFormattersExternal() { */ public function testAllFormattersInternal() { // Set up test values. - $this->testValue = 'admin'; + $this->testValue = '/admin'; $this->entity = entity_create('entity_test', array()); - $this->entity->{$this->fieldName}->uri = 'user-path:admin'; + $this->entity->{$this->fieldName}->uri = 'user-path:/admin'; // Set up the expected result. // AssertFormatterRdfa looks for a full path. $expected_rdf = array( - 'value' => $this->uri . '/' . $this->testValue, + 'value' => $this->uri . $this->testValue, 'type' => 'uri', ); diff --git a/core/modules/shortcut/src/Tests/ShortcutLinksTest.php b/core/modules/shortcut/src/Tests/ShortcutLinksTest.php index d91d602..945beb9 100644 --- a/core/modules/shortcut/src/Tests/ShortcutLinksTest.php +++ b/core/modules/shortcut/src/Tests/ShortcutLinksTest.php @@ -41,8 +41,7 @@ public function testShortcutLinkAdd() { // Create some paths to test. $test_cases = [ - '/', - '/admin', + '', 'admin', 'admin/config/system/site-information', 'node/' . $this->node->id() . '/edit', @@ -62,7 +61,7 @@ public function testShortcutLinkAdd() { $this->assertResponse(200); $saved_set = ShortcutSet::load($set->id()); $paths = $this->getShortcutInformation($saved_set, 'link'); - $this->assertTrue(in_array('user-path:/' . $test_path, $paths), 'Shortcut created: ' . $test_path); + $this->assertTrue(in_array('user-path:/' . ($test_path == '' ? '' : $test_path), $paths), 'Shortcut created: ' . $test_path); $this->assertLink($title, 0, String::format('Shortcut link %url found on the page.', ['%url' => $test_path])); } $saved_set = ShortcutSet::load($set->id()); @@ -86,7 +85,7 @@ public function testShortcutLinkAdd() { ]; $this->drupalPostForm('admin/config/user-interface/shortcut/manage/' . $set->id() . '/add-link', $form_data, t('Save')); $this->assertResponse(200); - $this->assertRaw(t("The path '@link_path' is either invalid or you do not have access to it.", ['@link_path' => '/admin'])); + $this->assertRaw(t("The path '@link_path' is either invalid or you do not have access to it.", ['@link_path' => 'admin'])); $form_data = [ 'title[0][value]' => $title, diff --git a/core/tests/Drupal/Tests/Core/Path/PathValidatorTest.php b/core/tests/Drupal/Tests/Core/Path/PathValidatorTest.php index b689dcc..3069b74 100644 --- a/core/tests/Drupal/Tests/Core/Path/PathValidatorTest.php +++ b/core/tests/Drupal/Tests/Core/Path/PathValidatorTest.php @@ -77,7 +77,19 @@ public function testIsValidWithFrontpage() { $this->accessAwareRouter->expects($this->never()) ->method('match'); - $this->assertTrue($this->pathValidator->isValid('/')); + $this->assertTrue($this->pathValidator->isValid('')); + } + + /** + * Tests the isValid() method for (used for jumplinks). + * + * @covers ::isValid + */ + public function testIsValidWithNone() { + $this->accessAwareRouter->expects($this->never()) + ->method('match'); + + $this->assertTrue($this->pathValidator->isValid('')); } /** @@ -323,7 +335,7 @@ public function testGetUrlIfValidWithoutAccess() { * Tests the getUrlIfValid() method with a front page + query + fragments. */ public function testGetUrlIfValidWithFrontPageAndQueryAndFragments() { - $url = $this->pathValidator->getUrlIfValid('/?hei=sen#berg'); + $url = $this->pathValidator->getUrlIfValid('?hei=sen#berg'); $this->assertEquals('', $url->getRouteName()); $this->assertEquals(['hei' => 'sen'], $url->getOptions()['query']); $this->assertEquals('berg', $url->getOptions()['fragment']); diff --git a/core/tests/Drupal/Tests/Core/UrlTest.php b/core/tests/Drupal/Tests/Core/UrlTest.php index e7922a2..4096e3d 100644 --- a/core/tests/Drupal/Tests/Core/UrlTest.php +++ b/core/tests/Drupal/Tests/Core/UrlTest.php @@ -602,8 +602,11 @@ public function testToUriStringForUserPath($uri, $options, $uri_string) { $url = Url::fromRoute('entity.test_entity.canonical', ['test_entity' => '1']); $this->pathValidator->expects($this->any()) ->method('getUrlIfValidWithoutAccessCheck') - ->with('test-entity/1') - ->willReturn($url); + ->willReturnMap([ + ['test-entity/1', $url], + ['', Url::fromRoute('')], + ['', Url::fromRoute('')], + ]); $url = Url::fromUri($uri, $options); $this->assertSame($url->toUriString(), $uri_string); } @@ -613,10 +616,23 @@ public function testToUriStringForUserPath($uri, $options, $uri_string) { */ public function providerTestToUriStringForUserPath() { return [ - ['user-path:test-entity/1', [], 'route:entity.test_entity.canonical;test_entity=1'], - ['user-path:test-entity/1', ['fragment' => 'top'], 'route:entity.test_entity.canonical;test_entity=1#top'], - ['user-path:test-entity/1', ['fragment' => 'top', 'query' => ['page' => '2']], 'route:entity.test_entity.canonical;test_entity=1?page=2#top'], - ['user-path:test-entity/1?page=2#top', [], 'route:entity.test_entity.canonical;test_entity=1?page=2#top'], + // The four permutations of a regular path. + ['user-path:/test-entity/1', [], 'route:entity.test_entity.canonical;test_entity=1'], + ['user-path:/test-entity/1', ['fragment' => 'top'], 'route:entity.test_entity.canonical;test_entity=1#top'], + ['user-path:/test-entity/1', ['fragment' => 'top', 'query' => ['page' => '2']], 'route:entity.test_entity.canonical;test_entity=1?page=2#top'], + ['user-path:/test-entity/1?page=2#top', [], 'route:entity.test_entity.canonical;test_entity=1?page=2#top'], + + // The four permutations of the special '' path. + ['user-path:/', [], 'route:'], + ['user-path:/', ['fragment' => 'top'], 'route:#top'], + ['user-path:/', ['fragment' => 'top', 'query' => ['page' => '2']], 'route:?page=2#top'], + ['user-path:/?page=2#top', [], 'route:?page=2#top'], + + // The four permutations of the special '' path. + ['user-path:', [], 'route:'], + ['user-path:', ['fragment' => 'top'], 'route:#top'], + ['user-path:', ['fragment' => 'top', 'query' => ['page' => '2']], 'route:?page=2#top'], + ['user-path:?page=2#top', [], 'route:?page=2#top'], ]; }