.idea/php.xml | 5 ++ .../PrimitiveTypeConstraintValidator.php | 7 +- core/modules/link/src/LinkItemInterface.php | 6 +- .../Plugin/Field/FieldFormatter/LinkFormatter.php | 5 +- .../link/src/Plugin/Field/FieldType/LinkItem.php | 28 ++++--- .../src/Plugin/Field/FieldWidget/LinkWidget.php | 79 ++++++++++++++++---- .../Validation/Constraint/LinkTypeConstraint.php | 18 ++--- .../src/Controller/ShortcutSetController.php | 4 +- .../shortcut/src/Tests/ShortcutLinksTest.php | 5 +- .../shortcut/src/Tests/ShortcutTestBase.php | 4 +- core/profiles/standard/standard.install | 4 +- .../PrimitiveTypeConstraintValidatorTest.php | 86 ++++++++++++++++++++++ 12 files changed, 201 insertions(+), 50 deletions(-) diff --git a/.idea/php.xml b/.idea/php.xml new file mode 100644 index 0000000..ff077ae --- /dev/null +++ b/.idea/php.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/PrimitiveTypeConstraintValidator.php b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/PrimitiveTypeConstraintValidator.php index fe3a1fb..d1bdc0f 100644 --- a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/PrimitiveTypeConstraintValidator.php +++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/PrimitiveTypeConstraintValidator.php @@ -49,7 +49,12 @@ public function validate($value, Constraint $constraint) { if ($typed_data instanceof StringInterface && !is_scalar($value)) { $valid = FALSE; } - if ($typed_data instanceof UriInterface && filter_var($value, FILTER_VALIDATE_URL) === FALSE) { + // Ensure that URIs comply with http://tools.ietf.org/html/rfc3986, which + // requires: + // - That it is well formed (parse_url() returns FALSE if not). + // - That it contains a scheme (parse_url(, PHP_URL_SCHEME) returns NULL if + // not). + if ($typed_data instanceof UriInterface && in_array(parse_url($value, PHP_URL_SCHEME), [NULL, FALSE], TRUE)) { $valid = FALSE; } // @todo: Move those to separate constraint validators. diff --git a/core/modules/link/src/LinkItemInterface.php b/core/modules/link/src/LinkItemInterface.php index b78a05f..cd3bce2 100644 --- a/core/modules/link/src/LinkItemInterface.php +++ b/core/modules/link/src/LinkItemInterface.php @@ -40,7 +40,11 @@ public function isExternal(); /** * Gets the URL object. * - * @return \Drupal\Core\Url + * @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. */ public function getUrl(); diff --git a/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php b/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php index 8819fe3..bbe282c 100644 --- a/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php +++ b/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php @@ -9,6 +9,7 @@ use Drupal\Component\Utility\String; use Drupal\Component\Utility\Unicode; +use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Field\FieldItemListInterface; use Drupal\Core\Field\FormatterBase; @@ -241,9 +242,7 @@ public function viewElements(FieldItemListInterface $items) { * An Url object. */ protected function buildUrl(LinkItemInterface $item) { - // @todo Consider updating the usage of the path validator with whatever - // gets added in https://www.drupal.org/node/2405551. - $url = $this->pathValidator->getUrlIfValidWithoutAccessCheck($item->uri) ?: Url::fromRoute(''); + $url = $item->getUrl() ?: Url::fromRoute(''); $settings = $this->getSettings(); $options = $item->options; diff --git a/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php b/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php index d5c1fec..8af3c46 100644 --- a/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php +++ b/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php @@ -8,6 +8,7 @@ namespace Drupal\link\Plugin\Field\FieldType; use Drupal\Component\Utility\Random; +use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Field\FieldItemBase; use Drupal\Core\Field\FieldStorageDefinitionInterface; @@ -44,9 +45,7 @@ public static function defaultFieldSettings() { * {@inheritdoc} */ public static function propertyDefinitions(FieldStorageDefinitionInterface $field_definition) { - // @todo Change the type from 'string' to 'uri': - // https://www.drupal.org/node/2412509. - $properties['uri'] = DataDefinition::create('string') + $properties['uri'] = DataDefinition::create('uri') ->setLabel(t('URI')); $properties['title'] = DataDefinition::create('string') @@ -153,9 +152,7 @@ public function isEmpty() { * {@inheritdoc} */ public function isExternal() { - // External links don't resolve to a route. - $url = \Drupal::pathValidator()->getUrlIfValid($this->uri); - return $url->isExternal(); + return $this->getUrl()->isExternal(); } /** @@ -166,15 +163,24 @@ public static function mainPropertyName() { } /** - * Gets the URL object. + * {@inheritdoc} * - * @return \Drupal\Core\Url + * @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() { - return \Drupal::pathValidator()->getUrlIfValidWithoutAccessCheck($this->uri); + 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); } - /** * {@inheritdoc} */ diff --git a/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php index 7768efe..3230a41 100644 --- a/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php +++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php @@ -8,10 +8,14 @@ namespace Drupal\link\Plugin\Field\FieldWidget; use Drupal\Component\Utility\Unicode; +use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Field\FieldItemListInterface; use Drupal\Core\Field\WidgetBase; use Drupal\Core\Form\FormStateInterface; use Drupal\link\LinkItemInterface; +use Symfony\Component\Validator\ConstraintViolation; +use Symfony\Component\Validator\ConstraintViolationInterface; +use Symfony\Component\Validator\ConstraintViolationListInterface; /** * Plugin implementation of the 'link' widget. @@ -37,23 +41,41 @@ public static function defaultSettings() { } /** + * Gets the URI without the 'user-path:' scheme, for display while editing. + * + * @param string $uri + * The URI to get the displayable string for. + * + * @return string + */ + protected function getUriAsDisplayableString($uri) { + $scheme = parse_url($uri, PHP_URL_SCHEME); + if ($scheme === 'user-path') { + $uri_reference = explode(':', $uri, 2)[1]; + } + else { + $uri_reference = $uri; + } + return $uri_reference; + } + + /** * {@inheritdoc} */ public function formElement(FieldItemListInterface $items, $delta, array $element, array &$form, FormStateInterface $form_state) { + /** @var \Drupal\link\LinkItemInterface $item */ + $item = $items[$delta]; - $default_url_value = NULL; - if (isset($items[$delta]->uri)) { - if ($url = \Drupal::pathValidator()->getUrlIfValid($items[$delta]->uri)) { - $url->setOptions($items[$delta]->options ?: []); - $url_string = $url->toString(); - $default_url_value = $url->isRouted() ? Unicode::substr($url_string, strlen(base_path())) : $url_string; - } - } $element['uri'] = array( '#type' => 'url', '#title' => $this->t('URL'), '#placeholder' => $this->getSetting('placeholder_url'), - '#default_value' => $default_url_value, + // 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, '#maxlength' => 2048, '#required' => $element['#required'], ); @@ -202,19 +224,44 @@ public function validateTitle(&$element, FormStateInterface $form_state, $form) public function massageFormValues(array $values, array $form, FormStateInterface $form_state) { foreach ($values as &$value) { if (!empty($value['uri'])) { - $url = \Drupal::pathValidator()->getUrlIfValid($value['uri']); - if (!$url) { - return $values; + // 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' => []]; - // Reset the URL value to contain only the path. - if (!$url->isExternal() && $this->supportsInternalLinks()) { - $value['uri'] = substr($url->toString(), strlen(\Drupal::request()->getBasePath() . '/')); - } } } return $values; } + + /** + * {@inheritdoc} + * + * Override the '%url' 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) { + /** @var \Symfony\Component\Validator\ConstraintViolationInterface $violation */ + foreach ($violations as $offset => $violation) { + $parameters = $violation->getMessageParameters(); + if (isset($parameters['%url'])) { + $parameters['%url'] = $this->getUriAsDisplayableString($parameters['%url']); + $violations->set($offset, new ConstraintViolation( + $this->t($violation->getMessageTemplate(), $parameters), + $violation->getMessageTemplate(), + $parameters, + $violation->getRoot(), + $violation->getPropertyPath(), + $violation->getInvalidValue(), + $violation->getMessagePluralization(), + $violation->getCode() + )); + } + } + parent::flagErrors($items, $violations, $form, $form_state); + } + } diff --git a/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php b/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php index 8ae29b2..22b1942 100644 --- a/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php +++ b/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php @@ -52,19 +52,19 @@ public function validate($value, Constraint $constraint) { /** @var $link_item \Drupal\link\LinkItemInterface */ $link_item = $value; $link_type = $link_item->getFieldDefinition()->getSetting('link_type'); - $url_string = $link_item->uri; - // Validate the url property. - if ($url_string !== '') { - if ($url = \Drupal::pathValidator()->getUrlIfValid($url_string)) { - $url_is_valid = (bool) $url; + $url = $link_item->getUrl(TRUE); - if ($url->isExternal() && !($link_type & LinkItemInterface::LINK_EXTERNAL)) { - $url_is_valid = FALSE; - } + if ($url) { + $url_is_valid = TRUE; + if ($url->isExternal() && !($link_type & LinkItemInterface::LINK_EXTERNAL)) { + $url_is_valid = FALSE; + } + if (!$url->isExternal() && !($link_type & LinkItemInterface::LINK_INTERNAL)) { + $url_is_valid = FALSE; } } if (!$url_is_valid) { - $this->context->addViolation($this->message, array('%url' => $url_string)); + $this->context->addViolation($this->message, array('%url' => $link_item->uri)); } } } diff --git a/core/modules/shortcut/src/Controller/ShortcutSetController.php b/core/modules/shortcut/src/Controller/ShortcutSetController.php index 4489964..54833d2 100644 --- a/core/modules/shortcut/src/Controller/ShortcutSetController.php +++ b/core/modules/shortcut/src/Controller/ShortcutSetController.php @@ -60,12 +60,12 @@ public static function create(ContainerInterface $container) { public function addShortcutLinkInline(ShortcutSetInterface $shortcut_set, Request $request) { $link = $request->query->get('link'); $name = $request->query->get('name'); - if ($this->pathValidator->isValid($link)) { + if (parse_url($link, PHP_URL_SCHEME) === NULL && $this->pathValidator->isValid($link)) { $shortcut = $this->entityManager()->getStorage('shortcut')->create(array( 'title' => $name, 'shortcut_set' => $shortcut_set->id(), 'link' => array( - 'uri' => $link, + 'uri' => 'user-path:' . $link, ), )); diff --git a/core/modules/shortcut/src/Tests/ShortcutLinksTest.php b/core/modules/shortcut/src/Tests/ShortcutLinksTest.php index 27202ba..6f6b4c7 100644 --- a/core/modules/shortcut/src/Tests/ShortcutLinksTest.php +++ b/core/modules/shortcut/src/Tests/ShortcutLinksTest.php @@ -61,8 +61,7 @@ public function testShortcutLinkAdd() { $this->assertResponse(200); $saved_set = ShortcutSet::load($set->id()); $paths = $this->getShortcutInformation($saved_set, 'link'); - $test_path = $test_path != '' ? $test_path : ''; - $this->assertTrue(in_array($test_path, $paths), 'Shortcut created: ' . $test_path); + $this->assertTrue(in_array('user-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()); @@ -158,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($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 99a092b..025826a 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' => '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' => 'admin/content', + 'uri' => 'user-path:admin/content', ), )); $shortcut->save(); diff --git a/core/profiles/standard/standard.install b/core/profiles/standard/standard.install index 085b561..86dd057 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' => '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' => 'admin/content'), + 'link' => array('uri' => 'user-path:admin/content'), )); $shortcut->save(); diff --git a/core/tests/Drupal/Tests/Core/Validation/Plugin/Validation/Constraint/PrimitiveTypeConstraintValidatorTest.php b/core/tests/Drupal/Tests/Core/Validation/Plugin/Validation/Constraint/PrimitiveTypeConstraintValidatorTest.php new file mode 100644 index 0000000..af34394 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Validation/Plugin/Validation/Constraint/PrimitiveTypeConstraintValidatorTest.php @@ -0,0 +1,86 @@ +getMockBuilder('Drupal\Core\TypedData\Validation\Metadata') + ->disableOriginalConstructor() + ->getMock(); + $metadata->expects($this->any()) + ->method('getTypedData') + ->willReturn($typed_data); + + $context = $this->getMock('Symfony\Component\Validator\ExecutionContextInterface'); + $context->expects($this->any()) + ->method('getMetadata') + ->willReturn($metadata); + + if ($valid) { + $context->expects($this->never()) + ->method('addViolation'); + } + else { + $context->expects($this->once()) + ->method('addViolation'); + } + + $constraint = new PrimitiveTypeConstraint(); + + $validate = new PrimitiveTypeConstraintValidator(); + $validate->initialize($context); + $validate->validate($value, $constraint); + } + + public function provideTestValidate() { + $data = []; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\BooleanInterface'), NULL, TRUE]; + + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\BooleanInterface'), 1, TRUE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\BooleanInterface'), 'test', FALSE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\FloatInterface'), 1.5, TRUE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\FloatInterface'), 'test', FALSE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\IntegerInterface'), 1, TRUE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\IntegerInterface'), 1.5, FALSE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\IntegerInterface'), 'test', FALSE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\StringInterface'), 'test', TRUE]; + // It is odd that 1 is a valid string. + // $data[] = [$this->getMock('Drupal\Core\TypedData\Type\StringInterface'), 1, FALSE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\StringInterface'), [], FALSE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\UriInterface'), 'http://www.drupal.org', TRUE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\UriInterface'), 'https://www.drupal.org', TRUE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\UriInterface'), 'Invalid', FALSE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\UriInterface'), 'entity:node/1', TRUE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\UriInterface'), 'base://', FALSE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\UriInterface'), 'base://node', TRUE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\UriInterface'), 'user-path:', TRUE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\UriInterface'), 'public://', FALSE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\UriInterface'), 'public://foo.png', TRUE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\UriInterface'), 'private://', FALSE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\UriInterface'), 'private://foo.png', TRUE]; + $data[] = [$this->getMock('Drupal\Core\TypedData\Type\UriInterface'), 'drupal.org', FALSE]; + + return $data; + } + +}