June 2005 - Development - lists.drupal.org

[drupal-devel] [bug] Do not execute functions in include files
by Bèr Kessels 08 Jun '05

08 Jun '05

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(a)www.drop.org Updated by: Bèr Kessels Status: patch I figured it would be hard. So lets ignore my post. I will think of a way to cache it better inside node.module and taxonomy.module. Bèr Kessels Previous comments: ------------------------------------------------------------------------ March 1, 2005 - 23:09 : killes(a)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 - 07: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 - 07: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 - 22:53 : killes(a)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 - 18: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 - 18:37 : chx Attachment: http://drupal.org/files/issues/boot_0.patch (10.31 KB) Hm, settings.php changes were ommited. ------------------------------------------------------------------------ May 4, 2005 - 18:37 : chx Attachment: http://drupal.org/files/issues/init.inc (3.46 KB) ------------------------------------------------------------------------ May 4, 2005 - 18:38 : chx Attachment: http://drupal.org/files/issues/settings_ini_set.php (850 bytes) ------------------------------------------------------------------------ May 4, 2005 - 18: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 - 19: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 - 22: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 - 20: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 - 22:12 : chx Attachment: http://drupal.org/files/issues/bootstrap_3.patch (12.36 KB) ------------------------------------------------------------------------ June 4, 2005 - 16: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 - 18: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 - 18: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 - 10: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 - 17: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.) ------------------------------------------------------------------------ June 6, 2005 - 16:19 : moshe weitzman 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. ------------------------------------------------------------------------ June 6, 2005 - 19:56 : Dries - Patch breaks update.php. Might want to double-check cron.php and xmlrpc.php. - The PHPdoc code wraps at variable width/length. Might want to tidy up that a little. ------------------------------------------------------------------------ June 7, 2005 - 21:25 : moshe weitzman in the big case statement in drupal_bootstrap(), perhaps use defined constants instead of 0, 1, 2 ... i think the 4 phase bootstrap is perfect. perhaps move the code in the drupal_bootstrap('all') section to common.inc under a new function called drupal_bootstrap_all(). that makes the code a bit tidier and keeps a few lines out of init.inc which is useful. saves a bit of parsing on requests that don't use the whole framework. you could do that for the other phases as well although the benefit is smaller. ------------------------------------------------------------------------ June 7, 2005 - 21:58 : chx Attachment: http://drupal.org/files/issues/bootstrap_7.patch (20.76 KB) We were already using named phases (not constants), but I have renamed them now as discussed with moshe. Moved the code for the last phase (now called 'full') to common.inc. Simplified index.php -- much simpler than in previous patch, a bit simpler than pre-patch. ------------------------------------------------------------------------ June 7, 2005 - 22:07 : chx Attachment: http://drupal.org/files/issues/bootstrap_8.patch (16.34 KB) Following Moshe's advice to see http://talks.php.net/show/acc_php/33 , I have added a few dots before the paths. ------------------------------------------------------------------------ June 7, 2005 - 22:34 : Bèr Kessels I love the idea of this. +1 for the last (#8) patch from me. I must mention i did not test it thoroughly, hust saw that it works here! The levelled include looks great to me. A minor thing , that I thnik would make this even better, is an additional level for feeds. Feeds need not load all the theme, blocks, and other stuff, only the nodes (items) in the feeds. For reference, my last statistics: Top 30 of 4679 Total URLs # Hits KBytes URL 1 22042 11.11% 2 0.00% /cron.php 2 9418 4.75% 14742 0.60% /favicon.ico 3 6917 3.49% 56598 2.29% /misc/drupal.css ... 13 3272 1.65% 31790 1.29% /node/feed So my main feeds are the 13th most referenced URL on my sites. (the 4-12 contain a lot of odd deeplinked images, and off course /node and / ) ------------------------------------------------------------------------ June 7, 2005 - 23:04 : chx Separating node/feed is not an easy thing, maybe impossible First off, you need most modules as nodeapi is called during this a theme call is possible. I do not see what can be omitted from 'full'. I think this could be a next patch, if we can (which I doubt) an intermediate between 'page cache' and 'full'.

1 0

[drupal-devel] RSS indicater?
by Chris Johnson 08 Jun '05

08 Jun '05

Apparently in the Tiger release of Mac OS, Safari somehow automatically indicates if a web site has an RSS feed. I don't have access to this version of Safari (stuck on Win2k :-( ) so this information is second hand. Anyway, the point is, my Drupal-based sites which do provide RSS feeds do not trigger this indicator. If there is some sort of widespread or semi-standard way for RSS auto-discovery, is that something we should add support for in Drupal? It seems like it would be useful. Some of my users did not know my site had RSS because their browser was not telling them it did. When I told them directly, they were pleased to take advantage of it. -- Chris Johnson

6 7

[drupal-devel] In database enryption.
by Gordon Heydon 08 Jun '05

08 Jun '05

Hello, I have developed a payment gateway for the ecommerce module for use with the St George Bank here in Australia. This uses there batch processing system which means there is user intervention to download and process payments, and then upload back the responses. I have gotten the communication between the webserver and external parties working through the encrypted links, so everything is safe there, but the credit cards are stored in an unencrypted form in the database. What I was thinking of doing is using the mcrypt module for php to encrypt the credit card for storage in the database. The problem that I have come up against is working out which would be the best method of creating the key for the encryption. I have though about using a random number that I then store in the database with a variable_set() which would be better than just using a static key. But if the key is lost then so is all the credit cards that have not been processed. This is not going to be the really secure as someone can just read the program and create small function that will decrypt it anyway, but at least it will stop them for 5 minutes. Any help would be most appreciated. Gordon.

3 3

[drupal-devel] Contract Module
by Nathan Wheatley 08 Jun '05

08 Jun '05

Hello World, Just wanted to run an idea past a few of you for a possible contrib. module for Drupal. I do freelance design work for people all around the world, and the major problem I find is that there is no way to 'sign off' on the work I am asked to do. What I propose is a 'contract' module. Using this, a specialised node, accessible only to select users (the creator of the node, and any other person involved in 'authentication' of the work) would be created. This presumes that the other parties involved are members of the Drupal site, but I feel that is a small requirement/ dependency. Then, all parties (including the node creator) would digitally sign the node (or... its attached file). I imagine the use of MD5, or SHA-1, as they are quite standard (maybe MD5, as it is already used by Drupal). I think that ALL parties should sign it because the client or the employer (node creator) may wish to change things, or, re-read the contract/ agreement. This is how I see the process: document being signed -> hash function -> hash of document -> private key (users password) -> encrypt file -> digital signature. This would be done for each party involved. Like the attachments are shown currently, the module could also show who has signed the document. Notification could also be given to ALL parties when the document has been signed by everyone. I know nothing of the processes used to do this, so if I am wrong, or there is a better way to do this, I would really appreciate it. I'd also love your feedback. I really need this module, but unfortunately don't have the coding experience to make it happen, so, after the details have been sorted out, I will post a bounty for it's development. I would then release it under GPL, back to Drupal of course :P -- Kind Regards, Nathan Wheatley

3 4

[drupal-devel] [bug] Do not execute functions in include files
by chx 08 Jun '05

08 Jun '05

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(a)www.drop.org Updated by: chx Status: patch Separating node/feed is not an easy thing, maybe impossible First off, you need most modules as nodeapi is called during this a theme call is possible. I do not see what can be omitted from 'full'. I think this could be a next patch, if we can (which I doubt) an intermediate between 'page cache' and 'full'. chx Previous comments: ------------------------------------------------------------------------ March 2, 2005 - 00:09 : killes(a)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 - 08: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 - 08: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 - 23:53 : killes(a)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 - 19: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 - 19:37 : chx Attachment: http://drupal.org/files/issues/boot_0.patch (10.31 KB) Hm, settings.php changes were ommited. ------------------------------------------------------------------------ May 4, 2005 - 19:37 : chx Attachment: http://drupal.org/files/issues/init.inc (3.46 KB) ------------------------------------------------------------------------ May 4, 2005 - 19:38 : chx Attachment: http://drupal.org/files/issues/settings_ini_set.php (850 bytes) ------------------------------------------------------------------------ May 4, 2005 - 19: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 - 20: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 - 23: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 - 21: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 - 23:12 : chx Attachment: http://drupal.org/files/issues/bootstrap_3.patch (12.36 KB) ------------------------------------------------------------------------ June 4, 2005 - 17: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 - 19: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 - 19: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 - 11: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 - 18: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.) ------------------------------------------------------------------------ June 6, 2005 - 17:19 : moshe weitzman 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. ------------------------------------------------------------------------ June 6, 2005 - 20:56 : Dries - Patch breaks update.php. Might want to double-check cron.php and xmlrpc.php. - The PHPdoc code wraps at variable width/length. Might want to tidy up that a little. ------------------------------------------------------------------------ June 7, 2005 - 22:25 : moshe weitzman in the big case statement in drupal_bootstrap(), perhaps use defined constants instead of 0, 1, 2 ... i think the 4 phase bootstrap is perfect. perhaps move the code in the drupal_bootstrap('all') section to common.inc under a new function called drupal_bootstrap_all(). that makes the code a bit tidier and keeps a few lines out of init.inc which is useful. saves a bit of parsing on requests that don't use the whole framework. you could do that for the other phases as well although the benefit is smaller. ------------------------------------------------------------------------ June 7, 2005 - 22:58 : chx Attachment: http://drupal.org/files/issues/bootstrap_7.patch (20.76 KB) We were already using named phases (not constants), but I have renamed them now as discussed with moshe. Moved the code for the last phase (now called 'full') to common.inc. Simplified index.php -- much simpler than in previous patch, a bit simpler than pre-patch. ------------------------------------------------------------------------ June 7, 2005 - 23:07 : chx Attachment: http://drupal.org/files/issues/bootstrap_8.patch (16.34 KB) Following Moshe's advice to see http://talks.php.net/show/acc_php/33 , I have added a few dots before the paths. ------------------------------------------------------------------------ June 7, 2005 - 23:34 : Bèr Kessels I love the idea of this. +1 for the last (#8) patch from me. I must mention i did not test it thoroughly, hust saw that it works here! The levelled include looks great to me. A minor thing , that I thnik would make this even better, is an additional level for feeds. Feeds need not load all the theme, blocks, and other stuff, only the nodes (items) in the feeds. For reference, my last statistics: Top 30 of 4679 Total URLs # Hits KBytes URL 1 22042 11.11% 2 0.00% /cron.php 2 9418 4.75% 14742 0.60% /favicon.ico 3 6917 3.49% 56598 2.29% /misc/drupal.css ... 13 3272 1.65% 31790 1.29% /node/feed So my main feeds are the 13th most referenced URL on my sites. (the 4-12 contain a lot of odd deeplinked images, and off course /node and / )

1 0

[drupal-devel] [bug] Do not execute functions in include files
by Bèr Kessels 07 Jun '05

07 Jun '05

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(a)www.drop.org Updated by: Bèr Kessels Status: patch I love the idea of this. +1 for the last (#8) patch from me. I must mention i did not test it thoroughly, hust saw that it works here! The levelled include looks great to me. A minor thing , that I thnik would make this even better, is an additional level for feeds. Feeds need not load all the theme, blocks, and other stuff, only the nodes (items) in the feeds. For reference, my last statistics: Top 30 of 4679 Total URLs # Hits KBytes URL 1 22042 11.11% 2 0.00% /cron.php 2 9418 4.75% 14742 0.60% /favicon.ico 3 6917 3.49% 56598 2.29% /misc/drupal.css ... 13 3272 1.65% 31790 1.29% /node/feed So my main feeds are the 13th most referenced URL on my sites. (the 4-12 contain a lot of odd deeplinked images, and off course /node and / ) Bèr Kessels Previous comments: ------------------------------------------------------------------------ March 1, 2005 - 23:09 : killes(a)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 - 07: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 - 07: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 - 22:53 : killes(a)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 - 18: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 - 18:37 : chx Attachment: http://drupal.org/files/issues/boot_0.patch (10.31 KB) Hm, settings.php changes were ommited. ------------------------------------------------------------------------ May 4, 2005 - 18:37 : chx Attachment: http://drupal.org/files/issues/init.inc (3.46 KB) ------------------------------------------------------------------------ May 4, 2005 - 18:38 : chx Attachment: http://drupal.org/files/issues/settings_ini_set.php (850 bytes) ------------------------------------------------------------------------ May 4, 2005 - 18: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 - 19: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 - 22: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 - 20: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 - 22:12 : chx Attachment: http://drupal.org/files/issues/bootstrap_3.patch (12.36 KB) ------------------------------------------------------------------------ June 4, 2005 - 16: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 - 18: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 - 18: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 - 10: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 - 17: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.) ------------------------------------------------------------------------ June 6, 2005 - 16:19 : moshe weitzman 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. ------------------------------------------------------------------------ June 6, 2005 - 19:56 : Dries - Patch breaks update.php. Might want to double-check cron.php and xmlrpc.php. - The PHPdoc code wraps at variable width/length. Might want to tidy up that a little. ------------------------------------------------------------------------ June 7, 2005 - 21:25 : moshe weitzman in the big case statement in drupal_bootstrap(), perhaps use defined constants instead of 0, 1, 2 ... i think the 4 phase bootstrap is perfect. perhaps move the code in the drupal_bootstrap('all') section to common.inc under a new function called drupal_bootstrap_all(). that makes the code a bit tidier and keeps a few lines out of init.inc which is useful. saves a bit of parsing on requests that don't use the whole framework. you could do that for the other phases as well although the benefit is smaller. ------------------------------------------------------------------------ June 7, 2005 - 21:58 : chx Attachment: http://drupal.org/files/issues/bootstrap_7.patch (20.76 KB) We were already using named phases (not constants), but I have renamed them now as discussed with moshe. Moved the code for the last phase (now called 'full') to common.inc. Simplified index.php -- much simpler than in previous patch, a bit simpler than pre-patch. ------------------------------------------------------------------------ June 7, 2005 - 22:07 : chx Attachment: http://drupal.org/files/issues/bootstrap_8.patch (16.34 KB) Following Moshe's advice to see http://talks.php.net/show/acc_php/33 , I have added a few dots before the paths.

1 0

[drupal-devel] [bug] Do not execute functions in include files
by chx 07 Jun '05

07 Jun '05

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(a)www.drop.org Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/bootstrap_8.patch (16.34 KB) Following Moshe's advice to see http://talks.php.net/show/acc_php/33 , I have added a few dots before the paths. chx Previous comments: ------------------------------------------------------------------------ March 2, 2005 - 00:09 : killes(a)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 - 08: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 - 08: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 - 23:53 : killes(a)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 - 19: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 - 19:37 : chx Attachment: http://drupal.org/files/issues/boot_0.patch (10.31 KB) Hm, settings.php changes were ommited. ------------------------------------------------------------------------ May 4, 2005 - 19:37 : chx Attachment: http://drupal.org/files/issues/init.inc (3.46 KB) ------------------------------------------------------------------------ May 4, 2005 - 19:38 : chx Attachment: http://drupal.org/files/issues/settings_ini_set.php (850 bytes) ------------------------------------------------------------------------ May 4, 2005 - 19: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 - 20: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 - 23: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 - 21: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 - 23:12 : chx Attachment: http://drupal.org/files/issues/bootstrap_3.patch (12.36 KB) ------------------------------------------------------------------------ June 4, 2005 - 17: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 - 19: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 - 19: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 - 11: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 - 18: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.) ------------------------------------------------------------------------ June 6, 2005 - 17:19 : moshe weitzman 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. ------------------------------------------------------------------------ June 6, 2005 - 20:56 : Dries - Patch breaks update.php. Might want to double-check cron.php and xmlrpc.php. - The PHPdoc code wraps at variable width/length. Might want to tidy up that a little. ------------------------------------------------------------------------ June 7, 2005 - 22:25 : moshe weitzman in the big case statement in drupal_bootstrap(), perhaps use defined constants instead of 0, 1, 2 ... i think the 4 phase bootstrap is perfect. perhaps move the code in the drupal_bootstrap('all') section to common.inc under a new function called drupal_bootstrap_all(). that makes the code a bit tidier and keeps a few lines out of init.inc which is useful. saves a bit of parsing on requests that don't use the whole framework. you could do that for the other phases as well although the benefit is smaller. ------------------------------------------------------------------------ June 7, 2005 - 22:58 : chx Attachment: http://drupal.org/files/issues/bootstrap_7.patch (20.76 KB) We were already using named phases (not constants), but I have renamed them now as discussed with moshe. Moved the code for the last phase (now called 'full') to common.inc. Simplified index.php -- much simpler than in previous patch, a bit simpler than pre-patch.

1 0

[drupal-devel] [bug] Do not execute functions in include files
by chx 07 Jun '05

07 Jun '05

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(a)www.drop.org Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/bootstrap_7.patch (20.76 KB) We were already using named phases (not constants), but I have renamed them now as discussed with moshe. Moved the code for the last phase (now called 'full') to common.inc. Simplified index.php -- much simpler than in previous patch, a bit simpler than pre-patch. chx Previous comments: ------------------------------------------------------------------------ March 2, 2005 - 00:09 : killes(a)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 - 08: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 - 08: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 - 23:53 : killes(a)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 - 19: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 - 19:37 : chx Attachment: http://drupal.org/files/issues/boot_0.patch (10.31 KB) Hm, settings.php changes were ommited. ------------------------------------------------------------------------ May 4, 2005 - 19:37 : chx Attachment: http://drupal.org/files/issues/init.inc (3.46 KB) ------------------------------------------------------------------------ May 4, 2005 - 19:38 : chx Attachment: http://drupal.org/files/issues/settings_ini_set.php (850 bytes) ------------------------------------------------------------------------ May 4, 2005 - 19: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 - 20: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 - 23: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 - 21: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 - 23:12 : chx Attachment: http://drupal.org/files/issues/bootstrap_3.patch (12.36 KB) ------------------------------------------------------------------------ June 4, 2005 - 17: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 - 19: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 - 19: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 - 11: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 - 18: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.) ------------------------------------------------------------------------ June 6, 2005 - 17:19 : moshe weitzman 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. ------------------------------------------------------------------------ June 6, 2005 - 20:56 : Dries - Patch breaks update.php. Might want to double-check cron.php and xmlrpc.php. - The PHPdoc code wraps at variable width/length. Might want to tidy up that a little. ------------------------------------------------------------------------ June 7, 2005 - 22:25 : moshe weitzman in the big case statement in drupal_bootstrap(), perhaps use defined constants instead of 0, 1, 2 ... i think the 4 phase bootstrap is perfect. perhaps move the code in the drupal_bootstrap('all') section to common.inc under a new function called drupal_bootstrap_all(). that makes the code a bit tidier and keeps a few lines out of init.inc which is useful. saves a bit of parsing on requests that don't use the whole framework. you could do that for the other phases as well although the benefit is smaller.

1 0

[drupal-devel] [bug] Do not execute functions in include files
by moshe weitzman 07 Jun '05

07 Jun '05

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(a)www.drop.org Updated by: moshe weitzman Status: patch in the big case statement in drupal_bootstrap(), perhaps use defined constants instead of 0, 1, 2 ... i think the 4 phase bootstrap is perfect. perhaps move the code in the drupal_bootstrap('all') section to common.inc under a new function called drupal_bootstrap_all(). that makes the code a bit tidier and keeps a few lines out of init.inc which is useful. saves a bit of parsing on requests that don't use the whole framework. you could do that for the other phases as well although the benefit is smaller. moshe weitzman Previous comments: ------------------------------------------------------------------------ March 1, 2005 - 17:09 : killes(a)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(a)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.) ------------------------------------------------------------------------ June 6, 2005 - 10:19 : moshe weitzman 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. ------------------------------------------------------------------------ June 6, 2005 - 13:56 : Dries - Patch breaks update.php. Might want to double-check cron.php and xmlrpc.php. - The PHPdoc code wraps at variable width/length. Might want to tidy up that a little.

1 0

[drupal-devel] [bug] Form element labels should not show a : if the label ends in a '?'
by rbarreca 07 Jun '05

07 Jun '05

Issue status update for http://drupal.org/node/24482 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: minor Assigned to: Anonymous Reported by: rbarreca Updated by: rbarreca Status: patch Attachment: http://drupal.org/files/issues/patch_no_colon_after_question_mark.patch (710 bytes) Not sure if this goes under theme system or other. When fields end in a '?', the theme_form_element function shows a label like this "Label?:" which looks weird. I added a patch to check if it ends in a question mark, and if so, don't add the colon to the label. I know this function is themeable so I could've done it there, but I feel this should be the default behavior for all those fields like "Are you a smoker?", "Add to front page?", etc. rbarreca

4 3