Description: ------------ push/popErrorHandling() access $this when called staticly, which is a no-no according to the PHP documentation. This causes the functions to fail when called staticly from a non-PEAR-derived class, as they assume that $this is derived from PEAR and try to call $this->setErrorHandling(), which is usually not defined. Reproduce code: --------------- require 'PEAR.php'; class Foo { function Bar() { PEAR::pushErrorHandling(PEAR_ERROR_RETURN); $err = PEAR::raiseError("This will fail."); PEAR::popErrorHandling(); return $err; } } PEAR::setErrorHandling(PEAR_ERROR_DIE); // This works PEAR::pushErrorHandling(PEAR_ERROR_RETURN); PEAR::popErrorHandling(); // This fails $foo = new Foo; $foo->bar(); Expected result: ---------------- the push/popErrorHandling() calls in Foo::Bar() should operate correctly. Actual result: -------------- An error is thrown: Fatal error: Call to undefined function: seterrorhandling() in /usr/ share/php/PEAR.php on line 579

--- PEAR.php.orig Thu Feb 12 12:20:29 2004 +++ PEAR.php Wed Jan 7 12:31:00 2004 @@ -566,7 +566,7 @@ function pushErrorHandling($mode, $options = null) { $stack = &$GLOBALS['_PEAR_error_handler_stack']; - if (isset($this)) { + if (isset($this) && is_a($this, 'PEAR')) { $def_mode = &$this->_default_error_mode; $def_options = &$this->_default_error_options; } else { @@ -575,7 +575,7 @@ } $stack[] = array($def_mode, $def_options); - if (isset($this)) { + if (isset($this) && is_a($this, 'PEAR')) { $this->setErrorHandling($mode, $options); } else { PEAR::setErrorHandling($mode, $options); @@ -600,7 +600,7 @@ array_pop($stack); list($mode, $options) = $stack[sizeof($stack) - 1]; array_pop($stack); - if (isset($this)) { + if (isset($this) && is_a($this, 'PEAR')) { $this->setErrorHandling($mode, $options); } else { PEAR::setErrorHandling($mode, $options);

I already reported this bug last year for PEAR 1.2: http://bugs.php.net/bug.php?id=25714 But the reproduce code I added works with PEAR 1.3 due to the replacement of if (isset($this)) by if (isset($this) && is_a($this, 'PEAR')) But this check isn't safe. What you need here is a check if the function setErrorHandling, pushErrorHandling or popErrorHandling is called staticly! Here's the modified reproduce code to show the bug. Reproduce code: --------------- <?php require_once("PEAR.php"); class Instigator extends PEAR { function doSomething() { PEAR::setErrorHandling(PEAR_ERROR_DIE); // won't work Killer::doSomething(); } } class Killer { function doSomething() { $victim = & new Victim(); $victim->doSomething(); print "He's still alive!"; } } class Victim extends PEAR { function doSomething() { return $this->raiseError("I'm dead!", 0); } } PEAR::setErrorHandling(PEAR_ERROR_RETURN); $instigator = new Instigator(); print "<br>first try: "; $instigator->doSomething(); PEAR::setErrorHandling(PEAR_ERROR_DIE); // works print "<br>second try: "; $instigator->doSomething(); ?> Expected result: ---------------- first try: I'm dead! Actual result: -------------- first try: He's still alive! second try: I'm dead!

Here's a patch that solves the problem for PHP >= 4.3. But from my point of view it's somehow dirty. Other suggestions are welcome. Markus --- PEAR.php 24 Feb 2004 14:55:02 -0000 1.1.12.1 +++ PEAR.php 26 Feb 2004 09:29:01 -0000 @@ -297,7 +297,8 @@ function setErrorHandling($mode = null, $options = null) { - if (isset($this) && is_a($this, 'PEAR')) { + $call = reset(debug_backtrace()); + if (isset($this) && is_a($this, 'PEAR') && $call['type'] == "->") { $setmode = &$this->_default_error_mode; $setoptions = &$this->_default_error_options; } else { @@ -574,7 +575,8 @@ function pushErrorHandling($mode, $options = null) { $stack = &$GLOBALS['_PEAR_error_handler_stack']; - if (isset($this) && is_a($this, 'PEAR')) { + $call = reset(debug_backtrace()); + if (isset($this) && is_a($this, 'PEAR') && $call['type'] == "->") { $def_mode = &$this->_default_error_mode; $def_options = &$this->_default_error_options; } else { @@ -583,7 +585,7 @@ } $stack[] = array($def_mode, $def_options); - if (isset($this) && is_a($this, 'PEAR')) { + if (isset($this) && is_a($this, 'PEAR') && $call['type'] == "->") { $this->setErrorHandling($mode, $options); } else { PEAR::setErrorHandling($mode, $options);

I just noticed that raiseError() has the same problem, p.e if (isset($this) && isset($this->_default_error_mode)) $this is allways defined if you call any method of PEAR.php staticly within a class that extends PEAR. In this case $this->_default_error_mode is also set. This results in absolutly unpredictable behaviour. Markus

this is a basic flaw in the original design of PEAR_Error. There is no good way to fix it and to maintain BC, as you describe. The only truly reliable way to fix is to separate a static version of raiseError(), staticRaiseError() that is always static, and to make raiseError() always an instance method. Greg

I already did something like this in a big project. I extended PEAR and provided methods called setGlobalErrorHandling(), pushGlobalErrorHandling() and popGlobalErrorHandling() as a replacement for static calls of the original methods. What do you think about using the debug_backtrace() hack if php version >= 4.3 The best solution would be a php extension that offers a command to get the information if the current method has been called staticly like debug_backtrace() already does but without it's overhead. Markus