Remove sites/default/settings.php and ship with sites/default/default.settings.php instead

I assume this should be for 5.x, since 4.7.x doesn't have install.php.

I agree with merlinofchaos' analysis, and would most like to see it go one step further and ask you what domain you'd like to access the new site under and create the appropriate folder in sites for that domain.

a big +1 for robert's further refined suggestion above--i think that would be a much more refined implementation than we have now, and probably not that hard to implement.

Hm. Except...

I usually set sites up on localhost, initially. Then move them to wherever. If I re-run install.php, does it create the settings file, even though the db tables already exist? Or does it error out saying "Drupal is already installed"?

Oh well cool. Looks like it does!

In that case, HUGE +1 for sites/whatever.com. How many times have we had to re-roll patches because sites/default/settings.php got overridden? ;)

If we do choose to add a UI element for this though, it should be under the "Advanced options" fieldset and should be pre-populated with the derived base URL.

Let's try this... may need work though because:

a) I don't know how platform-independent $_SERVER['HTTP_HOST'] is.
b) I only tested it on my own localhost server.

Uh huh. Not so fast.

1. If your site is at www.example.com/subdir and you install it by going there, how will the site work when you go to example.com/subdir ? We likely want to keep only the last two parts.
2. You just make a directory in sites? Wow. You shall check for write permissions on sites in the beginning and ask for removal of writeability at the end.
3. If Apache creates example.com.subdir then we might as well try to chmod the dir and the settings.php as well before asking the user to remove the writeability.
4. Indeed our guesswork might fail because there are so many multisite combinations, so there should be an UI element prefilled by example.com.subdir .

It's not impossible I left out a few things. But most importantly, given the number and the scope of changes and how installer indeed works at this time I deem this a feature request. How has this become a critical issue? A critical issue is when something is fundamentally broken, say node previews do not work.

settings.php overwrites were always possible , if you have special $conf then you will take care of it, if not, it's a matter of seconds recreating a $db_url. Do not go overboard, please.

How about if drupal ships with an example.com/settings.php instead of a default/settings.php

nobody will host example.com ( http://www.rfc-editor.org/rfc/rfc2606.txt ) so nobody will accidentally overwrite the settings for their own site when they update by ftp or run install.php etc.

mfer has been making some updates to UPGRADE.txt here:
http://drupal.org/node/102011

a simple note: we not only assume nobody is running example.com, we know nobody is running that domain.

example.com is a reserved domain that does not exist in real dns servers (see the rfc I mentioned above), and nobody will run it, unless they are in an intranet that has their own *.com space (but then all bets are off anyway and we must assume there is a network admin who knows what he's doing)

@merlinofchaos - over at http://drupal.org/node/102011 the proposed UPGRADE.txt file makes a point saying not just to backup but to backup the settings.php file and where to find it is both a single site and multisite setup. Does this work for your comments in #11?

The amount of variables involved in implementing this feature means it is not going to happen for 5.0. It is simply too risky to change the installation process now. It is not just a matter of getting file system permissions right, but verifying whether this is a workable approach on the average host and updating all the installation documentation.

However, since 5.0, this is really no longer a critical problem, but merely an inconvenience. We already told the user to edit settings.php in the default dir before, in 4.7.

Here's what happens in 5.0:
- User installs Drupal 5.0, lets install.php alter sites/default/settings.php.
- User runs their site.
- User extracts Drupal 5.1, overwriting sites/default/settings.php back to the default.
- Drupal detects a default db_url string and redirects to install.php.
- After entering the db information again and writing it into settings.php, Drupal detects that it has already been installed, and will tell the user, aborting the installation process. It will also offer the option of going to update.php.

As for having to re-roll patches during development, I find it trivial to use sites/localhost instead of sites/default. As long as you do it once at the beginning, there is never any problem (unless you don't re-use CVS trees). We should not delay 5.0 just because some developers are inconvenienced by this.

This patch looks straightforward and simple to me. I haven't tested the patch, but I've no comments on the coding style. I think this is good to go after some testing.

Testing within a couple of minutes.... :-)

I love the removal of all those cheesy hacks for checking for the default db string. I also love that a pristine copy of the settings file will always be present, in case you totally hose something on your site and need to revert, or need to see the differences during an update. And of course, the number of hours that have been lost by various testers, core committers, etc. encountering patches with settings.php in them is reason enough to commit this. ;)

I tested the following conditions:
- Variety of error stuff on the main db page: leaving fields blank, invalid credentials, etc.
- Pointing the installer to a pre-existing database; it still caught this fine.
- Doing a normal install, which worked properly.
- 'databasename' database with user 'username' and pass 'password' on host 'localhost'... while I couldn't get this to work (probably messed I something up), merlin confirmed that it worked for him, which means all our code checking for this has been eliminated.

Marking RTBC.

Looks good here, too. I don't have much to add to webchick's evaluation other than "database" "username" "password" works fine for me.

I have tested the patch and can verify that it works very nicely..
Noticed no (new) errors or mis-behaviour..

IMO rtbc.

Committed to CVS HEAD. Thanks folks! :)

Grm, this broken Drupal 6.x-dev for me, because it cannot open "settings.php" for writing in the folder it wants to. Seems like writability checking is not there. After I provide my DB detailes, I get:

Warning: fopen(./sites/default/settings.php) [function.fopen]: failed to open stream: Permission denied in /var/www/installer/includes/install.inc on line 228

Warning: Cannot modify header information - headers already sent by (output started at /var/www/installer/includes/install.inc:228) in /var/www/installer/includes/install.inc on line 567

By looking at the code, this is evidently the place (install.inc:228) where the settings file is tried to be written out to the new location.

Typos! By looking at the code and putting testing debug messages to some places, it turned out that $mask does not include the FILE_WRITABLE bit. That is because you try to use it as FILE_WRITEABLE (note the extra E) there. If the two constant names are fixed in install.php, the check is fixed.

Already comitted a fix to Drupal core to replace all "writeable" to "writable".

Erm, also noticed that now all works fine, but the sites/default folder is left wide open for writing, which allows anyone to place files there (like in the file uploads folder). This is unwanted, and insecure. Drupal should check if it can revoke permissions from there, just like it revokes permission from the settings file. If it is not able to revoke, both the install complete screen and the "site status" page in Drupal should provide a line about the directory left world-writable. This is important security-wise. (I will not work on a fix, but change to the installer form patch instead).

We need the Drupal status page to report the permission problem if not possible to fix automatically, just like it reports the settings.php permission problem if detected.

I agree that checking the directory permission status is critical.

Also, this new install process does not seems to be described or explained in INSTALL.txt

I think this new process will create some problems since the settings.php file will often not be owned by the user's account, but by the webserver account. Is the update system going to have a mechanism for replacing settings.php (e.g. the user has to set the direcetory back to writable)?

@pwolanin, the update system never touched settings.php as far as I remember. Why would that be required now? (I understand that settings.php might not be possible to edit for the user, being owned by the webserver user, that is a clear limitation of the current system). Added a documentation issue here to update the install.txt (it contains two other related updates, so this is why I have the separate issue): http://drupal.org/node/144256

@merlinofchaos: The problem is that even after I apply your latest patch, I get a green line that my config file is protected on the ?q=admin/logs/status page. But the sites/default directory is wide open for writing. A fix for this is what I miss from the patch to close the circle (I already posted a patch against INSTALL.txt as noted above).

Guys, sorry to arrive so late into this. But wouldn't the following be more ideal?

./sites/all/*
./sites/default/* <----- at the moment settings.php is here!

It would be better to get rid of the /default/ directory and have something like this in place:

./sites/all/default.settings.php <--- this is the new home of settings.php
./sites/example1.com/settings.php
./sites/example2.com/settings.php

Overall, it makes for nicer tree. Also since variables in the "default" settings.php applies to "all" sites. Correct me if I'm missing the point :)

As far as I can tell, there is still not a check in the system status that the sites/default (or whichever) directory is not writable. This seems to be a significant security hole.

BioALIEN: the default site settings are not used when you have specific sites folders, 'all' is very different from 'default'.

pwolanin: We are awiting merlinofchaos pushing a patch in to fix the hole.

just tested fix_sites_3.patch on 2 systems:

on a system with php running with suexec (so running as my account), both sites/default and sites/default/settings.php are changed to non-writable. If I change them to writable, they are set to non-writable any time I visit /admin or /admin/logs/status. this workes fine at /sites/example.com as well.

on a system with php running as Apache (user www), sites/default/settings.php is set to 444, and owned by www. /sites/default is left as 777, but I get the warning at the end of the install page, and it shows up as an error on the system status page. Changing sites/default to 555 clears the error.

For this latter system, error on system status page also works as expected if I move settings.php to sites/example.com and change the permissions of either settings.php or the directory.

code changes are minor and reasonable.

I'm happy with pwolanin's feedback, so long as changing the permissions avoids creating problems with umasks (http://drupal.org/node/116041).

Merlionfchaos: instead of "completely ignoring" you, we pointed out that the patch you posted in #30 is not adequate to inform users about security problems, and as such does not fulfill your intended goal, so it was basically not ready for committing. I am glad you provided a complete fix now, which looks logical and is even tested, so I committed it. Thanks for the patch.

Well, you have probably seen people installing with "Next, Next, Yes, I agree, Done, let me use it *now*" way... No matter what strong warning you put into the installer, it will not be effective if not coupled with warnings on the runtime system nagging the admin to actually fix the problem. People easily and quickly click through the installer. I think both the installer and runtime check functionality should have been *in the original patch*, even not in follow up patches, especially not in separate issues. The installer changes and the runtime notification changes are so close that they are almost copy-paste of each other (the messages differ a little bit).

Anyone could have provided this fix, for sure. Nobody come around to this issue to fix it and you were arguing that you already fixed the concerns brought up, so it was logical to try to underline our point that the previously posted patch did not solve the issue adequately.

Excuse me if I was not clear enough throughout handling this issue.