Bug #744

push/popErrorHandling() abuse $this

Details

Submitted	2004-02-12 20:19 UTC
From	ieure at php dot net
Status	Wont fix
Package	PEAR
PHP Version	Irrelevant
OS	Linux
Roadmaps	(Not assigned)

Comments

[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

[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 @@

- 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 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