Issue #2418181 by dawehner, Wim Leers, effulgentsia: Remove 404 validation from link creation

core/modules/link/src/Plugin/Field/FieldType/LinkItem.php

@@ -27,7 +27,7 @@
  *   description = @Translation("Stores a URL string, optional varchar link text, and optional blob of attributes to assemble a link."),
  *   default_widget = "link_default",
  *   default_formatter = "link",
- *   constraints = {"LinkType" = {}, "LinkAccess" = {}}
+ *   constraints = {"LinkType" = {}, "LinkAccess" = {}, "LinkExternalProtocols" = {}, "LinkNotExistingInternal" = {}}
  * )
  */
 class LinkItem extends FieldItemBase implements LinkItemInterface {

core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php

@@ -147,31 +147,6 @@ public static function validateUriElement($element, FormStateInterface $form_sta
       $form_state->setError($element, t('Manually entered paths should start with /, ? or #.'));
       return;
     }
-
-    // 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 external URLs using untrusted protocols.
-      $disallowed = $disallowed || ($url->isExternal() && !in_array(parse_url($uri, PHP_URL_SCHEME), UrlHelper::getAllowedProtocols()));
-      // Disallow routed URLs that don't exist.
-      if (!$disallowed && $url->isRouted()) {
-        try {
-          $url->toString();
-        }
-        catch (RouteNotFoundException $e) {
-          $disallowed = TRUE;
-        }
-      }
-
-      if ($disallowed) {
-        $form_state->setError($element, t("The path '@link_path' is invalid.", ['@link_path' => static::getUriAsDisplayableString($uri)]));
-      }
-    }
   }
 
   /**

core/modules/link/src/Plugin/Validation/Constraint/LinkExternalProtocolsConstraint.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\link\Plugin\Validation\Constraint\LinkExternalProtocols.
+ */
+
+namespace Drupal\link\Plugin\Validation\Constraint;
+
+use Symfony\Component\Validator\Constraint;
+
+/**
+ * Defines a protocol validation constraint for links to external URLs.
+ *
+ * @Plugin(
+ *   id = "LinkExternalProtocols",
+ *   label = @Translation("No dangerous external protocols", context = "Validation"),
+ * )
+ */
+class LinkExternalProtocolsConstraint extends Constraint {
+
+  public $message = "The path '@uri' is invalid.";
+
+}

core/modules/link/src/Plugin/Validation/Constraint/LinkExternalProtocolsConstraintValidator.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\link\Plugin\Validation\Constraint\LinkExternalProtocolsValidator.
+ */
+
+namespace Drupal\link\Plugin\Validation\Constraint;
+
+use Drupal\Component\Utility\UrlHelper;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidatorInterface;
+use Symfony\Component\Validator\ExecutionContextInterface;
+
+/**
+ * Validates the LinkExternalProtocols constraint.
+ */
+class LinkExternalProtocolsConstraintValidator implements ConstraintValidatorInterface {
+
+  /**
+   * Stores the validator's state during validation.
+   *
+   * @var \Symfony\Component\Validator\ExecutionContextInterface
+   */
+  protected $context;
+
+  /**
+   * {@inheritdoc}
+   */
+  public function initialize(ExecutionContextInterface $context) {
+    $this->context = $context;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function validate($value, Constraint $constraint) {
+    if (isset($value)) {
+      try {
+        /** @var \Drupal\Core\Url $url */
+        $url = $value->getUrl();
+      }
+      // If the URL is malformed this constraint cannot check further.
+      catch (\InvalidArgumentException $e) {
+        return;
+      }
+      // Disallow external URLs using untrusted protocols.
+      if ($url->isExternal() && !in_array(parse_url($url->getUri(), PHP_URL_SCHEME), UrlHelper::getAllowedProtocols())) {
+        $this->context->addViolation($constraint->message, array('@uri' => $value->uri));
+      }
+    }
+  }
+
+}

core/modules/link/src/Plugin/Validation/Constraint/LinkNotExistingInternalConstraint.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\link\Plugin\Validation\Constraint\LinkNotExistingInternal.
+ */
+
+namespace Drupal\link\Plugin\Validation\Constraint;
+
+use Symfony\Component\Validator\Constraint;
+
+/**
+ * Defines a protocol validation constraint for links to broken internal URLs.
+ *
+ * @Plugin(
+ *   id = "LinkNotExistingInternal",
+ *   label = @Translation("No broken internal links", context = "Validation"),
+ * )
+ */
+class LinkNotExistingInternalConstraint extends Constraint {
+
+  public $message = "The path '@uri' is invalid.";
+
+}

core/modules/link/src/Plugin/Validation/Constraint/LinkNotExistingInternalConstraintValidator.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\link\Plugin\Validation\Constraint\LinkNotExistingInternalConstraintValidator.
+ */
+
+namespace Drupal\link\Plugin\Validation\Constraint;
+
+use Symfony\Component\Routing\Exception\RouteNotFoundException;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidatorInterface;
+use Symfony\Component\Validator\ExecutionContextInterface;
+
+/**
+ * Validates the LinkNotExistingInternal constraint.
+ */
+class LinkNotExistingInternalConstraintValidator implements ConstraintValidatorInterface {
+
+  /**
+   * Stores the validator's state during validation.
+   *
+   * @var \Symfony\Component\Validator\ExecutionContextInterface
+   */
+  protected $context;
+
+  /**
+   * {@inheritdoc}
+   */
+  public function initialize(ExecutionContextInterface $context) {
+    $this->context = $context;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function validate($value, Constraint $constraint) {
+    if (isset($value)) {
+      try {
+        /** @var \Drupal\Core\Url $url */
+        $url = $value->getUrl();
+      }
+      // If the URL is malformed this constraint cannot check further.
+      catch (\InvalidArgumentException $e) {
+        return;
+      }
+
+      if ($url->isRouted()) {
+        $allowed = TRUE;
+        try {
+          $url->toString();
+        }
+        catch (RouteNotFoundException $e) {
+          $allowed = FALSE;
+        }
+        if (!$allowed) {
+          $this->context->addViolation($constraint->message, array('@uri' => $value->uri));
+        }
+      }
+    }
+  }
+
+}

core/modules/link/src/Tests/LinkFieldTest.php

@@ -136,16 +136,14 @@ function testURLValidation() {
     // Define some invalid URLs.
     $validation_error_1 = "The path '@link_path' is invalid.";
     $validation_error_2 = 'Manually entered paths should start with /, ? or #.';
+    $validation_error_3 = "The path '@link_path' is inaccessible.";
     $invalid_external_entries = array(
-      // Missing protcol
-      'not-an-url' => $validation_error_2,
       // Invalid protocol
       'invalid://not-a-valid-protocol' => $validation_error_1,
       // Missing host name
       'http://' => $validation_error_1,
     );
     $invalid_internal_entries = array(
-      '/non/existing/path' => $validation_error_1,
       'no-leading-slash' => $validation_error_2,
       'entity:non_existing_entity_type/yar' => $validation_error_1,
     );
@@ -165,6 +163,14 @@ function testURLValidation() {
     $this->field->save();
     $this->assertValidEntries($field_name, $valid_internal_entries);
     $this->assertInvalidEntries($field_name, $valid_external_entries + $invalid_internal_entries);
+
+    // Ensure that users with 'link to any page', don't apply access checking.
+    $this->drupalLogin($this->drupalCreateUser([
+      'view test entity',
+      'administer entity_test content',
+    ]));
+    $this->assertValidEntries($field_name, ['/entity_test/add' => '/entity_test/add']);
+    $this->assertInValidEntries($field_name, ['/admin' => $validation_error_3]);
   }
 
   /**

core/modules/link/tests/src/LinkAccessConstraintValidatorTest.php → core/modules/link/tests/src/Plugin/Validation/Constraint/LinkAccessConstraintValidatorTest.php

@@ -2,10 +2,10 @@
 
 /**
  * @file
- * Contains \Drupal\Tests\link\LinkAccessConstraintValidatorTest.
+ * Contains \Drupal\Tests\link\Plugin\Validation\Constraint\LinkAccessConstraintValidatorTest.
  */
 
-namespace Drupal\Tests\link;
+namespace Drupal\Tests\link\Plugin\Validation\Constraint;
 
 use Drupal\Core\Url;
 use Drupal\link\Plugin\Validation\Constraint\LinkAccessConstraint;

core/modules/link/tests/src/Plugin/Validation/Constraint/LinkExternalProtocolsConstraintValidatorTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\link\Plugin\Validation\Constraint\LinkExternalProtocolsConstraintValidatorTest.
+ */
+
+namespace Drupal\Tests\link\Plugin\Validation\Constraint;
+
+use Drupal\Component\Utility\UrlHelper;
+use Drupal\Core\Url;
+use Drupal\link\Plugin\Validation\Constraint\LinkExternalProtocolsConstraint;
+use Drupal\link\Plugin\Validation\Constraint\LinkExternalProtocolsConstraintValidator;
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * @coversDefaultClass \Drupal\link\Plugin\Validation\Constraint\LinkExternalProtocolsConstraintValidator
+ * @group Link
+ */
+class LinkExternalProtocolsConstraintValidatorTest extends UnitTestCase {
+
+  /**
+   * @covers ::validate
+   * @dataProvider providerValidate
+   */
+  public function testValidate($value, $valid) {
+    $context = $this->getMock('Symfony\Component\Validator\ExecutionContextInterface');
+
+    if ($valid) {
+      $context->expects($this->never())
+        ->method('addViolation');
+    }
+    else {
+      $context->expects($this->once())
+        ->method('addViolation');
+    }
+
+    // Setup some more allowed protocols.
+    UrlHelper::setAllowedProtocols(['http', 'https', 'magnet']);
+
+    $constraint = new LinkExternalProtocolsConstraint();
+
+    $validator = new LinkExternalProtocolsConstraintValidator();
+    $validator->initialize($context);
+    $validator->validate($value, $constraint);
+  }
+
+  /**
+   * Data provider for ::testValidate
+   */
+  public function providerValidate() {
+    $data = [];
+
+    // Test allowed protocols.
+    $data[] = ['http://www.drupal.org', TRUE];
+    $data[] = ['https://www.drupal.org', TRUE];
+    $data[] = ['magnet:?xt=urn:sha1:YNCKHTQCWBTRNJIV4WNAE52SJUQCZO5C', TRUE];
+
+    // Invalid protocols.
+    $data[] = ['ftp://ftp.funet.fi/pub/standards/RFC/rfc959.txt', FALSE];
+
+    foreach ($data as &$single_data) {
+      $url = Url::fromUri($single_data[0]);
+      $link = $this->getMock('Drupal\link\LinkItemInterface');
+      $link->expects($this->any())
+        ->method('getUrl')
+        ->willReturn($url);
+      $single_data[0] = $link;
+    }
+
+    return $data;
+  }
+
+  /**
+   * @covers ::validate
+   *
+   * @see \Drupal\Core\Url::fromUri
+   */
+  public function testValidateWithMalformedUri() {
+    $link = $this->getMock('Drupal\link\LinkItemInterface');
+    $link->expects($this->any())
+      ->method('getUrl')
+      ->willThrowException(new \InvalidArgumentException());
+
+    $context = $this->getMock('Symfony\Component\Validator\ExecutionContextInterface');
+    $context->expects($this->never())
+      ->method('addViolation');
+
+    $constraint = new LinkExternalProtocolsConstraint();
+
+    $validator = new LinkExternalProtocolsConstraintValidator();
+    $validator->initialize($context);
+    $validator->validate($link, $constraint);
+  }
+
+  /**
+   * @covers ::validate
+   */
+  public function testValidateIgnoresInternalUrls() {
+    $link = $this->getMock('Drupal\link\LinkItemInterface');
+    $link->expects($this->any())
+      ->method('getUrl')
+      ->willReturn(Url::fromRoute('example.test'));
+
+    $context = $this->getMock('Symfony\Component\Validator\ExecutionContextInterface');
+    $context->expects($this->never())
+      ->method('addViolation');
+
+    $constraint = new LinkExternalProtocolsConstraint();
+
+    $validator = new LinkExternalProtocolsConstraintValidator();
+    $validator->initialize($context);
+    $validator->validate($link, $constraint);
+  }
+
+}

core/modules/link/tests/src/Plugin/Validation/Constraint/LinkNotExistingInternalConstraintValidatorTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\link\Plugin\Validation\Constraint\LinkNotExistingInternalConstraintValidatorTest.
+ */
+
+namespace Drupal\Tests\link\Plugin\Validation\Constraint;
+
+use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\Core\Url;
+use Drupal\link\Plugin\Validation\Constraint\LinkNotExistingInternalConstraint;
+use Drupal\link\Plugin\Validation\Constraint\LinkNotExistingInternalConstraintValidator;
+use Drupal\Tests\UnitTestCase;
+use Symfony\Component\Routing\Exception\RouteNotFoundException;
+
+/**
+ * @coversDefaultClass \Drupal\link\Plugin\Validation\Constraint\LinkNotExistingInternalConstraintValidator
+ * @group Link
+ */
+class LinkNotExistingInternalConstraintValidatorTest extends UnitTestCase {
+
+  /**
+   * @covers ::validate
+   * @dataProvider providerValidate
+   */
+  public function testValidate($value, $valid) {
+    $context = $this->getMock('Symfony\Component\Validator\ExecutionContextInterface');
+
+    if ($valid) {
+      $context->expects($this->never())
+        ->method('addViolation');
+    }
+    else {
+      $context->expects($this->once())
+        ->method('addViolation');
+    }
+
+
+    $constraint = new LinkNotExistingInternalConstraint();
+
+    $validator = new LinkNotExistingInternalConstraintValidator();
+    $validator->initialize($context);
+    $validator->validate($value, $constraint);
+  }
+
+  /**
+   * Data provider for ::testValidate
+   */
+  public function providerValidate() {
+    $data = [];
+
+    // External URL
+    $data[] = [Url::fromUri('https://www.drupal.org'), TRUE];
+
+    // Existing routed URL.
+    $url = Url::fromRoute('example.existing_route');
+
+    $url_generator = $this->getMock('Drupal\Core\Routing\UrlGeneratorInterface');
+    $url_generator->expects($this->any())
+      ->method('generateFromRoute')
+      ->with('example.existing_route', [], [])
+      ->willReturn('/example/existing');
+    $url->setUrlGenerator($url_generator);
+
+    $data[] = [$url, TRUE];
+
+    // Not existing routed URL.
+    $url = Url::fromRoute('example.not_existing_route');
+
+    $url_generator = $this->getMock('Drupal\Core\Routing\UrlGeneratorInterface');
+    $url_generator->expects($this->any())
+      ->method('generateFromRoute')
+      ->with('example.not_existing_route', [], [])
+      ->willThrowException(new RouteNotFoundException());
+    $url->setUrlGenerator($url_generator);
+
+    $data[] = [$url, FALSE];
+
+    foreach ($data as &$single_data) {
+      $link = $this->getMock('Drupal\link\LinkItemInterface');
+      $link->expects($this->any())
+        ->method('getUrl')
+        ->willReturn($single_data[0]);
+
+      $single_data[0] = $link;
+    }
+
+    return $data;
+  }
+
+  /**
+   * @covers ::validate
+   *
+   * @see \Drupal\Core\Url::fromUri
+   */
+  public function testValidateWithMalformedUri() {
+    $link = $this->getMock('Drupal\link\LinkItemInterface');
+    $link->expects($this->any())
+      ->method('getUrl')
+      ->willThrowException(new \InvalidArgumentException());
+
+    $context = $this->getMock('Symfony\Component\Validator\ExecutionContextInterface');
+    $context->expects($this->never())
+      ->method('addViolation');
+
+    $constraint = new LinkNotExistingInternalConstraint();
+
+    $validator = new LinkNotExistingInternalConstraintValidator();
+    $validator->initialize($context);
+    $validator->validate($link, $constraint);
+  }
+
+}

core/modules/menu_ui/src/Tests/MenuTest.php

@@ -639,18 +639,13 @@ function addMenuLink($parent = '', $path = '/', $menu_name = 'tools', $expanded
    * Attempts to add menu link with invalid path or no access permission.
    */
   function addInvalidMenuLink() {
-    foreach (array('valid' => '/-&-', 'access' => '/admin/people/permissions') as $type => $link_path) {
+    foreach (array('access' => '/admin/people/permissions') as $type => $link_path) {
       $edit = array(
         'link[0][uri]' => $link_path,
         'title[0][value]' => 'title',
       );
       $this->drupalPostForm("admin/structure/menu/manage/{$this->menu->id()}/add", $edit, t('Save'));
-      if ($type === 'access') {
-        $this->assertRaw(t("The path '@link_path' is inaccessible.", array('@link_path' => $link_path)), 'Menu link was not created');
-      }
-      else {
-        $this->assertRaw(t("The path '@link_path' is invalid.", array('@link_path' => $link_path)), 'Menu link was not created');
-      }
+      $this->assertRaw(t("The path '@link_path' is inaccessible.", array('@link_path' => $link_path)), 'Menu link was not created');
     }
   }