Support Paths relative to the server root instead of drupal root

Great patch! This works for me. I can now user server:/images/someimage.png to get to the "root" of the server for content outside of my Drupal site.

1. I can see how this might be used in a situation where Drupal is being used as part of a suite of applications all running of the same server in sub-directories. Doing something like this certainly could make deployment simpler. I'm going to ping this to @dawehner for a sub system maintainer POV.

2. +++ b/core/lib/Drupal/Component/Utility/UrlHelper.php
   @@ -397,7 +397,9 @@ public static function isValid($url, $absolute = FALSE) {
   +      $/xi", $url) or
   +      //ckb root_server 5.20.2016
   +      (bool) preg_match("/^server:/", $url);
   

   This can be part of the first preg_match above. Also changing UrlHelper means we need new coverage in \Drupal\Tests\Component\Utility\UrlHelperTest - which the patch has - nice.

3. +++ b/core/lib/Drupal/Core/Url.php
   @@ -306,7 +306,8 @@ public static function fromUri($uri, $options = []) {
   +      //ckb root_server 5.20.2016
   +      if ($uri_parts['scheme'] !== 'base' and $uri_parts['scheme'] !== 'server') {
   

   Changing means we need test coverage in \Drupal\Tests\Core\UrlTest - which the patch has nice.

4. +++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php
   @@ -54,7 +54,8 @@ public function assemble($uri, array $options = [], $collect_bubbleable_metadata
   +    if (parse_url($uri, PHP_URL_SCHEME) === 'base' or parse_url($uri, PHP_URL_SCHEME) === 'server') {
   

   Let's just do one parsing of the URL not twice.

5. +++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php
   @@ -54,7 +54,8 @@ public function assemble($uri, array $options = [], $collect_bubbleable_metadata
          return $this->buildLocalUrl($uri, $options, $collect_bubbleable_metadata);
   
   @@ -107,7 +108,9 @@ protected function buildLocalUrl($uri, array $options = [], $collect_bubbleable_
   -    $uri = substr($uri, 5);
   +    //ckb root_server 5.20.2016
   +    $isServer = parse_url($uri, PHP_URL_SCHEME) === 'server';
   +    $uri = substr($uri, $isServer === TRUE?7:5);
   

   We should consider passing the pass scheme into buildLocalUrl so we don't have to re-parse the URL in order to remove base: or server: later.

6. +++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php
   @@ -120,10 +123,18 @@ protected function buildLocalUrl($uri, array $options = [], $collect_bubbleable_
        // Strip leading slashes from internal paths to prevent them becoming
        // external URLs without protocol. /example.com should not be turned into
        // //example.com.
   -    $uri = ltrim($uri, '/');
   +    //ckb root_server 5.20.2016
   +    if (!$isServer ) {
   +      $uri = ltrim($uri, '/');
   +    }
    
        // Add any subdirectory where Drupal is installed.
        $current_base_path = $request->getBasePath() . '/';
   +    //ckb root_server 5.20.2016
   +    if ($isServer){
   +      // don't include base drupal path
   +      $current_base_path = '';
   +    }
   

   This could be re-written to be more efficient. Ie.

   if ($is_server) {
     $current_base_path = '';
   }
   else {
     // Strip leading slashes from internal paths to prevent them becoming
     // external URLs without protocol. /example.com should not be turned into
     // //example.com.
     $url = ltrim($uri, '/');
     // Add any subdirectory where Drupal is installed.
     $current_base_path = $request->getBasePath() . '/';
   }
   </li>
   
   <li>
   <code>
   +++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php
   @@ -120,10 +123,18 @@ protected function buildLocalUrl($uri, array $options = [], $collect_bubbleable_
   +    //ckb root_server 5.20.2016
   

   Not needed in a Drupal patch

7. +++ b/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php
   @@ -585,4 +585,14 @@ public function providerTestExternalIsLocalInvalid() {
   +  /**
   +   * Tests isValid for a server: schema.
   +   * ckb root_server 5.20.2016
   +   * @covers ::isValid
   +   */
   +  public function testValidAbsoluteServerScheme() {
   +    $valid_url = UrlHelper::isValid('server:', TRUE);
   +    $this->assertTrue($valid_url, 'Server is a valid URL.');
   +  }
   

   This should be tested by adding something to providerTestValidAbsoluteData not its own test method. Plus 'server:' should not be valid URL on its own.

Hello @alexpott,

This is our exact use case, multiple sites (complete Drupal installs) on the same server under different directories.

Not only does this allow access to different Drupal sites on the same server, it allows simplified access to static content managed outside the Drupal projects due to sharing that same static content between sites. Say for example images, css, js, PDFs, etc that are managed by a git project.

I am new to submitting feature requests to the Drupal project. Is there more you need me to do, at this time, on this ticket for its consideration into the Drupal project?

Thanks,
Brooke

@hedrickbt well there are a number of points in the review in #6 that require the patch to be updated - and doing that will make it easier for reviewers to concentrate on the actual feature request.

@alexpott and myself talked about and agreed that server:/ is a bit of a confusing term. What about using domain:/? This is way more explicit and is part of the URL spec.

For the testbot

@burgdawg: Can you create interdiff.txt these make reviewing the changes to patches a lor easier. In your patch you are using tabs instead of spaces. Most editors let you make the setting to change a tab for a number of spaces. For Drupal this number is 2.

For the testbot.

@alexpott already gave a +1 to the concept in #6, so removing the "Needs framework manager review" tag. The implementation still needs to go through the normal patch review process, but it can do so without that tag.

What about using domain:/?

I would suggest host: instead, since that is the terminology for it in https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Syntax.

Funnily enough @dawehner came up with host: in IRC after posting #9 - I thought he'd posted the update here... definitely +1 to #18.

@burgdawg: You can make the changes that alexpott wants. You do not have to wait for the subsystem maintainer for that.

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge require as a guide.

Posted in the #contribute slack channel if a core committer could take look.

I'm removing the 'needs subsystem maintainer' review tag since as @smustgrave pointed out there isn't one for 'base system' and two framework managers have already reviewed this.

+++ b/core/lib/Drupal/Core/Utility/UnroutedUrlAssembler.php
@@ -107,6 +108,7 @@ protected function buildLocalUrl($uri, array $options = [], $collect_bubbleable_
+    $isServer = $urlScheme === 'host';

This should probably be $is_host now the scheme name has changed - in general any local variables or method parameters should be snake case.

The issue summary no longer matches the patch/discussion in the issue, so could use an update. Also this needs a change record to explain the addition.