| Package home | Report new bug | New search | Development Roadmap | Status: Open | Feedback | All | Closed Since Version 1.10.15 |
| Bug #744 | push/popErrorHandling() abuse $this | ||
|---|---|---|---|
| Submitted: | 2004-02-12 20:19 UTC | ||
| From: | ieure at php dot net | Assigned: | |
| Status: | Wont fix | Package: | PEAR |
| PHP Version: | Irrelevant | OS: | Linux |
| Roadmaps: | (Not assigned) | ||
[2004-02-12 20:19 UTC] ieure at php dot net
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
Comments
[2004-02-12 20:20 UTC] ieure at php dot net
--- 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);
[2004-02-26 08:33 UTC] mkalkbrenner at arcor dot de
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!
[2004-02-26 09:26 UTC] mkalkbrenner at arcor dot de
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);
[2004-02-26 13:20 UTC] mkalkbrenner at arcor dot de
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
[2004-02-26 13:29 UTC] cellog
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
[2004-02-26 15:24 UTC] mkalkbrenner at arcor dot de
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