Url::fromUrl('user-path:/') should throw an exception when a path component without a slash is given

I independently cross-rolled this patch yesterday.

Near identical.

May the reviewers choose which they prefer :)

Clarifying title; no leading slash is fine if no path component is specified in the URI. I.e. these are all valid:

• user-path:#foo
• user-path:?foo=bar#baz

And none of them have a leading slash.

Yep we added the validation for that in #2417567: Rename user-path: scheme to internal:, and I was also thinking it should happen with the scheme generally, rather than at the wrapper level.

Adding that coverage.

Here's that extended test coverage. Note that I've done this:

    $url = Url::fromUri('user-path:' . $path);

...in the hope of reusing the data providers for tests for fromUserInput() (see #2417567: Rename user-path: scheme to internal:).

+++ b/core/tests/Drupal/Tests/Core/UrlTest.php
@@ -637,6 +637,78 @@ public function providerTestToUriStringForUserPath() {
+   * Tests the fromUri() method with an invalid entity: URI.
+   *
+   * @covers ::fromUri
+   * @dataProvider providerFromValidUserPathUri
+   */
+  public function testFromValidUserPathUri($path) {
+    $url = Url::fromUri('user-path:' . $path);
+    $this->assertInstanceOf('Drupal\Core\Url', $url);
+  }

The docblock should say "valid" here rather than "invalid". :P

Agreed:

1. +++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php
   @@ -154,13 +154,20 @@ public static function validateUriElement($element, FormStateInterface $form_sta
   +      // Why isn't there a return here?
   

   That's a great, great question. Let's do that.

2. +++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php
   @@ -154,13 +154,20 @@ public static function validateUriElement($element, FormStateInterface $form_sta
   -      $url = Url::fromUri($uri);
   +      try {
   +        $url = Url::fromUri($uri);
   +      }
   +      catch (\InvalidArgumentException $e) {
   +        $form_state->setError($element, t("The path '@link_path' is either invalid or you do not have access to it.", ['@link_path' => static::getUriAsDisplayableString($uri)]));
   +        return;
   +      }
   

   These changes are also being made over at #2419693: Move URI access validation from widget to field constraint — you discovered exactly the same problem here.

Awesome! Just one super silly typo :P But marking RTBC, because that can easily be fixed on commit.

+++ b/core/tests/Drupal/Tests/Core/UrlTest.php
@@ -637,6 +637,78 @@ public function providerTestToUriStringForUserPath() {
+   * Tests the fromUri() method with a valid entity: URI.
...
+   * Tests the fromUri() method with an invalid entity: URI.

s/entity:/user-path:/

I am working and fixing #16

I replaced entity: with user-path: Tests the fromUri() in this two methods documents.

Thanks! :)

My one worry with this patch was that this developer-oriented exception message would leak its way out into "user space" but that doesn't seem to be the case:

Therefore, looks good! Committed and pushed to 8.0.x. Thanks!

#20: Indeed it doesn't, that would indeed be unacceptable. Validation callback + constraints are responsible for what you see in the UI. Do we ever really show PHP-level exception to the end user? I can't think of any place where we do that. I sure hope we don't :P

Minor followup: #2431085: Validation error for manually entered paths in the link widget is a little confusing