Add support for /web as a subdirectory of the webroot

Unfortunately 11.4 seems to have made this a bigger problem again. The workaround in #92 is no longer adequate.

Workaround Failure

The error appears again (but not by merely visiting the site):

Redirects to external URLs are not allowed by default, use \Drupal\Core\Routing\TrustedRedirectResponse for it.

It can be reliably triggered by (so far):

1. submitting any form -- e.g. saving a display mode, saving a node, using the "reset" button on a view with Facets/BEF/AJAX.
2. visiting the login form at /user (I didn't expect this, so maybe it's a useful clue)

Brief tests

Manually appending a destination parameter results in the following:

1. adding the usual destination, without prefixing the path, throws the same error
2. prefixing the destination with "web/" does not throw the error

On that second point:

A user, logged in via mydomain.com/user, is not recognized as logged in by mydomain.com/web/whatever, unless they have also logged in at mydomain.com/web/user.

That said: blocking people from seeing a site configured this way by visiting "mydomain.com/web" should also be a goal. So if possible, the instructions for .htaccess in "Relocated web root on shared hosting" should be updated (if possible) to prevent that, for many reasons.

Next steps?

Hopefully this info is useful to someone with more insight to the changes between 11.3 and 11.4 -- that is not me, at least not today.

In any event -- whether or not a fix, workaround, or feature emerges -- the instructions for "Relocated web root on shared hosting" are obsolete. They'll need to be updated, refuted, or slapped with a disclaimer for >11.4

Thoughts on a built-in solution

1. Being able to do this without seeing the site / submitting a form, critical. As you mentioned, it's possible with your solution.
2. Having a way to do it during initial install would be even better (I'll list reasons if anyone asks).
3. Perhaps it could be even simpler: rather than a text value for the folder name, have a single toggle for if web is the root, or if it's a child of the root.

#2 and #3 together would allow people to get up and running without immediate problems / interruptions. I think a more reasonable scope is "to allow the site to serve from a root containing vendor and web" rather than to attempt "to allow people to specify a custom web root" and all that implies.

My apologies for any confusion!

The two things I refer to are #92 in the issue you linked and (by extension) the documentation page that you worked on, and also linked.

I put the details here, since the inadequacy of the workaround makes a formal, permanent solution more necessary... and also to make sure that anyone who tries to tackle this, is aware that the workaround isn't adequate.

I posted a short comment in that issue, linking here, so that anyone who finds the workaround is aware it's not adequate for 11.4.x

Again: apologies. Please give my comment in this thread another read, in this context.

What we probably actually need, is for someone more familiar with the specific differences between <11.3.x, and 11.4.x, to chime in -- I for one don't know where (in the code) to start looking, to try to put together an updated workaround.

I don't think this should be a setting in the UI. What if you set it to an invalid value, how do you get back to the site to fix it? And if it's wrong in the first place how do you get to the page to fix it anyway?

The failure of the workaround is tied to the changes in D11.4 to bootstrapping and request handling. Changing the globals in settings.php is too little, too late, for when $base_url is prepared in the kernel.

The check in Drupal\Component\Utility\UrlHelper::externalIsLocal() uses $base_url -- which includes the subfolder -- and fails.

A start is patching DrupalKernel itself, to remove the web folder from the path while building $base_url. I believe they're only used when building links, so I think it should be safe.

I have not tested on a multisite installation of any kind.

In the below, your value would be $subfolder

protected function initializeRequestGlobals(Request $request) {
    global $base_url;
    // Set and derived from $base_url by this function.
    global $base_path, $base_root;
    global $base_secure_url, $base_insecure_url;

    // Create base URL.
    $base_root = $request->getSchemeAndHttpHost();
    $base_url = $base_root;

    // For a request URI of '/index.php/foo', $_SERVER['SCRIPT_NAME'] is
    // '/index.php', whereas $_SERVER['PHP_SELF'] is '/index.php/foo'.
    if ($dir = rtrim(dirname($request->server->get('SCRIPT_NAME')), '\/')) {

      // --- BEGIN NEW WORKAROUND CODE ---
      // Remove the subfolder if present, allowing .htaccess to point into /web.
      $webroot = "/web";
      $len_webroot = strlen($webroot);
      if ($len_webroot > 0 && strncmp($dir, $webroot, $len_webroot) === 0) {
        $dir = substr($dir, $len_webroot);
      }
      // --- END NEW WORKAROUND CODE ---

      // Remove "core" directory if present, allowing install.php,
      // authorize.php, and others to auto-detect a base path.
      $core_position = strrpos($dir, '/core');
      if ($core_position !== FALSE && strlen($dir) - 5 == $core_position) {
        $base_path = substr($dir, 0, $core_position);
      }
      else {
        $base_path = $dir;
      }
      $base_url .= $base_path;
      $base_path .= '/';
    }
    else {
      $base_path = '/';
    }
    $base_secure_url = str_replace('http://', 'https://', $base_url);
    $base_insecure_url = str_replace('https://', 'http://', $base_url);
  }

Redirects to external URLs are not allowed by default, use \Drupal\Core\Routing\TrustedRedirectResponse for it.

Yep, that's the one. I'll crosspost it in there later, to up the odds of anyone finding it.

I was hesitant to update the doc page, without double-checking... but the end goal really is just doing the same thing (making $base_url and the request object happy) but with different timing, more flexibility, and less mystery. If the original were harmful, hopefully someone would have spoken up in the "bug thread" (as they did for the dangerous patch).

For now: thank you for helping keep knowledge of this problem out there, and documented!

Opening this back up to work on a possible formal solution.

This is a drop-in snippet I was been using for a while, with "success".
It's got its limitations, obviously... it only handles $base_url which is on its way out.

// if the request contains a directory $dir
$webroot = getenv('DRUPAL_WEBROOT');
if ($webroot !== FALSE && str_starts_with($dir, $webroot)) {
  $dir = substr($dir, strlen($webroot));
}
// derive $base_path from $dir by removing "core" directory
// end if

But it was handy: the most common case is covered entirely by one extra line in the .htaccess:

SetEnv DRUPAL_WEBROOT /web
RewriteCond %{REQUEST_URI} !^/web/
RewriteRule ^(.*)$ /web/$1 [L,QSA]

...a pattern which could adapt to complex setups.

Thank you for the typos and the clean-up.

Please don't promote stuff that explicitly says "do not use" as a fix. I shouldn't have put it in the "bug report" issue to begin with.

I want to actively prevent people from using that. That code is not a fix. It breaks things because $base_path and $base_url aren't enough.

I know this, but a lot of people aren't going to know the difference. Ten years ago, people were applying a security hole. I don't want anyone running a proof-of-concept with known (and serious) flaws and omissions.

I've put together something more equivalent to the "settings.php" part of the old workaround. It's way safer and easier for people to drop in, if anyone really must. It's in #2612160: .htaccess redirect on shared hosting error: Redirects to external URLs are not allowed by default -- comment #106.

After late-night reading in core and Symfony, I still have more to read.

I only ask you be as patient, as you are dedicated and eager :)

After some more reading:

Changing $_SERVER['SCRIPT_NAME'] is how it happens (if at all).

The way Symfony itself is structured and written* it's simply not practical, from within Drupal, to change what happens when the $_SERVER variables are used to create what gets returned by $request->getBaseUrl (et al)

I'll put up another MR later which does so in the best-suited place.

*hey, at least it was educational

Has anyone solved this issue?

@yousefanbar:

Hello! Please don't use patch 22 or MR !16356.

Has anyone solved this issue?

There's a workaround in this comment that should do the trick.

I have updated code to put in this thread later, but: it's the same code, only where it "belongs" instead of where it's easy for people to do.

@ressa:

I made a "user friendly" workaround which is actually intended to work. That's the thing that people should get linked to.

Please stop telling people to use MR !16356 and the patch from it. I removed it everywhere I could for a reason.

It was part of a thought process. It's not an "experimental" fix and it is definitely not part of "Steps to Reproduce." That code won't help anyone. Feedback on it won't help me. It was used it to confirm a few suspicions, and that's it.

I am not saying "it needs a disclaimer." I am asking you to please stop making links to it.

I will hopefully be back with an actual MR soon.

Use the workaround in comment 106.

All right, my intention was only to help the process gain traction

Understood and appreciated, honestly. I just needed your patience to match your determination and enthusiasm :)

This last MR and the #106 workaround ended being similar, but:
the "workaround" version is to do it in a small file in /web.
the MR is in the file it belongs in, down in /core/lib/Drupal/Core/Runtime

...so even if I did "the real one" first: different audience, for a different usage = different package

The workaround is #106, where it's easier to find / remember, and the file itself is less likely to change. That's important to me, since (if no fix gets committed) this is a workaround that needs to be reapplied every update.

That's important for smaller / less experienced shops -- I bet there's a lot of overlap between those, and people who need this patch!

Good news: MR !16369 passed its tests. Probably won't get committed, but -- you never know unless you ask ;)

The workaround version uses the same mechanism in (almost) the same place in the code flow, so -- if the workaround works, this will work.

A big help would be, if you can test it in a safe place, including install.php and update.php. Be sure the ".htaccess stuff" is in place before you try to install.

The change in DrupalRuntime's constructor won't cover install.php, authorize.php, and rebuild.php.

At a glance, the three don't have a lot in common, so I'm going to put this back down for a bit, before double-checking.

Not ideal, but they can get spot-fixed, if needed. If that's the case, I'd try to find an appropriate place to put a function which they can all make use of --- which might just end up being an include. We'll see.

This means for the workaround covers the two big ones fine -- which the old one was (out of the box) limited to, actually -- but it's simply not there yet. So, it's enough for a rescue, but not enough to declare support (which the environment variable will imply).

It'll be a bit before I can get back on this.

Thank you, almost-only other proactive person :)

I tweaked the doc page a little bit for clarity and to be more user friendly.

The workaround with the MR 16369 here and the .htaccess shared in comment #106 in the main issue and the Relocated web root on shared hosting doc page work well for the common use cases like accessing /admin/reports/updates/check, flushing caches, etc.

Thank you for testing!

It's good timing to post an actual patch file of "the workaround version" (changes autoload_runtime.php) over there in the bug report, and then add a quick little link to it on the doc page. Want to tackle that while I keep digging?

You caught that it won't cover install.php, authorize.php, and rebuild.php before I got around to try that out

For the record, neither did the old workaround.

But I don't know if they need to be covered. I think we might be fine!

1. Installation, just do it at example.com/web.
2. Emergency rebuild is very deep under the hood, but I'm investigating anyway.
3. Authorize is the server talking to itself about filesystem, and it's deprecated anyway: #3512364: authorize.php, the FileTransfer and Updater systems, and all related code is deprecated

But again, I haven't REALLY looked through them.

For the workaround, for now, we just need to make sure that 11.4 made us have to ONLY change the means (settings.php). That the method (modifying SCRIPT_NAME early) doesn't break anything that involves /web/index.php and /web/update.php.

On what IS covered:

The old workaround explicitly targets only /web/index.php and /web/update.php. The "omission" of the others isn't new, and...

the current workaround covers the most important parts

...which are probably the only parts most people using it "need."

Yeah, I can confirm: Hostinger's lower-tier plans definitely don't allow it. Their shell is also currently PHP 8.2 regardless of which version your server actually runs (but you can run composer and drush just fine -- for now? -- by calling them through the proper version's executable).

In my experience, a lot of people hear about them being "good cheap Drupal hosting," get a plan that isn't VPS, and start hitting roadblocks immediately with anything but the auto-install "set up Drupal on my domain" button.

So, rebuild is fine, until the redirect at the end. Do we care? I'd rather it be fixed.

But: the heat is off --- the workaround's good! -- so I'm not sure when that will be.

Covering everything means reintroducing core/includes/request.inc (pretty cheeky) to hold the function, and calling it from each of the front controllers (index.php et al)

The "workaround" used autoload_runtime.php to simply things -- one paste instead of two -- but that's not where it belongs. My last attempt was a nice try, but it would make a mess if I kept it. It's better to just use the same pattern in all of the front controllers, and they'd all be within two directories of request.inc so it's clean and clear.

Next one from me will be the last one, when it comes -- I made a couple of sloppy mistakes trying to do it between other things, so I'm going to wait until I've bought time (or the weekend comes). But it will be good for

Thank you for testing and documenting!

@mw4ll4c3: I see that patching has been deprecated, but nonetheless was able to create one from comment #106 from this issue, as @b0gucki3 had done.

Also, in thinking this problem over, it reminds me of a similar issue with Drush linking. To get around that, the recommended fix is to add symlinks that can be called on. The only problem I would see with symlinking /web is when having separate environments on one server. That's currently what I deal with, say having a staging.site.tld and site.tld.