[drupal-devel] [bug] Bootstrap patch fixes

Dries drupal-devel at drupal.org
Sun Jul 3 10:10:25 UTC 2005


Issue status update for 
http://drupal.org/node/25600
Post a follow up: 
http://drupal.org/project/comments/add/25600

 Project:      Drupal
 Version:      cvs
 Component:    base system
 Category:     bug reports
 Priority:     critical
 Assigned to:  chx
 Reported by:  kubaZygmunt
 Updated by:   Dries
 Status:       patch

FYI: I committed Bart's update.php patch.  Sorry, it got lost in the
discussion/flow.




Dries



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

June 23, 2005 - 13:28 : kubaZygmunt

Attachment: http://drupal.org/files/issues/bootstrap.diff (422 bytes)

I've found that new cvs version doesn't get properly db_prefix.


In bootstrap.inc (function _drupal_bootstrap($phase) )
I added another global variable $db_prefix, and then it goes .


I don't know if my solution is good because on my local site I can't
see any blocks and on my internet site I have some caching and I can't
see anything :)




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

June 23, 2005 - 14:45 : kubaZygmunt

Above I've put my patch




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

June 23, 2005 - 14:56 : Jaza

Attachment: http://drupal.org/files/issues/db_prefix_global.patch (582 bytes)

Also noticed this bug, when trying to install a fresh CVS snapshot.
Attached is my patch against CVS HEAD (same as kubaZygmunt's,
basically).




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

June 23, 2005 - 15:10 : chx

Yes, this is a bug, if you look at the bootstrap patch, there were
versions when I thought variables in settings.php should be written as
$GLOBALS['something'] but then introduced global $something . Forgot
when that something is db_prefix .




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

June 28, 2005 - 07:26 : Jaza

Can this patch be applied to HEAD please? This is not something that
should be left sitting around in the issues list. Chx has identified
that it was a simple honest mistake on his part, and has indicated that
the submitted patch(es) will fix the mistake.


Until this patch is applied, anyone who tries to install HEAD using db
prefixes will get a fatal error. It doesn't get much more critical than
that :-p.




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

June 30, 2005 - 16:56 : Bart Jansens

Attachment: http://drupal.org/files/issues/20050630.bootstrap-prefix-update.patch (1.21 KB)

There is another related problem, it is no longer possible to override
variables in settings.php.
And update.php no longer works.


Updated patch attached.




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

June 30, 2005 - 17:04 : chx

Attachment: http://drupal.org/files/issues/20050630.bootstrap-patch.patch (2.92 KB)

Oh, and XML-RPC client is broken too. I talked with walkah and we found
it the best to move xmlrpc.inc to modules which use them.




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

June 30, 2005 - 17:09 : chx

Attachment: http://drupal.org/files/issues/20050630.1.bootstrap-patch.patch (3.34 KB)

some more xml-rpc fixes.




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

June 30, 2005 - 17:20 : Steven

Making XML-RPC a conditional include was discussed a while ago, and
decided against due to problems with the class persistency. Why is this
now possible?




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

June 30, 2005 - 19:36 : walkah

basically - it's not. the problem is, well, the xmlrpc.inc is not very
well written. It requires  / depends on several global variables, etc.
which leads to a fair bit of the scoping issues. If you look at what
PEAR has done with this (since their XML-RPC module is based on the
same original code)... you'll notice lots of $GLOBALS['...'] usage (see
http://cvs.php.net/co.php/pear/XML_RPC/RPC.php) . Basically, I think we
have 3 options:


1) keep banging on this old code to make it work.
2) switch libraries to either PEAR::XML-RPC or something like incutio's
(http://scripts.incutio.com/xmlrpc/)
3) re-write our own custom xml-rpc library 


personally, I'd +1 #2 - since #1 is ugly, and #3 I personally don't
have time for, and I'd rather see our developer resources going to more
interesting things.


that said, i know there are those with strong feelings against 3rd
party libraries... but this needs to get fixed quickly. blogapi and
drupal dist_auth are both currently broken in HEAD. (isn't there anyone
else here that tests XML-RPC stuff??)




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

June 30, 2005 - 19:38 : walkah

just to follow up.. chx's patch in #7 only fixes XMLRPC servers...
anything that uses XML-RPC client (i.e. drupal_auth or ping) will still
be busted.




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

June 30, 2005 - 19:48 : moshe weitzman

the 'we shouldn't use 3rd party libraries' argument is hogwash.  that
was recenyly followed up with 'look where that got us recently' which
is even more stupid. security problems happen in your own code, and in
other people's code. we've had several security mistakes of our own,
and those mistakes have persisted a long time.


the benefit of 3rd party libraries is that you don't have to write
them, debug them, document them, or maintain them. if someone here is
willing to do all of this for XMLRPC then great. But thats extremely
unlikely so we either have to drop the whole feature set or use a 3rd
party library.


as you can tell, i favor #2 as well - switching libraries so we can do
conditional inclusion.




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

June 30, 2005 - 20:01 : Dries

#2 isn't GPL




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

June 30, 2005 - 20:07 : Dries

Looks like WordPress might be using that class though.  Weird.  I know
Wordpress had a least two security issues with their XML-RPC library
(and they aren't using Edd Dumbill's).  There is no word about this at
http://scripts.incutio.com/xmlrpc/ so it looks like it isn't maintained
either (which renders the advantages of using third-party libraries
moot-ish).




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

June 30, 2005 - 20:17 : Dries

The WP developers confirmed that they are using this library.


Turns out there is a new website for it too:
http://simon.incutio.com/archive/2005/05/23/ixr.  WordPress' version
lives at http://trac.wordpress.org/log/trunk/wp-includes/class-IXR.php.
 Some extra details at http://trac.wordpress.org/ticket/1400.  


Looking at the code, it doesn't appear to be PHP5 compliant.  Plus, I'm
not sure the license is compatible.




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

June 30, 2005 - 20:21 : chx

I will rewrite the Incutio XML-RPC library for our own usage in July, I
know this for quite a long time.


a) I intend to start with Incutio's XML-RPC
b) I contacted the author already and he is fine with GPL'ing the
result.
c) the result will be quite a bit different from the original, 'cos
I'll make it use procedural notation and overall, more Drupalish.




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

June 30, 2005 - 20:31 : chx

"
From: Simon Willison
To: Karoly Negyesi
Subject: Re: IXR GPL?
Date: 05/25/05 18:40


On 25 May 2005, at 17:35, Karoly Negyesi wrote:
> I'd like to get rid of the current XML-RPC implementation in Drupal
 
> and use
> IXR instead. Yours seems much more transparent and up to date.
>
> I plan to rewrite it a bit so that it looses OOP, this enables us to
> conditionally include.
>
> However, Drupal is GPL and I need IXR under GPL to work with.


Hmm... since you'll be rewriting it a bit, how's about I just give  
you permission to use it and relicense it as GPL provided you stick a
 
note that it was derived (with permission) from IXR in a comment at  
the top somewhere?


I really hate software licensing - I just want people to be able to  
use my code!


Thanks,


Simon


"


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

July 2, 2005 - 21:45 : chx

Dries, while the xmlrpc rewrite is under way please commit Bart's patch
http://drupal.org/node/25600#comment-34362




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

July 3, 2005 - 00:14 : mathias

I've tested out Bart's patch and it does indeed restore update.php and
allow $conf overrides. Might be worth fixing this for now while we
continue to work on a long term solution.




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

July 3, 2005 - 11:36 : Dries

I think error handling is broken too.  On drupal.org, PHP
warnings/errors show up on pages while they are configured only to show
up in the logs. They no longer show up in logs; it looks like the call
to set_error_handler() didn't "stick" ...




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

July 3, 2005 - 12:07 : Dries

update.php is broken too.  


chx: if you can't come up fixes within 24 hour, I'll have to rollback
this patch for the time being.  It's a bottleneck problem.







More information about the drupal-devel mailing list