\Drupal\Core\Url does not accept root-relative (file) URLs, making it impossible to let LinkGenerator create root-relative file URL links

, which means trouble as soon as you encounter sites available over both HTTP and HTTPS.

Isn't that the point of protocol relative URLs? //foo/bar/baz.js

There is #2573635: Url::fromUri() should accept protocol-relative URLs for protocol relative URLs.
In that issue I made the point that the URL class should implement everything which is possible, but some people disagree with it.

We usually treat the URL generation as a part of routing, so moving there. (Maybe the component should be "routing and URL generation system".)

@Wim Leers

But protocol-relative URLs are also not supported by the Url class.

There is an open issue for protocol-relative: #1783278: Scheme-relative URL rejected by validation

Discussed this with @catch, @alexpott, @xjm, and @Cottser, and we agreed this is Major, because it appears to be a regression from Drupal 7, which would impede the porting of sites that rely on it to Drupal 8.

This is very related to this issue and others that need an improvement at the Url class: #2691099: Improve external URL validation in many ways

needs work for tests

There is the available test in the patch. Can you say what's wrong there, please?

@Oleksiy it needs "test-only" patch to prove coverage

Providing interdiff for patch #15 and #19 .

@zymbian for testonly patch no interdiff is needed, it is the same patch with tests except bug fix - see 13) in https://www.drupal.org/contributor-tasks/write-tests

1. +++ b/core/lib/Drupal/Core/Url.php
   @@ -277,6 +277,10 @@ public static function fromUri($uri, $options = []) {
   +    // We support root-relative URLs.
   

   The word We is unnecessary.

2. +++ b/core/tests/Drupal/Tests/Core/UnroutedUrlTest.php
   @@ -84,6 +84,8 @@ public function providerFromUri() {
   +      // A root-relative URL.
   +      ['/robots.txt', FALSE],
   
   @@ -115,7 +117,6 @@ public function providerFromInvalidUri() {
   -      ['/test'],
   

   I wonder if the change in behaviour will have any negative impacts. I.e. times when we want the invalid URL exception for validation purposes.

3. The issue summary has lots of TBD and therefore needs updating.

Coming here from the @todo in \Drupal\file\Plugin\Field\FieldFormatter\BaseFieldFileFormatterBase::viewElements().

The thing is that the @todo there and the issue title at least here are wrong. As the patch provides, root-relative paths work just fine, you simply have to prefix it with base:.

I'm not sure we need to fix anything here, it simply means that a caller to fromUri() needs to prefix the URL with base.

+++ b/core/modules/image/tests/src/Functional/ImageFieldDisplayTest.php
@@ -127,11 +127,9 @@ public function _testImageFieldFormatters($scheme) {
     $this->assertCacheTag($file->getCacheTags()[0]);
-    // @todo Remove in https://www.drupal.org/node/2646744.
-    $this->assertCacheContext('url.site');

does it make sense to assert that we don't have any cache contexts explicitly?

Looks like a really nice cleanup, will wait with RTBC until #2402533: Provide File::createFileUrl() as a replacement for the deprecated File:url() implementation is committed as this will need quite a reroll then, should then however also be simpler as we can just remove the FALSE argument in many of these cases.

Hm, so turns out it doesn't actually work yet, not if you have a installed Drupal in a subfolder. The problem is that file_transform_relative() already includes (or actually, does not remove) the base path, and then if you run that test with against http://localhost/d8, you end up with /d8/d8 as the path.

This fixes the test but is obviously very ugly. Not quite sure what to do about that. This behavior is hardcoded in \Drupal\Core\Utility\UnroutedUrlAssembler::buildLocalUrl(), we have nothing to control that.

Good thing our tests are still running in a sub-folder..

Also fixed ComputedFileUrlTest, that could now actually be a unit test, wasn't sure about converting that here.

1. +++ b/core/modules/image/src/Plugin/Field/FieldFormatter/ImageFormatter.php
   @@ -200,16 +200,8 @@ public function viewElements(FieldItemListInterface $items, $langcode) {
        foreach ($files as $delta => $file) {
   ...
          if (isset($link_file)) {
   ...
   +        $url = Url::fromUri(mb_substr($file->createFileUrl(), mb_strlen(\Drupal::request()->getBasePath())));
          }
   

   It feels like creating a helper method would helpful. Maybe something like Url::fromUri('relative_drupal://') or so? What do you think about that, would that be worth moving to a separate issue?

2. +++ b/core/modules/image/tests/src/Functional/ImageFieldDisplayTest.php
   diff --git a/core/tests/Drupal/Tests/Core/UnroutedUrlTest.php b/core/tests/Drupal/Tests/Core/UnroutedUrlTest.php
   index fb4b84cc93..777b79b74c 100644
   
   index fb4b84cc93..777b79b74c 100644
   --- a/core/tests/Drupal/Tests/Core/UnroutedUrlTest.php
   
   --- a/core/tests/Drupal/Tests/Core/UnroutedUrlTest.php
   +++ b/core/tests/Drupal/Tests/Core/UnroutedUrlTest.php
   
   +++ b/core/tests/Drupal/Tests/Core/UnroutedUrlTest.php
   +++ b/core/tests/Drupal/Tests/Core/UnroutedUrlTest.php
   @@ -83,6 +83,8 @@ public function providerFromUri() {
   
   @@ -83,6 +83,8 @@ public function providerFromUri() {
          ['https://www.drupal.org', TRUE],
          // A protocol-relative URL.
          ['//www.drupal.org', TRUE],
   +      // A root-relative URL.
   +      ['/robots.txt', FALSE, 'base:robots.txt'],
          // An internal, unrouted, base-relative URI.
          ['base:robots.txt', FALSE],
          // Base-relative URIs with special characters.
   @@ -114,7 +116,6 @@ public function providerFromInvalidUri() {
   
   @@ -114,7 +116,6 @@ public function providerFromInvalidUri() {
        return [
          // Schemeless paths.
          ['test'],
   -      ['/test'],
          // Schemeless path with a query string.
          ['foo?bar'],
   

   Kudos for adding tests in multiple places!

+++ b/core/lib/Drupal/Core/Url.php
@@ -273,10 +273,15 @@ public static function fromUri($uri, $options = []) {
+    // Support root-relative URLs.
+    elseif (strpos($uri, '/') === 0) {
+      $uri_parts['scheme'] = 'base';
+      $uri = 'base:' . substr($uri, 1);
+    }

It fascinates me that there is already documentation for this case.

I was also thinking about another function. But we're trying to deprecate the transform functions in #2669074: Convert file_create_url() & file_url_transform_relative() to service, deprecate it in favor of a new service that actually has the transform-relative built-in in the create URL method, so I' not sure if we should have a new method there (different return values based on query arguments are ugly, we already had to add too much of that for render cacheability stuff), or on Url or what :)

I paused on the related issue to wait for this one to be finished, unsure which order makes sense now.

The problem is that I'm not sure how many other cases are affected, it's a pretty fundamental problem and we only had very few test fails and only noticed it because testbot still runs with a subdirectory for history reasons.. more things could be broken. In fact, I've been advocating to use this API in other issues, so I expect that a considerable amount of code out there is using it incorrectly already :-/

#2669074: Convert file_create_url() & file_url_transform_relative() to service, deprecate it is now in, with a method to return Url objects, and I believe it removed all those @todos from the code base.

The issue here is not yet fixed, but we now do have an API that can generate correct Url objects for files, so I'm downgrading this to normal for now unless we have a use case that doesn't work yet.

We still have work to do to modernize the underlying API's, so that we don't have to convert back and forth between the getExternalUrl() method of stream wrappers, so there _might_ still be specific issues, but they should hopefully be far less common now.

Rerolled patch for Drupal 9.3

Per my comment above, you shoudn't need this patch anymore, you are mostly just reverting back to wrong/deprecated code. If you use this patch then that might also explain why you are getting errors.

I updated the core to 9.3.2 and this issue came up. After updating the node where I attached some file to it then I saved it and when trying to go to that node the Invalid URL error WSOD came up. This patch is solving the issue. Why? I don't know, but it does.

parented to #3087434: [meta] Use FileUrlGenerator::generate() everywhere, then deprecate generateString() and generateAbsoluteString()

hiding all patches as useless

Rerolled the patch for Drupal 10.1

Think this needs an issue summary update, reading comment 51-53 @berdir makes it sound like this patch shouldn't be needed anymore.