Problem Description

Previous to Drupal 7 (Drupal 5 and 6, at least), Drupal respected the use of symlinks.

Drupal 7 seems to break the use of symlinks with the creation of DRUPAL_ROOT and the use of the realpath php function to define it.

(The reason for the symlinks is to maintain more flexible version/type of Drupal multisite)

...

I did notice that a few of the files within the scripts directory define DRUPAL_ROOT with the php function getcwd(). Is there a reason that getcwd() isn't used in all the places DRUPAL_ROOT is defined? A quick test seems to indicates that getcwd() seems to respect the symlinks and allow DRUPAL_ROOT to accurately reflect the root drupal directory.

Recreating the problem

* Put Drupal 7 core in a known directory: drupal-7.x
* Make a new directory somewhere else: mysite
* Within the mysite directory, symlink Drupal's core files and directories (except the sites directory) from drupal-7.x

Behavior expected

Drupal should respect the symlinks and define DRUPAL_ROOT as the directory mysite ... Just like it did in Drupal 5 and 6.

What Happens Instead

Drupal does not respect the symlinks and instead expands the symlinks and defines DRUPAL_ROOT as the directory drupal-7.x

CommentFileSizeAuthor
#14 getcwd-symlink-test.php_.txt425 bytesAmrMostafa
#3 0001-stop-expanding-symlinks.patch1.58 KBAnonymous (not verified)

Comments

mikey_p’s picture

mikey_p commented

subscribing

damien tournoud’s picture

damien tournoud commented

I'm not really sure I see the point of those symlinks... but I'm not opposed we use getcwd(). Please roll a patch!

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 0001-stop-expanding-symlinks.patch1.58 KB

Thanks. I'm sure it might seem strange, but breaking symlink behavior is annoying.

Attempting to roll a patch...

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Needs review

Changing the status to 'patch (code needs review)' for automatic testing

Anonymous’s picture

Anonymous (not verified) commented

The patch has passed all tests at least 8 times now.

When might I expect this to be tested by others/commited to core?

I would love to start testing Drupal 7 and submitting/patching bugs, but the current expanding-symlinks behavior is a deal breaker thus far.

Status: Needs review » Needs work

The last submitted patch failed testing.

alexanderpas’s picture

alexanderpas commented
Issue tags: +multi-site

in my opinion we should never break the usage of symlinks, as they might be used to create schemes we've never tought of!

#132988: Allow specifying a global directory for user pictures ($100 bounty for D5) also sprouted from the inability to use symlinks!

also, i tend to use symlinks in the sites directory for some cases.

damien tournoud’s picture

damien tournoud commented
Status: Needs work » Reviewed & tested by the community

Makes total sense.

Don't listen nor trust the testbot.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs work

I grepped for "define('DRUPAL_ROOT" and see the following instances which are not part of this patch:

- /scripts/password-hash.sh
- /scripts/run-tests.sh

We should be consistent everywhere, no?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs work » Reviewed & tested by the community

Oops. :D My mistake. :D Those already use getcwd(). Duh.

I want to make sure this comes back from testing bot clean, and then I'll commit it.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Cool. Test bot is happy, committed to HEAD. Thanks!

Anonymous’s picture

Anonymous (not verified) commented

Perfect! Thanks for getting that into D7 core!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

AmrMostafa’s picture

AmrMostafa commented
Status: Closed (fixed) » Needs review
StatusFileSize
new getcwd-symlink-test.php_.txt425 bytes

Are you sure this is correct? According to my tests getcwd() dereferences symlinks. See: http://www.opengroup.org/onlinepubs/9699919799/functions/getcwd.html

Steps to verify this:

cd /tmp
mkdir getcwd-symlink-test
cd getcwd-symlink-test
mkdir real
ln -s real symlink
cd symlink
wget -O test.php <attached-file> 
php test.php

On my system I get:

basename(getcwd()) == "symlink" -> FALSE
thedavidmeister’s picture

thedavidmeister commented
Status: Needs review » Needs work

Any chance we can get #14 reworked into an actual test the bots can run?

  • webchick committed d802c69 on 8.3.x 
    #363013 by mannkind: Use getwd() rather than realpath(__FILE__) to...

  • webchick committed d802c69 on 8.3.x 
    #363013 by mannkind: Use getwd() rather than realpath(__FILE__) to...

  • webchick committed d802c69 on 8.4.x 
    #363013 by mannkind: Use getwd() rather than realpath(__FILE__) to...

  • webchick committed d802c69 on 8.4.x 
    #363013 by mannkind: Use getwd() rather than realpath(__FILE__) to...

  • webchick committed d802c69 on 9.1.x 
    #363013 by mannkind: Use getwd() rather than realpath(__FILE__) to...

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.