Skip to content

Commit 0425a66

Browse files
SammyKSammyK
committed
Fire open observer end handlers after a zend_bailout
Closes GH-6377
1 parent 3fb1b08 commit 0425a66

File tree

7 files changed

+176
-1
lines changed

7 files changed

+176
-1
lines changed

‎Zend/zend_observer.c‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,13 @@ zend_llist zend_observer_error_callbacks;
4444
int zend_observer_fcall_op_array_extension = -1;
4545

4646
ZEND_TLS zend_arena *fcall_handlers_arena = NULL;
47+
ZEND_TLS zend_execute_data *first_observed_frame = NULL;
48+
ZEND_TLS zend_execute_data *current_observed_frame = NULL;
4749

4850
// Call during minit/startup ONLY
4951
ZEND_API void zend_observer_fcall_register(zend_observer_fcall_init init) {
50-
/* We don't want to get an extension handle unless an ext installs an observer */
5152
if (!ZEND_OBSERVER_ENABLED) {
53+
/* We don't want to get an extension handle unless an ext installs an observer */
5254
zend_observer_fcall_op_array_extension =
5355
zend_get_op_array_extension_handle("Zend Observer");
5456

@@ -160,6 +162,11 @@ static void ZEND_FASTCALL _zend_observe_fcall_begin(zend_execute_data *execute_d
160162
return;
161163
}
162164

165+
if (first_observed_frame == NULL) {
166+
first_observed_frame = execute_data;
167+
}
168+
current_observed_frame = execute_data;
169+
163170
end = fcall_data->end;
164171
for (handlers = fcall_data->handlers; handlers != end; ++handlers) {
165172
if (handlers->begin) {
@@ -208,6 +215,25 @@ ZEND_API void ZEND_FASTCALL zend_observer_fcall_end(
208215
handlers->end(execute_data, return_value);
209216
}
210217
}
218+
219+
if (first_observed_frame == execute_data) {
220+
first_observed_frame = NULL;
221+
current_observed_frame = NULL;
222+
} else {
223+
current_observed_frame = execute_data->prev_execute_data;
224+
}
225+
}
226+
227+
ZEND_API void zend_observer_fcall_end_all(void)
228+
{
229+
zend_execute_data *ex = current_observed_frame;
230+
while (ex != NULL) {
231+
if (ex->func->type != ZEND_INTERNAL_FUNCTION) {
232+
zend_observer_fcall_end(ex, NULL);
233+
}
234+
ex = ex->prev_execute_data;
235+
}
236+
current_observed_frame = NULL;
211237
}
212238

213239
ZEND_API void zend_observer_error_register(zend_observer_error_cb cb)

‎Zend/zend_observer.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ ZEND_API void ZEND_FASTCALL zend_observer_fcall_end(
7070
zend_execute_data *execute_data,
7171
zval *return_value);
7272

73+
ZEND_API void zend_observer_fcall_end_all(void);
74+
7375
typedef void (*zend_observer_error_cb)(int type, const char *error_filename, uint32_t error_lineno, zend_string *message);
7476

7577
ZEND_API void zend_observer_error_register(zend_observer_error_cb callback);

‎ext/zend_test/tests/observer_error_01.phpt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
Observer: End handlers fire after a fatal error
3+
--SKIPIF--
4+
<?php if (!extension_loaded('zend-test')) die('skip: zend-test extension required'); ?>
5+
--INI--
6+
zend_test.observer.enabled=1
7+
zend_test.observer.observe_all=1
8+
zend_test.observer.show_return_value=1
9+
memory_limit=1M
10+
--FILE--
11+
<?php
12+
function foo()
13+
{
14+
str_repeat('.', 1024 * 1024 * 2); // 2MB
15+
}
16+
17+
foo();
18+
19+
echo 'You should not see this.';
20+
?>
21+
--EXPECTF--
22+
<!-- init '%s/observer_error_%d.php' -->
23+
<file '%s/observer_error_%d.php'>
24+
<!-- init foo() -->
25+
<foo>
26+
27+
Fatal error: Allowed memory size of 2097152 bytes exhausted%s(tried to allocate %d bytes) in %s on line %d
28+
</foo:NULL>
29+
</file '%s/observer_error_%d.php'>

‎ext/zend_test/tests/observer_error_02.phpt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
Observer: End handlers fire after a userland fatal error
3+
--SKIPIF--
4+
<?php if (!extension_loaded('zend-test')) die('skip: zend-test extension required'); ?>
5+
--INI--
6+
zend_test.observer.enabled=1
7+
zend_test.observer.observe_all=1
8+
zend_test.observer.show_return_value=1
9+
--FILE--
10+
<?php
11+
function foo()
12+
{
13+
trigger_error('Foo error', E_USER_ERROR);
14+
}
15+
16+
foo();
17+
18+
echo 'You should not see this.';
19+
?>
20+
--EXPECTF--
21+
<!-- init '%s/observer_error_%d.php' -->
22+
<file '%s/observer_error_%d.php'>
23+
<!-- init foo() -->
24+
<foo>
25+
26+
Fatal error: Foo error in %s on line %d
27+
</foo:NULL>
28+
</file '%s/observer_error_%d.php'>

‎ext/zend_test/tests/observer_error_03.phpt‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
Observer: non-fatal errors do not fire end handlers prematurely
3+
--SKIPIF--
4+
<?php if (!extension_loaded('zend-test')) die('skip: zend-test extension required'); ?>
5+
--INI--
6+
zend_test.observer.enabled=1
7+
zend_test.observer.observe_all=1
8+
zend_test.observer.show_return_value=1
9+
--FILE--
10+
<?php
11+
function foo()
12+
{
13+
return $this_does_not_exit; // E_WARNING
14+
}
15+
16+
function main()
17+
{
18+
foo();
19+
echo 'After error.' . PHP_EOL;
20+
}
21+
22+
main();
23+
24+
echo 'Done.' . PHP_EOL;
25+
?>
26+
--EXPECTF--
27+
<!-- init '%s/observer_error_%d.php' -->
28+
<file '%s/observer_error_%d.php'>
29+
<!-- init main() -->
30+
<main>
31+
<!-- init foo() -->
32+
<foo>
33+
34+
Warning: Undefined variable $this_does_not_exit in %s on line %d
35+
</foo:NULL>
36+
After error.
37+
</main:NULL>
38+
Done.
39+
</file '%s/observer_error_%d.php'>

‎ext/zend_test/tests/observer_error_04.phpt‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--TEST--
2+
Observer: fatal errors caught with zend_try will not fire end handlers prematurely
3+
--SKIPIF--
4+
<?php if (!extension_loaded('zend-test')) die('skip: zend-test extension required'); ?>
5+
<?php if (!extension_loaded('soap')) die('skip: soap extension required'); ?>
6+
--INI--
7+
zend_test.observer.enabled=1
8+
zend_test.observer.observe_all=1
9+
zend_test.observer.show_return_value=1
10+
--FILE--
11+
<?php
12+
function foo()
13+
{
14+
// ext/soap catches a zend_bailout and then throws an exception
15+
$client = new SoapClient('foo');
16+
}
17+
18+
function main()
19+
{
20+
foo();
21+
}
22+
23+
// try/catch is on main() to ensure ZEND_HANDLE_EXCEPTION does not fire the end handlers again
24+
try {
25+
main();
26+
} catch (SoapFault $e) {
27+
echo $e->getMessage() . PHP_EOL;
28+
}
29+
30+
echo 'Done.' . PHP_EOL;
31+
?>
32+
--EXPECTF--
33+
<!-- init '%s/observer_error_%d.php' -->
34+
<file '%s/observer_error_%d.php'>
35+
<!-- init main() -->
36+
<main>
37+
<!-- init foo() -->
38+
<foo>
39+
<!-- Exception: SoapFault -->
40+
</foo:NULL>
41+
<!-- Exception: SoapFault -->
42+
</main:NULL>
43+
SOAP-ERROR: Parsing WSDL: Couldn't load from 'foo' : failed to load external entity "foo"
44+
45+
Done.
46+
</file '%s/observer_error_%d.php'>

‎main/main.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1740,6 +1740,11 @@ void php_request_shutdown(void *dummy)
17401740

17411741
php_deactivate_ticks();
17421742

1743+
/* 0. Call any open observer end handlers that are still open after a zend_bailout */
1744+
if (ZEND_OBSERVER_ENABLED) {
1745+
zend_observer_fcall_end_all();
1746+
}
1747+
17431748
/* 1. Call all possible shutdown functions registered with register_shutdown_function() */
17441749
if (PG(modules_activated)) {
17451750
php_call_shutdown_functions();

0 commit comments

Comments
 (0)