Add a Url::fromUserInput() wrapper method for generating URLs from user-entered paths

Updating the @todos and moving some documentation from the protected method.

+++ b/core/tests/Drupal/Tests/Core/UrlTest.php
@@ -171,6 +171,79 @@ public function testFromRouteFront() {
+  public function testFromUserInput($path) {
+    $url = Url::fromUserInput($path);
+
+    $this->assertInstanceOf('Drupal\Core\Url', $url);
+  }

Is that really the only thing we can assert? For example fragment/options extracting could be tested.

Good idea; I'll look into that too.

Sorry @xjm, the 2/18 deadline on the parent is looming. Few additional assertions.

Assert all the things! Not sure why ::assetEmpty() didn't work here.

I checked through the patch and the code and found all invocations to fromUri are changed to fromUserInput wherever user-path was used. I think everything else looks good too. All the todos should be handled in a follow-up (including the todo in the doc for fromUserInput method.

an @todo without an corresponding issue on d.o is like a scream in space

All the @todo should be accompanied d.o link

Rerolling to add the link to the existing followup issue, move the documentation from the protected method, and use the data providers added in #2423579: Url::fromUrl('user-path:/') should throw an exception when a path component without a slash is given.

1. +++ b/core/lib/Drupal/Core/Url.php
   @@ -190,6 +191,48 @@ public static function fromRouteMatch(RouteMatchInterface $route_match) {
   +   *   A non-empty string from user input that conforms to the syntax of a
   +   *   @link https://tools.ietf.org/html/rfc3986#section-4.2 relative URI reference @endlink
   +   *   where the relative part is of type @link https://tools.ietf.org/html/rfc3986#section-3.3 path-abempty @endlink.
   +   *   In other words, the first character must be a '/', '?', or '#'.
   

   Not sure about putting all this in there. It's not just about conforming to blah blah RFC blah, it's also about supporting the user interaction pattern of these characters indicating the user wants to enter a path instead of autocompleting a title.

2. +++ b/core/lib/Drupal/Core/Url.php
   @@ -190,6 +191,48 @@ public static function fromRouteMatch(RouteMatchInterface $route_match) {
   +    if ($scheme === FALSE) {
   +      throw new \InvalidArgumentException(String::format('The URI reference "@uri" is malformed.', ['@uri' => $relative_reference]));
   +    }
   +    if ($scheme !== NULL) {
   +      throw new \InvalidArgumentException(String::format('The URI reference "@uri" has a scheme, but only relative references are supported by Url::fromUserInput().', ['@uri' => $relative_reference]));
   +    }
   +    if ((strpos($relative_reference, '/') !== 0) && (strpos($relative_reference, '#') !== 0) && (strpos($relative_reference, '?') !== 0)) {
   +      throw new \InvalidArgumentException(String::format('The URI reference "@uri" has a non-empty but rootless path component, which is not supported by Url::fromUserInput(). The reference must begin with a "/", "?", or "#".', ['@uri' => $relative_reference]));
   +    }
   

   I also disagree with these exceptions. The messages are too technical. We already added an exception in #2423579: Url::fromUrl('user-path:/') should throw an exception when a path component without a slash is given and this is not about malformed URIs. This method should be about supporting user input.

3. +++ b/core/lib/Drupal/Core/Form/ConfirmFormHelper.php
   @@ -34,7 +34,7 @@ public static function buildCancelLink(ConfirmFormInterface $form, Request $requ
   -      $url = Url::fromUri('user-path:/' . $options['path'], $options);
   +      $url = Url::fromUserInput('/' . $options['path'], $options);
   
   +++ b/core/modules/field_ui/src/FieldUI.php
   @@ -61,7 +61,7 @@ public static function getNextDestination(array $destinations) {
   -      $next_destination = Url::fromUri('user-path:/' . $options['path']);
   +      $next_destination = Url::fromUserInput('/' . $options['path']);
   
   +++ b/core/modules/path/src/Controller/PathController.php
   @@ -86,10 +86,10 @@ public function adminOverview(Request $request) {
   -      $row['data']['alias'] = $this->l(Unicode::truncate($data->alias, 50, FALSE, TRUE), Url::fromUri('user-path:/' . $data->source, array(
   +      $row['data']['alias'] = $this->l(Unicode::truncate($data->alias, 50, FALSE, TRUE), Url::fromUserInput('/' . $data->source, array(
   ...
   -      $row['data']['source'] = $this->l(Unicode::truncate($data->source, 50, FALSE, TRUE), Url::fromUri('user-path:/' . $data->source, array(
   +      $row['data']['source'] = $this->l(Unicode::truncate($data->source, 50, FALSE, TRUE), Url::fromUserInput('/' . $data->source, array(
   
   +++ b/core/modules/system/tests/modules/form_test/src/Form/FormTestRedirectForm.php
   @@ -55,7 +55,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
   -        $form_state->setRedirectUrl(Url::fromUri('user-path:/' . $form_state->getValue('destination')));
   +        $form_state->setRedirectUrl(Url::fromUserInput('/' . $form_state->getValue('destination')));
   
   +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
   @@ -1313,7 +1315,7 @@ protected function renderAsLink($alter, $text, $tokens) {
   -        $alter['url'] = CoreUrl::fromUri('user-path:/' . ltrim($path, '/'));
   +        $alter['url'] = CoreUrl::fromUserInput('/' . ltrim($path, '/'));
   
   +++ b/core/tests/Drupal/Tests/Core/UrlTest.php
   @@ -171,6 +172,101 @@ public function testFromRouteFront() {
   +      // Path without a leading slash containing a query string.
   

   Also still concerned about the leading slashes here. I think maybe we need another followup issue about destinations?

   It also concerns me that literally every use is also needing to append a leading slash. Is this method accomplishing our goal here?

Also realized that the exceptions themselves are now in several different code paths and thrown in other cases. I think this is incorrect. This method should define what's valid as user input, not what's valid as a URI. #2423579: Url::fromUrl('user-path:/') should throw an exception when a path component without a slash is given and fromUri() already take care of validating URIs.

Discussed with @effulgentsia. For #14 points 1 and 2, we agreed to rename the parameter to $user_input, to make it explicit that this is an unvalidated user string. We will remove the first two exceptions as they are redundant with the third, but include a comment explaining why it's sufficient to require the leading / # ? and append the scheme. For point 3, we will fix that in #2418219: Deprecate destination URLs that don't include the base path. @effulgentsia is also filing a related followup for the path alias storage.

Here we are; sorry for the delay. Two things are still outstanding: One, we still need the followup for path alias storage (see #16), and two, there's some docs on fromUserPathUri() that I was intending to move but now I'm not sure about. That could easily also be a followup.

Other than that, I think this is probably ready.

All those changes look great to me.

Beta eval.

Committed 8d87f7f and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

diff --git a/core/lib/Drupal/Core/Form/ConfirmFormHelper.php b/core/lib/Drupal/Core/Form/ConfirmFormHelper.php
index d7b557e..57a2137 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'));
-      // @todo Revist this in https://www.drupal.org/node/2418219.
+      // @todo Revisit this in https://www.drupal.org/node/2418219.
       $url = Url::fromUserInput('/' . $options['path'], $options);
     }
     // Check for a route-based cancel link.
diff --git a/core/modules/field_ui/src/FieldUI.php b/core/modules/field_ui/src/FieldUI.php
index 15fd97e..f15e7c7 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.
-      // @todo Revist this in https://www.drupal.org/node/2418219.
+      // @todo Revisit this in https://www.drupal.org/node/2418219.
       $next_destination = Url::fromUserInput('/' . $options['path']);
     }
     return $next_destination;

Fixed on commit.