[drupal-devel] [bug] Do not execute functions in include files

moshe weitzman drupal-devel at drupal.org
Mon Jun 6 15:19:58 UTC 2005


Issue status update for http://drupal.org/node/18213

 Project:      Drupal
 Version:      cvs
 Component:    base system
 Category:     bug reports
 Priority:     normal
 Assigned to:  chx
 Reported by:  killes at www.drop.org
 Updated by:   moshe weitzman
 Status:       patch

i did a quick code review and this patch looks quite nice to me. note
that it changes some of the same bits that are changed by my  'ban
hosts' patch. i'm ok with committing this one first, and i will update
accordingly.




moshe weitzman



Previous comments:
------------------------------------------------------------------------

March 1, 2005 - 17:09 : killes at www.drop.org

Drupal currently executes functions in include files. This is very
annoying if you want to include files from external scripts such as
update scripts or other scripts that want to use Drupal as a framework.


Offending files are:


database.inc:
db_set_active();


session.inc:
session_set_save_handler("sess_open", "sess_close", "sess_read",
"sess_write", "sess_destroy", "sess_gc");
session_start();


common.inc:
A bunch of stuff at the bottom.


All this should be moved into a start.inc file.




------------------------------------------------------------------------

March 27, 2005 - 01:44 : adrian

I haven't done this recently, but back in 4.4 era I attempted something
similar to this for the install api.


A _lot_ of modifications were required to be able to use (for instance)
the theme api without needing to have a database connection active.




------------------------------------------------------------------------

March 27, 2005 - 01:55 : adrian

It's definately not a simple problem.


For instance.. the db connection needs to be started during bootstrap,
and during normal execution.


I think the better idea is to include a DRUPAL_INIT constant we can
check before executing the function. (or a DRUPAL_NO_INIT .. i'm not
picky).




------------------------------------------------------------------------

April 28, 2005 - 16:53 : killes at www.drop.org

Here's an old patch by me that illustrates what I want to do.
http://lists.drupal.org/archives/drupal-devel/2005-03/txtq23pAcHATw.txt


Instead of putting the stuff into index.php, alternative file names
need to be found.




------------------------------------------------------------------------

May 4, 2005 - 12:36 : chx

Attachment: http://drupal.org/files/issues/boot.patch (6.83 KB)

Well, I have written something to address the issue.


Now you can include everything to your hearts content just include
init.inc as well and do an appropriate drupal_bootstrap.


Alas, ATM database and bootstrap is very interwined. This
interdependency could be obliterated if we'd not use
variable_get('dev_query') in database.*.inc.


cvs diff -u -N -F^f against cvs.drupal.org as CVSROOT failed to include
to include the two new files, so I'll include them as followups.




------------------------------------------------------------------------

May 4, 2005 - 12:37 : chx

Attachment: http://drupal.org/files/issues/boot_0.patch (10.31 KB)

Hm, settings.php changes were ommited.




------------------------------------------------------------------------

May 4, 2005 - 12:37 : chx

Attachment: http://drupal.org/files/issues/init.inc (3.46 KB)




------------------------------------------------------------------------

May 4, 2005 - 12:38 : chx

Attachment: http://drupal.org/files/issues/settings_ini_set.php (850 bytes)




------------------------------------------------------------------------

May 4, 2005 - 12:44 : chx

Attachment: http://drupal.org/files/issues/boot_1.patch (14.49 KB)

Following advice from darix I have now created one patch. Still, I'd
like to know how could I do this with cvs.




------------------------------------------------------------------------

May 4, 2005 - 13:01 : chx

Attachment: http://drupal.org/files/issues/boot_2.patch (14.48 KB)

Steef found that I had $$ in settings.php.


He also asked what's this good for. Several things, but most advantages
come when you use only parts of Drupal. If you want use, say, only our
forms functions then now you can include('common.inc'); without side
effects. If you need the database layer, you do a include('init.inc')
and call drupal_bootstrap('database')




------------------------------------------------------------------------

May 5, 2005 - 16:03 : chx

Attachment: http://drupal.org/files/issues/boot_3.patch (17.98 KB)

The database layer is now totally independent of other parts of Drupal.




------------------------------------------------------------------------

May 15, 2005 - 14:37 : chx

A very stupid bug related to variable init is fixed and I am sync'd to
HEAD, so timer_init('page') is called.




------------------------------------------------------------------------

May 16, 2005 - 16:12 : chx

Attachment: http://drupal.org/files/issues/bootstrap_3.patch (12.36 KB)




------------------------------------------------------------------------

June 4, 2005 - 10:29 : chx

Attachment: http://drupal.org/files/issues/bootstrap_4.patch (12.36 KB)

Fixed drupal_bootstrap('database') -- $phase was 0 , and == FALSE fired.
=== FALSE is used.




------------------------------------------------------------------------

June 4, 2005 - 12:03 : nsk

Some days before I was studying the Drupal source code and it caused me
headaches that it executed code via include calls.  I think it is
better design to not execute anything automatically in includes.




------------------------------------------------------------------------

June 4, 2005 - 12:25 : chx

Attachment: http://drupal.org/files/issues/bootstrap_5.patch (15.26 KB)

Added phpdoc and moved drupal_unpack to init.inc thus now session
handler can be used independently.




------------------------------------------------------------------------

June 5, 2005 - 04:13 : Dries

Sorry but the quality of the PHPdoc is not up to standards.  Please use
proper punctuation, typography, line-wrapping, etc.  It should also
explain when/why to use this function.


I can't help but think this is ugly code.  drupal_bootstrap, for
example, can be written more elegantly (not tested):
function drupal_bootstrap($phase) {
  $phases = array('database', 'session', 'bootstrap', 'common');
  foreach ($phases as $step) {
    _drupal_bootstrap($step);
    if ($phase == $step) {
      break;
    }
  }
}


instead of:


function drupal_bootstrap($phase) {
  static $done = -1;
  $phase = array_search($phase, array('database', 'session',
'bootstrap', 'common'));
  if ($phase === FALSE) {
    return;
  }
  for (; $done <= $phase; $done++) {
    _drupal_bootstrap($done);
  }
}




------------------------------------------------------------------------

June 5, 2005 - 11:34 : chx

Attachment: http://drupal.org/files/issues/bootstrap_6.patch (15.56 KB)

Some code simplification in drupal_bootstrap and better PHPdoc. (Thanks
Dries and Steven.)







More information about the drupal-devel mailing list