My D8 docroot is located under this path:

http://localhost/drupal.org/8/core/

This apparently breaks the install system and from what I can tell the global base path is wrong. It assumes my base path is drupal.org/8/ instead of drupal.org/8/core.

From what I can tell here's the problamatic code:


    // $_SERVER['SCRIPT_NAME'] can, in contrast to $_SERVER['PHP_SELF'], not
    // be modified by a visitor.
    if ($dir = rtrim(dirname($_SERVER['SCRIPT_NAME']), '\/')) {
      // Remove "core" directory if present, allowing install.php, update.php,
      // cron.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 = '/';
    }
CommentFileSizeAuthor
#9 dir-core-check_for_name_of_install_dir-1542570.patch1.47 KBchemical
#7 core-dir-1542570-7.patch26.55 KBDavid_Rothstein
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ericduran’s picture

ericduran CreditAttribution: ericduran commented
Title: Install broken if you're site url has the path /core in it. » Install broken if your site url has the path /core in it.
lucascaro’s picture

lucascaro CreditAttribution: lucascaro commented
Issue tags: -wtf
David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein commented
Title: Install broken if your site url has the path /core in it. » Drupal's base path (and install) are broken if your site is in a directory called 'core'
Component: install system » base system

Ouch, that's pretty bad.

It looks like this bug isn't limited to the installer, though. If the $base_path is wrong, a lot of things will break even on an existing site, I think?

I guess the simplest fix would involve changing the code above so it doesn't strip out 'core' when the path ends in "core/index.php" (only when it ends in "core/[some.other.script].php"). But this whole thing is kind of strange, and it looks like there's similar stuff going on in .htaccess also.

I wonder if a better fix would be to take this opportunity to try making all core Drupal requests go through index.php and to have that script load the other ones as appropriate, rather than actually having users visit each of those scripts directly...

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

We could also just move install.php, update.php and cron.php out of /core, I'm not really sure why they're in there.

David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein commented

I'm guessing they're there to facilitate easier upgrades (so people upgrading sites can just swap out the core/ directory even in the case where one of those files changed)?

On the other hand, those files all should be pretty minimal anyway (for example, install.php already is). And it would be easy enough to have a copy of each that's as minimal as one or two lines; e.g., the top-level update.php could literally just include core/update.php. So maybe that's the way to go.

I thought a little bit more about my idea of having everything go through index.php, and it probably needs to be a separate issue. It's straightforward (and a pretty good cleanup) without worrying about clean URLs, but since we have to allow for sites with clean URLs off, we'd start having lots of links like http://example.com/?q=update.php in the documentation (since the docs always tend to cater to the possibility that clean URLs are off) and in the user interface itself (since especially in install.php we have a lot of links that need to be generated before we know whether or not clean URLs are available).

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

I think a copy of the files in the root directory that just include the ones in the /core directory sounds good

David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein commented
Status: Active » Needs review
FileSize
core-dir-1542570-7.patch26.55 KB

So, something like the attached patch then...

Although the duplicate files in and outside of 'core' is a little confusing, plus if you do go ahead and visit core/install.php you now will get fatal errors.

So possibly, moving the ones in 'core' to 'core/includes' instead, renaming them to a common pattern (like core/includes/install.bootstrap.inc, core/includes/update.bootstrap.inc, etc?) and then moving all their top-level code into a function (named along the lines of drupal_deliver_install_page(), drupal_deliver_update_page(), etc?) would be a bit better. At that point the top level scripts would be two lines of code rather than one but still pretty simple:

require_once './core/includes/install.bootstrap.inc';
drupal_deliver_install_page();

A similar pattern could be applied to index.php if we wanted to.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Priority: Major » Normal
Status: Needs review » Needs work

The proposal wasn't to move the files, but to include them; i.e.:

require dirname(__FILE__) . '/core/' . basename(__FILE__);

Not sure why this is major.

I actually vote for a won't fix. This is trivial to document (for the 0.00001% it affects...).

chemical’s picture

chemical CreditAttribution: chemical commented
Status: Needs work » Needs review
FileSize
dir-core-check_for_name_of_install_dir-1542570.patch1.47 KB

I made a patch which does the the additional checks to handle that the Drupal installation is located in a directory named "core".

amontero’s picture

amontero
Location Barcelona
CreditAttribution: amontero commented
Issue summary: View changes
Status: Needs review » Closed (duplicate)
Related issues: +#2045685: Broken "Visit your new site" link at install process finish page if web path contains "/core"

Duplicate of fixed #2045685: Broken "Visit your new site" link at install process finish page if web path contains "/core"