From b83df13d07cee852521e23915283e80bba6cbed4 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sat, 24 Mar 2012 12:52:15 +0100 Subject: [PATCH 1/3] Allow resetting the error handler This allows the error handler to be reset using set_error_handler(null). As the code suggests this behavior was already previously intended, but the callback check was done too strictly. --- Zend/tests/bug60738.phpt | 17 +++++++++++++++++ Zend/zend_builtin_functions.c | 14 ++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) create mode 100644 Zend/tests/bug60738.phpt diff --git a/Zend/tests/bug60738.phpt b/Zend/tests/bug60738.phpt new file mode 100644 index 0000000000000..e0c9793fedf27 --- /dev/null +++ b/Zend/tests/bug60738.phpt @@ -0,0 +1,17 @@ +--TEST-- +Bug #60738 Allow 'set_error_handler' to handle NULL +--FILE-- + +--EXPECTF-- +Intercepted error! + +Notice: Error! in %s on line %d diff --git a/Zend/zend_builtin_functions.c b/Zend/zend_builtin_functions.c index 8d39a31ad0cd1..5b3a3cbca492c 100644 --- a/Zend/zend_builtin_functions.c +++ b/Zend/zend_builtin_functions.c @@ -1421,13 +1421,15 @@ ZEND_FUNCTION(set_error_handler) return; } - if (!zend_is_callable(error_handler, 0, &error_handler_name TSRMLS_CC)) { - zend_error(E_WARNING, "%s() expects the argument (%s) to be a valid callback", - get_active_function_name(TSRMLS_C), error_handler_name?error_handler_name:"unknown"); + if (Z_TYPE_P(error_handler) != IS_NULL) { /* NULL == unset */ + if (!zend_is_callable(error_handler, 0, &error_handler_name TSRMLS_CC)) { + zend_error(E_WARNING, "%s() expects the argument (%s) to be a valid callback", + get_active_function_name(TSRMLS_C), error_handler_name?error_handler_name:"unknown"); + efree(error_handler_name); + return; + } efree(error_handler_name); - return; } - efree(error_handler_name); if (EG(user_error_handler)) { had_orig_error_handler = 1; @@ -1439,7 +1441,7 @@ ZEND_FUNCTION(set_error_handler) } ALLOC_ZVAL(EG(user_error_handler)); - if (!zend_is_true(error_handler)) { /* unset user-defined handler */ + if (Z_TYPE_P(error_handler) == IS_NULL) { /* unset user-defined handler */ FREE_ZVAL(EG(user_error_handler)); EG(user_error_handler) = NULL; RETURN_TRUE; From de8d30b0a50ca610303e44434317b17a735606a8 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sat, 24 Mar 2012 13:10:51 +0100 Subject: [PATCH 2/3] Return previous error handler when resetting the error handler set_error_handler(null) and set_exception_handler(null) now return the previous error/exception handler instead of just returning bool(true). This is consistent with the behavior of these functions with non-null values. --- Zend/tests/bug60738.phpt | 9 +++++++-- Zend/tests/bug60738_variation.phpt | 23 +++++++++++++++++++++++ Zend/zend_builtin_functions.c | 4 ++-- 3 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 Zend/tests/bug60738_variation.phpt diff --git a/Zend/tests/bug60738.phpt b/Zend/tests/bug60738.phpt index e0c9793fedf27..e4080715ec72a 100644 --- a/Zend/tests/bug60738.phpt +++ b/Zend/tests/bug60738.phpt @@ -3,15 +3,20 @@ Bug #60738 Allow 'set_error_handler' to handle NULL --FILE-- --EXPECTF-- +NULL Intercepted error! +object(Closure)#1 (0) { +} Notice: Error! in %s on line %d diff --git a/Zend/tests/bug60738_variation.phpt b/Zend/tests/bug60738_variation.phpt new file mode 100644 index 0000000000000..d7cf00ecdbc48 --- /dev/null +++ b/Zend/tests/bug60738_variation.phpt @@ -0,0 +1,23 @@ +--TEST-- +Bug #60738 Allow 'set_error_handler' to handle NULL +--FILE-- + +--EXPECTF-- +NULL +object(Closure)#1 (0) { +} + +Fatal error: Uncaught exception 'Exception' with message 'Exception!' in %s:%d +Stack trace: +#0 {main} + thrown in %s on line %d + diff --git a/Zend/zend_builtin_functions.c b/Zend/zend_builtin_functions.c index 5b3a3cbca492c..d1f85107535d3 100644 --- a/Zend/zend_builtin_functions.c +++ b/Zend/zend_builtin_functions.c @@ -1444,7 +1444,7 @@ ZEND_FUNCTION(set_error_handler) if (Z_TYPE_P(error_handler) == IS_NULL) { /* unset user-defined handler */ FREE_ZVAL(EG(user_error_handler)); EG(user_error_handler) = NULL; - RETURN_TRUE; + return; } EG(user_error_handler_error_reporting) = (int)error_type; @@ -1515,7 +1515,7 @@ ZEND_FUNCTION(set_exception_handler) if (Z_TYPE_P(exception_handler) == IS_NULL) { /* unset user-defined handler */ FREE_ZVAL(EG(user_exception_handler)); EG(user_exception_handler) = NULL; - RETURN_TRUE; + return; } MAKE_COPY_ZVAL(&exception_handler, EG(user_exception_handler)) From 1e9e69f4bc2033018165020cbf3b6fb091be22c6 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sat, 24 Mar 2012 13:48:11 +0100 Subject: [PATCH 3/3] Simplify set_error_handler/set_exception_handler code --- Zend/zend_builtin_functions.c | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/Zend/zend_builtin_functions.c b/Zend/zend_builtin_functions.c index d1f85107535d3..ab79ec14f3bdd 100644 --- a/Zend/zend_builtin_functions.c +++ b/Zend/zend_builtin_functions.c @@ -1413,7 +1413,6 @@ ZEND_FUNCTION(trigger_error) ZEND_FUNCTION(set_error_handler) { zval *error_handler; - zend_bool had_orig_error_handler=0; char *error_handler_name = NULL; long error_type = E_ALL | E_STRICT; @@ -1432,29 +1431,20 @@ ZEND_FUNCTION(set_error_handler) } if (EG(user_error_handler)) { - had_orig_error_handler = 1; - *return_value = *EG(user_error_handler); - zval_copy_ctor(return_value); - INIT_PZVAL(return_value); + RETVAL_ZVAL(EG(user_error_handler), 1, 0); + zend_stack_push(&EG(user_error_handlers_error_reporting), &EG(user_error_handler_error_reporting), sizeof(EG(user_error_handler_error_reporting))); zend_ptr_stack_push(&EG(user_error_handlers), EG(user_error_handler)); } - ALLOC_ZVAL(EG(user_error_handler)); if (Z_TYPE_P(error_handler) == IS_NULL) { /* unset user-defined handler */ - FREE_ZVAL(EG(user_error_handler)); EG(user_error_handler) = NULL; return; } + ALLOC_ZVAL(EG(user_error_handler)); + MAKE_COPY_ZVAL(&error_handler, EG(user_error_handler)); EG(user_error_handler_error_reporting) = (int)error_type; - *EG(user_error_handler) = *error_handler; - zval_copy_ctor(EG(user_error_handler)); - INIT_PZVAL(EG(user_error_handler)); - - if (!had_orig_error_handler) { - RETURN_NULL(); - } } /* }}} */ @@ -1488,7 +1478,6 @@ ZEND_FUNCTION(set_exception_handler) { zval *exception_handler; char *exception_handler_name = NULL; - zend_bool had_orig_exception_handler=0; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "z", &exception_handler) == FAILURE) { return; @@ -1505,24 +1494,18 @@ ZEND_FUNCTION(set_exception_handler) } if (EG(user_exception_handler)) { - had_orig_exception_handler = 1; - *return_value = *EG(user_exception_handler); - zval_copy_ctor(return_value); + RETVAL_ZVAL(EG(user_exception_handler), 1, 0); + zend_ptr_stack_push(&EG(user_exception_handlers), EG(user_exception_handler)); } - ALLOC_ZVAL(EG(user_exception_handler)); if (Z_TYPE_P(exception_handler) == IS_NULL) { /* unset user-defined handler */ - FREE_ZVAL(EG(user_exception_handler)); EG(user_exception_handler) = NULL; return; } + ALLOC_ZVAL(EG(user_exception_handler)); MAKE_COPY_ZVAL(&exception_handler, EG(user_exception_handler)) - - if (!had_orig_exception_handler) { - RETURN_NULL(); - } } /* }}} */