ID: 23619
Comment by: wboring at qualys dot com
Reported By: ldemailly at qualys dot com
Status: Closed
Bug Type: Scripting Engine problem
Operating System: Linux 2.4.20
PHP Version: 4CVS-2003-05-13 (stable)
New Comment:
I tested this with php 4.3.1 and found that the code assumes that what
is passed in to set_error_handler() is a string, which you can clearly
see by the line
...
if (Z_STRLEN_PP(error_handler)==0) { /* unset user-defined
...
This is wrong and does not work when you pass an array into
set_error_handler( array(&$this, "myhandler") );
error_handler is an array zval, and therefore the test for Z_STRLEN_PP
always returns 0. This case falls through and sets the
user_error_handler = NULL;
which prevents the user defined handler from working at all. I am able
to reproduce this 100% of the time. Please fix this, as it is totally
broken in its current state. The patch provided works.
Also, if you assume that the variable passed in is a string, and
happens to be empty, then why do we RETURN_TRUE; in that block, when in
fact the user_error_handler is NOT set, and is set to NULL. This is
also wrong. This is an error case which should return FALSE.
Previous Comments:
------------------------------------------------------------------------
[2003-05-15 12:06:20] sniper@php.net
Seems to work fine with current stable CVS.
(without the patch, which is bogus anyway.)
------------------------------------------------------------------------
[2003-05-14 16:18:14] ldemailly at qualys dot com
A collegue of mine (Thanks Walter!) here actually found
a fix (patch below);
looking at the code it is puzzling that it would
ever work, there seems to be some other bug/corruption
in the zend engine that would explain why it works/
behave differently sometimes. Also that test for empty
string should probably be before the test for callable... ?
php-4.3.1-walt/Zend/zend_builtin_functions.c
--- php-4.3.1/Zend/zend_builtin_functions.c 2002-11-27
12:11:10.000000000 -0800
+++ php-4.3.1-walt/Zend/zend_builtin_functions.c 2003-05-14
13:15:39.000000000 -0700
@@ -891,7 +891,8 @@
}
ALLOC_ZVAL(EG(user_error_handler));
- if (Z_STRLEN_PP(error_handler)==0) { /* unset user-defined
handler */
+ /* Make sure we test for an empty string on a non-array value
here
*/
+ if (Z_TYPE_PP(error_handler) == IS_STRING &&
Z_STRLEN_PP(error_handler)==0) { /* unset user-defined handler */
FREE_ZVAL(EG(user_error_handler));
EG(user_error_handler) = NULL;
RETURN_TRUE;
------------------------------------------------------------------------
[2003-05-14 14:01:07] ldemailly at qualys dot com
Aha, I managed to reduce the repro script dramatically :
<?php
ini_set("error_reporting" , E_ALL);
class TestErrorHdl {
// Will not be called !!
function errorHandler($errno, $errstr, $errfile, $errline) {
echo("ERROR HANDLER CALLED (EXPECTED)\n");
}
function runTest() {
$res = set_error_handler(array(&$this, "errorHandler"));
echo("set_error_handler done retcode=" . $res . "\n");
$bar = $foo;
}
}
$t = new TestErrorHdl();
$t->runTest();
?>
--- output (buggy, with current 4.3 php cli versions): ----
$ php ErrorBug.php
set_error_handler done retcode=1
Notice: Undefined variable: foo in /home/dl/test_err_tiny/ErrorBug.php
on line
12
--- output (expected) : ----
set_error_handler done retcode=1
ERROR HANDLER CALLED (EXPECTED)
[note that with a longer script it does *most of the time*
work as expected, so it's not like the feature of
having an instance method for a error handler is not
supported at all...]
------------------------------------------------------------------------
[2003-05-14 10:42:42] sniper@php.net
You're propably doing something wrong. As long as you can't provide a
_SHORT_ example script which clearly shows the problem, we'll assume
that.
------------------------------------------------------------------------
[2003-05-13 21:26:28] ldemailly at qualys dot com
set_error_handler registered not called:
very nasty/triky bug to nail/reproduce:
I'm now at the point where if I try to minimize
further the test scripts, the bug goes away:
for instance, removing the "require q.inc" where
q.inc is an empty file, or changing code that
is *after* the offending code makes the bug go
away (hides it), so I suspect some nasty problem
in the engine, because it seems memory usage and/or
complexity of the script to be parsed affects the outcome.
to repro: copy paste, uudecode; gtar xvfz test_err.tgz
cd test_err; php ErrorBug.php
it will output "INSIDE test_should_error_out()"
but not "ERROR HANDLER CALLED" as it should if
you look at PHPUnit/TestCase.php line 158 and 147
the error is raised (as the Notice: Undefined variable: foo in
/home/dl/dev/cvs/qualys/web/dev/current/internal/test_err/ErrorBug.php
on line 7 demonstrates) but the handler is not called...
if you edit BugTest1.php to remove the require 'q.inc' for
instance, you will now see "ERROR HANDLER CALLED"
even though q.inc is empty and should thus make no difference at all !
ps: the code has been modified from pear's phpunit module with a
submitted patch for error handling; which works 'sometimes' (most of
the time actually). in the tgz below, I removed a lot of stuff to make
the files as small as possible... (incl headers/copyright notices,
etc...))
I spent many many hours trying to narrow it down further
(having a simpler script) but didn't manage, sorry for
having a large set of files needed ...
This is with the latest cvs (2003-05-13) on redhat linux 9
next message will be the .uu, if you're having any
trouble, please email and I'll (re)send the repro scripts
------------------------------------------------------------------------
--
Edit this bug report at http://bugs.php.net/?id=23619&edit=1