core/lib/Drupal/Core/Form/ConfirmFormHelper.php | 2 +- core/lib/Drupal/Core/Path/PathValidator.php | 3 ++ core/lib/Drupal/Core/Url.php | 39 ++++++++++++++++++++++ core/modules/field_ui/src/FieldUI.php | 2 +- .../src/Plugin/Field/FieldWidget/LinkWidget.php | 39 ++++++++++++++++++++++ core/modules/menu_ui/src/Tests/MenuTest.php | 6 ++-- .../rdf/src/Tests/Field/LinkFieldRdfaTest.php | 10 +++--- .../src/Controller/ShortcutSetController.php | 2 +- .../shortcut/src/Tests/ShortcutLinksTest.php | 4 +-- .../shortcut/src/Tests/ShortcutTestBase.php | 4 +-- .../form_test/src/Form/FormTestRedirectForm.php | 2 +- core/modules/views_ui/src/ViewListBuilder.php | 2 +- core/modules/views_ui/src/ViewUI.php | 2 +- core/profiles/standard/standard.install | 4 +-- .../Drupal/Tests/Core/Path/PathValidatorTest.php | 12 +++++++ core/tests/Drupal/Tests/Core/UrlTest.php | 32 +++++++++++++----- 16 files changed, 137 insertions(+), 28 deletions(-) diff --git a/core/lib/Drupal/Core/Form/ConfirmFormHelper.php b/core/lib/Drupal/Core/Form/ConfirmFormHelper.php index 6cbf234..8d608f1 100644 --- a/core/lib/Drupal/Core/Form/ConfirmFormHelper.php +++ b/core/lib/Drupal/Core/Form/ConfirmFormHelper.php @@ -34,7 +34,7 @@ public static function buildCancelLink(ConfirmFormInterface $form, Request $requ // If a destination is specified, that serves as the cancel link. if ($query->has('destination')) { $options = UrlHelper::parse($query->get('destination')); - $url = Url::fromUri('user-path:' . $options['path'], $options); + $url = Url::fromUri('user-path:/' . $options['path'], $options); } // Check for a route-based cancel link. else { diff --git a/core/lib/Drupal/Core/Path/PathValidator.php b/core/lib/Drupal/Core/Path/PathValidator.php index 51cdc2f..fcf0968 100644 --- a/core/lib/Drupal/Core/Path/PathValidator.php +++ b/core/lib/Drupal/Core/Path/PathValidator.php @@ -109,6 +109,9 @@ protected function getUrl($path, $access_check) { 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'])) { return FALSE; diff --git a/core/lib/Drupal/Core/Url.php b/core/lib/Drupal/Core/Url.php index 859cf07..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 @@ -328,6 +352,21 @@ protected static function fromEntityUri(array $uri_parts, array $options, $uri) * A new Url object for a 'user-path:' URI. */ protected static function fromUserPathUri(array $uri_parts, array $options) { + // Both PathValidator::getUrlIfValidWithoutAccessCheck() and 'base:' URIs + // 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). + 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/modules/field_ui/src/FieldUI.php b/core/modules/field_ui/src/FieldUI.php index dcac29a..9ee73c7 100644 --- a/core/modules/field_ui/src/FieldUI.php +++ b/core/modules/field_ui/src/FieldUI.php @@ -61,7 +61,7 @@ public static function getNextDestination(array $destinations) { $options['query']['destinations'] = $destinations; } // Redirect to any given path within the same domain. - $next_destination = Url::fromUri('user-path:' . $options['path']); + $next_destination = Url::fromUri('user-path:/' . $options['path']); } return $next_destination; } diff --git a/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php index e92fe53..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,6 +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=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; } } diff --git a/core/modules/menu_ui/src/Tests/MenuTest.php b/core/modules/menu_ui/src/Tests/MenuTest.php index fbcad78..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); } @@ -637,7 +637,7 @@ function addMenuLink($parent = '', $path = '', $menu_name = 'tools', $exp * Attempts to add menu link with invalid path or no access permission. */ function addInvalidMenuLink() { - foreach (array('-&-', 'admin/people/permissions', '#') as $link_path) { + foreach (array('-&-', 'admin/people/permissions') as $link_path) { $edit = array( 'link[0][uri]' => $link_path, 'title[0][value]' => 'title', diff --git a/core/modules/rdf/src/Tests/Field/LinkFieldRdfaTest.php b/core/modules/rdf/src/Tests/Field/LinkFieldRdfaTest.php index 4ed8d5c..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', ); @@ -84,9 +84,9 @@ public function testAllFormattersInternal() { */ public function testAllFormattersFront() { // Set up test values. - $this->testValue = ''; + $this->testValue = '/'; $this->entity = entity_create('entity_test', array()); - $this->entity->{$this->fieldName}->uri = 'user-path:'; + $this->entity->{$this->fieldName}->uri = 'user-path:/'; // Set up the expected result. $expected_rdf = array( diff --git a/core/modules/shortcut/src/Controller/ShortcutSetController.php b/core/modules/shortcut/src/Controller/ShortcutSetController.php index 54833d2..528b02d 100644 --- a/core/modules/shortcut/src/Controller/ShortcutSetController.php +++ b/core/modules/shortcut/src/Controller/ShortcutSetController.php @@ -65,7 +65,7 @@ public function addShortcutLinkInline(ShortcutSetInterface $shortcut_set, Reques 'title' => $name, 'shortcut_set' => $shortcut_set->id(), 'link' => array( - 'uri' => 'user-path:' . $link, + 'uri' => 'user-path:/' . $link, ), )); diff --git a/core/modules/shortcut/src/Tests/ShortcutLinksTest.php b/core/modules/shortcut/src/Tests/ShortcutLinksTest.php index b9c3ee5..945beb9 100644 --- a/core/modules/shortcut/src/Tests/ShortcutLinksTest.php +++ b/core/modules/shortcut/src/Tests/ShortcutLinksTest.php @@ -61,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()); @@ -157,7 +157,7 @@ public function testShortcutLinkChangePath() { $this->drupalPostForm('admin/config/user-interface/shortcut/link/' . $shortcut->id(), array('title[0][value]' => $shortcut->getTitle(), 'link[0][uri]' => $new_link_path), t('Save')); $saved_set = ShortcutSet::load($set->id()); $paths = $this->getShortcutInformation($saved_set, 'link'); - $this->assertTrue(in_array('user-path:' . $new_link_path, $paths), 'Shortcut path changed: ' . $new_link_path); + $this->assertTrue(in_array('user-path:/' . $new_link_path, $paths), 'Shortcut path changed: ' . $new_link_path); $this->assertLinkByHref($new_link_path, 0, 'Shortcut with new path appears on the page.'); } diff --git a/core/modules/shortcut/src/Tests/ShortcutTestBase.php b/core/modules/shortcut/src/Tests/ShortcutTestBase.php index 025826a..600176e 100644 --- a/core/modules/shortcut/src/Tests/ShortcutTestBase.php +++ b/core/modules/shortcut/src/Tests/ShortcutTestBase.php @@ -66,7 +66,7 @@ protected function setUp() { 'title' => t('Add content'), 'weight' => -20, 'link' => array( - 'uri' => 'user-path:node/add', + 'uri' => 'user-path:/node/add', ), )); $shortcut->save(); @@ -76,7 +76,7 @@ protected function setUp() { 'title' => t('All content'), 'weight' => -19, 'link' => array( - 'uri' => 'user-path:admin/content', + 'uri' => 'user-path:/admin/content', ), )); $shortcut->save(); diff --git a/core/modules/system/tests/modules/form_test/src/Form/FormTestRedirectForm.php b/core/modules/system/tests/modules/form_test/src/Form/FormTestRedirectForm.php index c99105a..c1b0a83 100644 --- a/core/modules/system/tests/modules/form_test/src/Form/FormTestRedirectForm.php +++ b/core/modules/system/tests/modules/form_test/src/Form/FormTestRedirectForm.php @@ -55,7 +55,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) { if (!$form_state->isValueEmpty('redirection')) { if (!$form_state->isValueEmpty('destination')) { // The destination is a random URL, so we can't use routed URLs. - $form_state->setRedirectUrl(Url::fromUri('user-path:' . $form_state->getValue('destination'))); + $form_state->setRedirectUrl(Url::fromUri('user-path:/' . $form_state->getValue('destination'))); } } else { diff --git a/core/modules/views_ui/src/ViewListBuilder.php b/core/modules/views_ui/src/ViewListBuilder.php index b4a53c6..9e094db 100644 --- a/core/modules/views_ui/src/ViewListBuilder.php +++ b/core/modules/views_ui/src/ViewListBuilder.php @@ -263,7 +263,7 @@ protected function getDisplayPaths(EntityInterface $view) { if ($display->hasPath()) { $path = $display->getPath(); if ($view->status() && strpos($path, '%') === FALSE) { - $all_paths[] = \Drupal::l('/' . $path, Url::fromUri('user-path:' . $path)); + $all_paths[] = \Drupal::l('/' . $path, Url::fromUri('user-path:/' . $path)); } else { $all_paths[] = String::checkPlain('/' . $path); diff --git a/core/modules/views_ui/src/ViewUI.php b/core/modules/views_ui/src/ViewUI.php index 7f0d8bd..08e887c 100644 --- a/core/modules/views_ui/src/ViewUI.php +++ b/core/modules/views_ui/src/ViewUI.php @@ -723,7 +723,7 @@ public function renderPreview($display_id, $args = array()) { Xss::filterAdmin($this->executable->getTitle()), ); if (isset($path)) { - $path = \Drupal::l($path, Url::fromUri('user-path:' . $path)); + $path = \Drupal::l($path, Url::fromUri('user-path:/' . $path)); } else { $path = t('This display has no path.'); diff --git a/core/profiles/standard/standard.install b/core/profiles/standard/standard.install index 86dd057..f9466ff 100644 --- a/core/profiles/standard/standard.install +++ b/core/profiles/standard/standard.install @@ -58,7 +58,7 @@ function standard_install() { 'shortcut_set' => 'default', 'title' => t('Add content'), 'weight' => -20, - 'link' => array('uri' => 'user-path:node/add'), + 'link' => array('uri' => 'user-path:/node/add'), )); $shortcut->save(); @@ -66,7 +66,7 @@ function standard_install() { 'shortcut_set' => 'default', 'title' => t('All content'), 'weight' => -19, - 'link' => array('uri' => 'user-path:admin/content'), + 'link' => array('uri' => 'user-path:/admin/content'), )); $shortcut->save(); diff --git a/core/tests/Drupal/Tests/Core/Path/PathValidatorTest.php b/core/tests/Drupal/Tests/Core/Path/PathValidatorTest.php index 0e2471c..3069b74 100644 --- a/core/tests/Drupal/Tests/Core/Path/PathValidatorTest.php +++ b/core/tests/Drupal/Tests/Core/Path/PathValidatorTest.php @@ -81,6 +81,18 @@ public function testIsValidWithFrontpage() { } /** + * 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('')); + } + + /** * Tests the isValid() method for an external URL. * * @covers ::isValid diff --git a/core/tests/Drupal/Tests/Core/UrlTest.php b/core/tests/Drupal/Tests/Core/UrlTest.php index 3f0ec3e..4096e3d 100644 --- a/core/tests/Drupal/Tests/Core/UrlTest.php +++ b/core/tests/Drupal/Tests/Core/UrlTest.php @@ -180,7 +180,7 @@ public function testFromRoutedPathWithInvalidRoute() { ->method('getUrlIfValidWithoutAccessCheck') ->with('invalid-path') ->willReturn(FALSE); - $url = Url::fromUri('user-path:invalid-path'); + $url = Url::fromUri('user-path:/invalid-path'); $this->assertSame(FALSE, $url->isRouted()); $this->assertSame('base:invalid-path', $url->getUri()); } @@ -196,7 +196,7 @@ public function testFromRoutedPathWithValidRoute() { ->method('getUrlIfValidWithoutAccessCheck') ->with('valid-path') ->willReturn($url); - $result_url = Url::fromUri('user-path:valid-path'); + $result_url = Url::fromUri('user-path:/valid-path'); $this->assertSame($url, $result_url); } @@ -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'], ]; }