Currently the installer code basically lives in two files:
- install.php a huge and hard to maintain file
- includes/install.inc file which contains other common code used by the installer and the system.

The install.php file contains installer helper functions, form API form definitions, validation and submit handlers for the configuration forms, batch handler functions, inline Javascript code, etc. Parts of the code is not modularized, being difficult to maintain, debug, test and extend.

For this issue #315901: Add server capabilities check to installation/upgrade tasks is important to include the fewest amount of code during the first bootstrap of the installation/upgrade process, so that the server capabilities can be checked before continuing the installation and getting a fatal error on a sever with no capabilities to run Drupal 7.x (note that we are requiring PHP 5, PDO, etc..., and a lot of people don't read the requirements).

To achieve this, we require to organize an make the installer more modular.

I propose the following structure:

/drupal root
--includes
--install (installer container directory)
----installer.inc (installer helper functions)
----installer.js (installer /javascript code)
----installer.pages.inc (installer form definitions, for validation and submit handlers)
----installer.tasks.inc (installer tasks - the big install_task() function split in several individual function for each task)
--
--
--install.php (installer entry point)

This will allow us to extend, improve and maintain the installer easier.

The attached patch implements this idea. It's a big patch, but has not been added any new code, only the code has been organized in several files or has been included in separate functions. With the patch applied, the installer continue being functional and work as expected.

Tests and feedback are welcome.

CommentFileSizeAuthor
install-directory.patch82.8 KBdropcube
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

I'd also recommend the same implementation for the update.php file as well.

dropcube’s picture

dropcube CreditAttribution: dropcube commented

@Dave Reid: Yes, if we get success with this, we can follow up with the update subsystem.

bsherwood’s picture

bsherwood CreditAttribution: bsherwood commented

What about moving everything pertaining to installation to /install (including install.php)? This way a user could delete the entire folder upon installation. This might help provide some security. To the best of my knowledge Joomla! handles this process. I am unaware of any security risk leaving it in Drupal root but if hacked install.php could wipe out an entire system.

Also, since the code in install.php is used rather infrequently and has the potential to cause harm if hacked, I would like to see drupal recommend the folder be deleted upon successful installation. Maybe create a 'warning' flag on the status report.

The only issue I could see is a user wanting to create a multi-site setup. Under this setup, if an admin did delete the /install directory, all they would need to do is copy it from the drupal tarball to drupal root and proceed as normal.

Comments?

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

@specmav: I do rename my install.php because if the DB is down for some reason we get that nasty link to install.php. A big WTF is exclaimed the first time one sees it. Rather than deleted a .htaccess file could be created that could add a layer of security against accessing the install script.

bsherwood’s picture

bsherwood CreditAttribution: bsherwood commented

@earnie: Renaming install.php seems like a good idea in the interim. Otherwise if the DB does go down you get a list of "can't connect to table" error messages. It doesn't look pretty.

My concern was that the whole install process should be self contained for admin's to delete the whole folder to give an extra measure of security. A lot of CMS's do this as a "just in case" method.

Under this method, an admin can install as many Drupal sites as he wants, then delete the /install directory. If he needs to add another site to his multi-site setup, just take the /install directory from the tarball and place it back into Drupal root.

This might be helpful for admins on shared hosting where the box might have a weak security policy or incorrectly configured permissions/groups. If they just delete the folder, an external/internal user can't exploit it.

Kind of a "don't invite trouble" policy.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

@specmav: The problem with your plan is that it is too technical for a large percentage of the Drupal users. Renaming is something they non-techi can achieve without much fuss. An install directory with a .htaccess that requires authentication would be good. The non-techi could manage to change a default password.

dropcube’s picture

dropcube CreditAttribution: dropcube commented

@specmav, @earnie: The admin should be able to decide if remove or not the entire install package upon installation. What Drupal can do is to provide a warning on the status report page, +1 for this idea.

Any reviews to the patch?

bsherwood’s picture

bsherwood CreditAttribution: bsherwood commented

"Drupal is to technical for average users?" Really! I thought we left the kiddie CMS's to Joomla and Wordpress?

*end of arrogant rant*

I don't see a problem with making the install script easy to remove, rename, etc...

@dropcube: I do agree that it should be the admins decision, but a simple warning on the status report could prove valuable.

My point of suggesting that an admin could simply delete the whole folder, without worry that the whole codebase, is that I don't want to make the assumption that install.php is 100% bug free.

Maybe it is my ignorance with how Drupal determines that the install script shouldn't be run. If a site is already installed, accessing install.php gives you the "Drupal has already be installed" message. What are the steps that Drupal takes to reach that decision? Does it just check to see if the DB has tables already installed? Or is it more complicated?

My "pestering" about this, is because I do think there is a lot of powerful code in install.php. So protecting my site from it, to me, seems like a wise stance. Deletion is the radical method to help prevent any unintended problems.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs review » Needs work

The last submitted patch failed testing.

himanshu123’s picture

himanshu123 CreditAttribution: himanshu123 commented
Component: install system » theme system
Assigned: dropcube » himanshu123
himanshu123’s picture

himanshu123 CreditAttribution: himanshu123 commented

how we can change in drupal home page as our home page design in html ..

Frando’s picture

Frando CreditAttribution: Frando commented
Component: theme system » install system

Please don't hijack issues. You should file a support request instead or post in the forums. Anyway, without much more information no one will be able to help you.

boombatower’s picture

boombatower CreditAttribution: boombatower commented

subscribe.

sun.core’s picture

sun.core CreditAttribution: sun.core commented
Version: 7.x-dev » 8.x-dev
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented
Status: Needs work » Closed (works as designed)

I think this has been addressed by the move to a core directory, plus autoloading, only the bare minimum amount of code is loaded to check system reqs.