From 187b97060f2ba6010e9dbab564e8af8241248c41 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 21 Aug 2015 15:55:52 +0200 Subject: [PATCH 01/11] Stop JS execution on PHP exceptions, refs #144 --- tests/exception_propagation_2.phpt | 1 + tests/php_exceptions_001.phpt | 50 ++++++++++++++++++++++ tests/php_exceptions_002.phpt | 67 ++++++++++++++++++++++++++++++ tests/php_exceptions_basic.phpt | 42 +++++++++++++++++++ v8js_methods.cc | 15 +------ v8js_object_export.cc | 6 ++- v8js_timer.cc | 4 +- v8js_v8.cc | 17 ++++++-- v8js_v8.h | 2 +- 9 files changed, 183 insertions(+), 21 deletions(-) create mode 100644 tests/php_exceptions_001.phpt create mode 100644 tests/php_exceptions_002.phpt create mode 100644 tests/php_exceptions_basic.phpt diff --git a/tests/exception_propagation_2.phpt b/tests/exception_propagation_2.phpt index a822eda..c12830a 100644 --- a/tests/exception_propagation_2.phpt +++ b/tests/exception_propagation_2.phpt @@ -1,6 +1,7 @@ --TEST-- Test V8::executeString() : Exception propagation test 2 --SKIPIF-- +SKIP needs discussion, see issue #144 --FILE-- +--FILE-- +foo = new \Foo(); + +$JS = <<< EOT +try { + PHP.foo.throwException(); + // the exception should abort further execution, + // hence the print must not pop up + print("after throwException\\n"); +} catch(e) { + // JS should not catch in default mode + print("JS caught exception"); +} +EOT; + +for($i = 0; $i < 5; $i ++) { + var_dump($i); + try { + $v8->executeString($JS); + } catch (Exception $e) { + var_dump($e->getMessage()); + } +} +?> +===EOF=== +--EXPECTF-- +int(0) +string(14) "Test-Exception" +int(1) +string(14) "Test-Exception" +int(2) +string(14) "Test-Exception" +int(3) +string(14) "Test-Exception" +int(4) +string(14) "Test-Exception" +===EOF=== diff --git a/tests/php_exceptions_002.phpt b/tests/php_exceptions_002.phpt new file mode 100644 index 0000000..2ddac8d --- /dev/null +++ b/tests/php_exceptions_002.phpt @@ -0,0 +1,67 @@ +--TEST-- +Test V8::executeString() : PHP Exception handling (multi-level) +--SKIPIF-- + +--FILE-- +foo = new \Foo(); + +$work = $v8->executeString(<<getMessage()); + } +} +?> +===EOF=== +--EXPECT-- +int(0) +string(14) "Test-Exception" +int(1) +recurse[0] ... +string(14) "Test-Exception" +int(2) +recurse[1] ... +recurse[0] ... +string(14) "Test-Exception" +int(3) +recurse[2] ... +recurse[1] ... +recurse[0] ... +string(14) "Test-Exception" +int(4) +recurse[3] ... +recurse[2] ... +recurse[1] ... +recurse[0] ... +string(14) "Test-Exception" +===EOF=== diff --git a/tests/php_exceptions_basic.phpt b/tests/php_exceptions_basic.phpt new file mode 100644 index 0000000..1ddc678 --- /dev/null +++ b/tests/php_exceptions_basic.phpt @@ -0,0 +1,42 @@ +--TEST-- +Test V8::executeString() : PHP Exception handling (basic) +--SKIPIF-- + +--FILE-- +foo = new \Foo(); + +$JS = <<< EOT +try { + PHP.foo.throwException(); + // the exception should abort further execution, + // hence the print must not pop up + print("after throwException\\n"); +} catch(e) { + // JS should not catch in default mode + print("JS caught exception"); +} +EOT; + +try { + $v8->executeString($JS); +} catch (Exception $e) { + var_dump($e->getMessage()); + var_dump($e->getFile()); + var_dump($e->getLine()); +} +?> +===EOF=== +--EXPECTF-- +string(14) "Test-Exception" +string(%d) "%sphp_exceptions_basic.php" +int(5) +===EOF=== diff --git a/v8js_methods.cc b/v8js_methods.cc index 2bd9013..722736b 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -26,20 +26,7 @@ extern "C" { V8JS_METHOD(exit) /* {{{ */ { v8::Isolate *isolate = info.GetIsolate(); - - /* Unfortunately just calling TerminateExecution on the isolate is not - * enough, since v8 just marks the thread as "to be aborted" and doesn't - * immediately do so. Hence we enter an endless loop after signalling - * termination, so we definitely don't execute JS code after the exit() - * statement. */ - v8::Locker locker(isolate); - v8::Isolate::Scope isolate_scope(isolate); - v8::HandleScope handle_scope(isolate); - - v8::Local source = V8JS_STR("for(;;);"); - v8::Local script = v8::Script::Compile(source); - v8::V8::TerminateExecution(isolate); - script->Run(); + v8js_terminate_execution(isolate); } /* }}} */ diff --git a/v8js_object_export.cc b/v8js_object_export.cc index e108740..80ba129 100644 --- a/v8js_object_export.cc +++ b/v8js_object_export.cc @@ -134,11 +134,15 @@ static void v8js_call_php_func(zval *value, zend_class_entry *ce, zend_function isolate->Enter(); } zend_catch { - v8::V8::TerminateExecution(isolate); + v8js_terminate_execution(isolate); V8JSG(fatal_error_abort) = 1; } zend_end_try(); + if(EG(exception)) { + v8js_terminate_execution(isolate); + } + failure: /* Cleanup */ if (argc) { diff --git a/v8js_timer.cc b/v8js_timer.cc index 11ea08e..d556716 100644 --- a/v8js_timer.cc +++ b/v8js_timer.cc @@ -55,7 +55,7 @@ static void v8js_timer_interrupt_handler(v8::Isolate *isolate, void *data) { /* if (timer_ctx->memory_limit > 0 && hs.used_heap_size() > timer_ctx->memory_limit) { timer_ctx->killed = true; - v8js_terminate_execution(c TSRMLS_CC); + v8::V8::TerminateExecution(c->isolate); c->memory_limit_hit = true; } } @@ -80,7 +80,7 @@ void v8js_timer_thread(TSRMLS_D) /* {{{ */ } else if(timer_ctx->time_limit > 0 && now > timer_ctx->time_point) { timer_ctx->killed = true; - v8js_terminate_execution(c TSRMLS_CC); + v8::V8::TerminateExecution(c->isolate); c->time_limit_hit = true; } else if (timer_ctx->memory_limit > 0) { diff --git a/v8js_v8.cc b/v8js_v8.cc index a202c8a..12e413d 100644 --- a/v8js_v8.cc +++ b/v8js_v8.cc @@ -199,10 +199,21 @@ void v8js_v8_call(v8js_ctx *c, zval **return_value, } /* }}} */ -void v8js_terminate_execution(v8js_ctx *c TSRMLS_DC) /* {{{ */ +void v8js_terminate_execution(v8::Isolate *isolate) /* {{{ */ { - // Forcefully terminate the current thread of V8 execution in the isolate - v8::V8::TerminateExecution(c->isolate); + /* Unfortunately just calling TerminateExecution on the isolate is not + * enough, since v8 just marks the thread as "to be aborted" and doesn't + * immediately do so. Hence we enter an endless loop after signalling + * termination, so we definitely don't execute JS code after the exit() + * statement. */ + v8::Locker locker(isolate); + v8::Isolate::Scope isolate_scope(isolate); + v8::HandleScope handle_scope(isolate); + + v8::Local source = V8JS_STR("for(;;);"); + v8::Local script = v8::Script::Compile(source); + v8::V8::TerminateExecution(isolate); + script->Run(); } /* }}} */ diff --git a/v8js_v8.h b/v8js_v8.h index 1e91970..e57d6f8 100644 --- a/v8js_v8.h +++ b/v8js_v8.h @@ -45,7 +45,7 @@ void v8js_v8_init(TSRMLS_D); void v8js_v8_call(v8js_ctx *c, zval **return_value, long flags, long time_limit, long memory_limit, std::function< v8::Local(v8::Isolate *) >& v8_call TSRMLS_DC); -void v8js_terminate_execution(v8js_ctx *c TSRMLS_DC); +void v8js_terminate_execution(v8::Isolate *isolate); /* Fetch V8 object properties */ int v8js_get_properties_hash(v8::Handle jsValue, HashTable *retval, int flags, v8::Isolate *isolate TSRMLS_DC); From f7a592052fa0d854a8159f2e2877add11e0fdda5 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 21 Aug 2015 16:12:12 +0200 Subject: [PATCH 02/11] Store flags in v8js_ctx class instead of v8 hidden value --- php_v8js_macros.h | 4 ---- v8js_class.h | 2 ++ v8js_object_export.cc | 10 +++++----- v8js_v8.cc | 2 +- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/php_v8js_macros.h b/php_v8js_macros.h index cc7aaa4..7fbcd1d 100644 --- a/php_v8js_macros.h +++ b/php_v8js_macros.h @@ -80,10 +80,6 @@ extern "C" { # define V8JS_CONST (char *) #endif -/* Global flags */ -#define V8JS_GLOBAL_SET_FLAGS(isolate,flags) V8JS_GLOBAL(isolate)->SetHiddenValue(V8JS_SYM("__php_flags__"), V8JS_INT(flags)) -#define V8JS_GLOBAL_GET_FLAGS(isolate) V8JS_GLOBAL(isolate)->GetHiddenValue(V8JS_SYM("__php_flags__"))->IntegerValue(); - /* Options */ #define V8JS_FLAG_NONE (1<<0) #define V8JS_FLAG_FORCE_ARRAY (1<<1) diff --git a/v8js_class.h b/v8js_class.h index 43268bb..26fb26d 100644 --- a/v8js_class.h +++ b/v8js_class.h @@ -40,6 +40,8 @@ struct v8js_ctx { int in_execution; v8::Isolate *isolate; + long flags; + long time_limit; bool time_limit_hit; long memory_limit; diff --git a/v8js_object_export.cc b/v8js_object_export.cc index 80ba129..218e566 100644 --- a/v8js_object_export.cc +++ b/v8js_object_export.cc @@ -39,7 +39,7 @@ static void v8js_call_php_func(zval *value, zend_class_entry *ce, zend_function zval fname, *retval_ptr = NULL, **argv = NULL; zend_uint argc = info.Length(), min_num_args = 0, max_num_args = 0; char *error; - int error_len, i, flags = V8JS_FLAG_NONE; + int error_len, i; v8js_ctx *ctx = (v8js_ctx *) isolate->GetData(0); @@ -84,7 +84,6 @@ static void v8js_call_php_func(zval *value, zend_class_entry *ce, zend_function /* Convert parameters passed from V8 */ if (argc) { - flags = V8JS_GLOBAL_GET_FLAGS(isolate); fci.params = (zval ***) safe_emalloc(argc, sizeof(zval **), 0); argv = (zval **) safe_emalloc(argc, sizeof(zval *), 0); for (i = 0; i < argc; i++) { @@ -98,7 +97,7 @@ static void v8js_call_php_func(zval *value, zend_class_entry *ce, zend_function Z_ADDREF_P(argv[i]); } else { MAKE_STD_ZVAL(argv[i]); - if (v8js_to_zval(info[i], argv[i], flags, isolate TSRMLS_CC) == FAILURE) { + if (v8js_to_zval(info[i], argv[i], ctx->flags, isolate TSRMLS_CC) == FAILURE) { fci.param_count++; error_len = spprintf(&error, 0, "converting parameter #%d passed to %s() failed", i + 1, method_ptr->common.function_name); return_value = V8JS_THROW(isolate, Error, error, error_len); @@ -529,6 +528,8 @@ inline v8::Local v8js_named_property_callback(v8::Local p const char *method_name; uint method_name_len; + v8js_ctx *ctx = (v8js_ctx *) isolate->GetData(0); + v8::Local self = info.Holder(); v8::Local ret_value; v8::Local cb; @@ -652,9 +653,8 @@ inline v8::Local v8js_named_property_callback(v8::Local p zval_ptr_dtor(&php_value); } } else if (callback_type == V8JS_PROP_SETTER) { - int flags = V8JS_GLOBAL_GET_FLAGS(isolate); MAKE_STD_ZVAL(php_value); - if (v8js_to_zval(set_value, php_value, flags, isolate TSRMLS_CC) != SUCCESS) { + if (v8js_to_zval(set_value, php_value, ctx->flags, isolate TSRMLS_CC) != SUCCESS) { ret_value = v8::Handle(); } else { diff --git a/v8js_v8.cc b/v8js_v8.cc index 12e413d..4251888 100644 --- a/v8js_v8.cc +++ b/v8js_v8.cc @@ -77,7 +77,7 @@ void v8js_v8_call(v8js_ctx *c, zval **return_value, v8::TryCatch try_catch; /* Set flags for runtime use */ - V8JS_GLOBAL_SET_FLAGS(isolate, flags); + c->flags = flags; /* Check if timezone has been changed and notify V8 */ tz = getenv("TZ"); From 462eb623b3d5d64d97db3aa2d1c340d5238679eb Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sat, 22 Aug 2015 15:16:02 +0200 Subject: [PATCH 03/11] Allow PHP exception to JS propagation --- php_v8js_macros.h | 1 + tests/php_exceptions_003.phpt | 36 +++++++++++++++++++++++++++++++++++ v8js_class.cc | 1 + v8js_object_export.cc | 21 ++++++++++++-------- 4 files changed, 51 insertions(+), 8 deletions(-) create mode 100644 tests/php_exceptions_003.phpt diff --git a/php_v8js_macros.h b/php_v8js_macros.h index 7fbcd1d..61f30c6 100644 --- a/php_v8js_macros.h +++ b/php_v8js_macros.h @@ -83,6 +83,7 @@ extern "C" { /* Options */ #define V8JS_FLAG_NONE (1<<0) #define V8JS_FLAG_FORCE_ARRAY (1<<1) +#define V8JS_FLAG_PROPAGATE_PHP_EXCEPTIONS (1<<2) #define V8JS_DEBUG_AUTO_BREAK_NEVER 0 #define V8JS_DEBUG_AUTO_BREAK_ONCE 1 diff --git a/tests/php_exceptions_003.phpt b/tests/php_exceptions_003.phpt new file mode 100644 index 0000000..6bf71a4 --- /dev/null +++ b/tests/php_exceptions_003.phpt @@ -0,0 +1,36 @@ +--TEST-- +Test V8::executeString() : PHP Exception handling (basic JS propagation) +--SKIPIF-- + +--FILE-- +foo = new \Foo(); + +$JS = <<< EOT +try { + PHP.foo.throwException(); + // the exception should abort further execution, + // hence the print must not pop up + print("after throwException\\n"); +} catch(e) { + print("JS caught exception!\\n"); + var_dump(e.getMessage()); +} +EOT; + +$v8->executeString($JS, 'php_exceptions_003', V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS); + +?> +===EOF=== +--EXPECTF-- +JS caught exception! +string(14) "Test-Exception" +===EOF=== diff --git a/v8js_class.cc b/v8js_class.cc index 3c25442..d242049 100644 --- a/v8js_class.cc +++ b/v8js_class.cc @@ -1078,6 +1078,7 @@ PHP_MINIT_FUNCTION(v8js_class) /* {{{ */ zend_declare_class_constant_long(php_ce_v8js, ZEND_STRL("FLAG_NONE"), V8JS_FLAG_NONE TSRMLS_CC); zend_declare_class_constant_long(php_ce_v8js, ZEND_STRL("FLAG_FORCE_ARRAY"), V8JS_FLAG_FORCE_ARRAY TSRMLS_CC); + zend_declare_class_constant_long(php_ce_v8js, ZEND_STRL("FLAG_PROPAGATE_PHP_EXCEPTIONS"), V8JS_FLAG_PROPAGATE_PHP_EXCEPTIONS TSRMLS_CC); #ifdef ENABLE_DEBUGGER_SUPPORT zend_declare_class_constant_long(php_ce_v8js, ZEND_STRL("DEBUG_AUTO_BREAK_NEVER"), V8JS_DEBUG_AUTO_BREAK_NEVER TSRMLS_CC); diff --git a/v8js_object_export.cc b/v8js_object_export.cc index 218e566..ec11263 100644 --- a/v8js_object_export.cc +++ b/v8js_object_export.cc @@ -21,6 +21,7 @@ extern "C" { #include "ext/standard/php_string.h" #include "zend_interfaces.h" #include "zend_closures.h" +#include "zend_exceptions.h" } #include "php_v8js_macros.h" @@ -33,7 +34,7 @@ static void v8js_weak_object_callback(const v8::WeakCallbackData& info TSRMLS_DC) /* {{{ */ { - v8::Handle return_value; + v8::Handle return_value = V8JS_NULL; zend_fcall_info fci; zend_fcall_info_cache fcc; zval fname, *retval_ptr = NULL, **argv = NULL; @@ -138,10 +139,6 @@ static void v8js_call_php_func(zval *value, zend_class_entry *ce, zend_function } zend_end_try(); - if(EG(exception)) { - v8js_terminate_execution(isolate); - } - failure: /* Cleanup */ if (argc) { @@ -152,11 +149,19 @@ failure: efree(fci.params); } - if (retval_ptr != NULL) { + if(EG(exception)) { + if(ctx->flags & V8JS_FLAG_PROPAGATE_PHP_EXCEPTIONS) { + return_value = isolate->ThrowException(zval_to_v8js(EG(exception), isolate TSRMLS_CC)); + zend_clear_exception(TSRMLS_C); + } else { + v8js_terminate_execution(isolate); + } + } else if (retval_ptr != NULL) { return_value = zval_to_v8js(retval_ptr, isolate TSRMLS_CC); + } + + if (retval_ptr != NULL) { zval_ptr_dtor(&retval_ptr); - } else { - return_value = V8JS_NULL; } info.GetReturnValue().Set(return_value); From f7c33539c243ecf625f8b1624a664f0d1cf4f7fc Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 23 Aug 2015 15:09:21 +0200 Subject: [PATCH 04/11] Improve PHP->JS->PHP exception back propagation --- tests/php_exceptions_004.phpt | 36 +++++++++++++++++++++++++++++++++++ v8js_class.cc | 5 +++-- v8js_exceptions.cc | 20 +++++++++++++++---- v8js_exceptions.h | 7 ++++--- v8js_v8.cc | 7 ++++--- 5 files changed, 63 insertions(+), 12 deletions(-) create mode 100644 tests/php_exceptions_004.phpt diff --git a/tests/php_exceptions_004.phpt b/tests/php_exceptions_004.phpt new file mode 100644 index 0000000..6b37e21 --- /dev/null +++ b/tests/php_exceptions_004.phpt @@ -0,0 +1,36 @@ +--TEST-- +Test V8::executeString() : PHP Exception handling (PHP->JS->PHP back propagation) +--SKIPIF-- + +--FILE-- +foo = new \Foo(); + +$JS = <<< EOT +PHP.foo.throwException(); +// the exception should abort further execution, +// hence the print must not pop up +print("after throwException\\n"); +EOT; + +try { + $v8->executeString($JS, 'php_exceptions_004', V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS); +} +catch(V8JsScriptException $e) { + echo "Got V8JsScriptException\n"; + var_dump($e->getPrevious()->getMessage()); +} +?> +===EOF=== +--EXPECTF-- +Got V8JsScriptException +string(14) "Test-Exception" +===EOF=== diff --git a/v8js_class.cc b/v8js_class.cc index d242049..ef6b896 100644 --- a/v8js_class.cc +++ b/v8js_class.cc @@ -2,12 +2,13 @@ +----------------------------------------------------------------------+ | PHP Version 5 | +----------------------------------------------------------------------+ - | Copyright (c) 1997-2013 The PHP Group | + | Copyright (c) 1997-2015 The PHP Group | +----------------------------------------------------------------------+ | http://www.opensource.org/licenses/mit-license.php MIT License | +----------------------------------------------------------------------+ | Author: Jani Taskinen | | Author: Patrick Reilly | + | Author: Stefan Siegl | +----------------------------------------------------------------------+ */ @@ -498,7 +499,7 @@ static void v8js_compile_script(zval *this_ptr, const char *str, int str_len, co /* Compile errors? */ if (script.IsEmpty()) { - v8js_throw_script_exception(&try_catch TSRMLS_CC); + v8js_throw_script_exception(c->isolate, &try_catch TSRMLS_CC); return; } res = (v8js_script *)emalloc(sizeof(v8js_script)); diff --git a/v8js_exceptions.cc b/v8js_exceptions.cc index 3ecdeb2..c2e2379 100644 --- a/v8js_exceptions.cc +++ b/v8js_exceptions.cc @@ -2,12 +2,13 @@ +----------------------------------------------------------------------+ | PHP Version 5 | +----------------------------------------------------------------------+ - | Copyright (c) 1997-2013 The PHP Group | + | Copyright (c) 1997-2015 The PHP Group | +----------------------------------------------------------------------+ | http://www.opensource.org/licenses/mit-license.php MIT License | +----------------------------------------------------------------------+ | Author: Jani Taskinen | | Author: Patrick Reilly | + | Author: Stefan Siegl | +----------------------------------------------------------------------+ */ @@ -38,7 +39,7 @@ zend_class_entry *php_ce_v8js_memory_limit_exception; /* {{{ Class: V8JsScriptException */ -void v8js_create_script_exception(zval *return_value, v8::TryCatch *try_catch TSRMLS_DC) /* {{{ */ +void v8js_create_script_exception(zval *return_value, v8::Isolate *isolate, v8::TryCatch *try_catch TSRMLS_DC) /* {{{ */ { v8::String::Utf8Value exception(try_catch->Exception()); const char *exception_string = ToCString(exception); @@ -81,6 +82,17 @@ void v8js_create_script_exception(zval *return_value, v8::TryCatch *try_catch TS const char* stacktrace_string = ToCString(stacktrace); PHPV8_EXPROP(_string, JsTrace, stacktrace_string); } + + if(try_catch->Exception()->IsObject()) { + v8::Local php_ref = try_catch->Exception()->ToObject()->GetHiddenValue(V8JS_SYM(PHPJS_OBJECT_KEY)); + + if(!php_ref.IsEmpty()) { + assert(php_ref->IsExternal()); + zval *php_exception = reinterpret_cast(v8::External::Cast(*php_ref)->Value()); + zend_exception_set_previous(return_value, php_exception TSRMLS_CC); + } + } + } PHPV8_EXPROP(_string, message, message_string); @@ -89,7 +101,7 @@ void v8js_create_script_exception(zval *return_value, v8::TryCatch *try_catch TS } /* }}} */ -void v8js_throw_script_exception(v8::TryCatch *try_catch TSRMLS_DC) /* {{{ */ +void v8js_throw_script_exception(v8::Isolate *isolate, v8::TryCatch *try_catch TSRMLS_DC) /* {{{ */ { v8::String::Utf8Value exception(try_catch->Exception()); const char *exception_string = ToCString(exception); @@ -99,7 +111,7 @@ void v8js_throw_script_exception(v8::TryCatch *try_catch TSRMLS_DC) /* {{{ */ zend_throw_exception(php_ce_v8js_script_exception, (char *) exception_string, 0 TSRMLS_CC); } else { MAKE_STD_ZVAL(zexception); - v8js_create_script_exception(zexception, try_catch TSRMLS_CC); + v8js_create_script_exception(zexception, isolate, try_catch TSRMLS_CC); zend_throw_exception_object(zexception TSRMLS_CC); } } diff --git a/v8js_exceptions.h b/v8js_exceptions.h index e1ca0b7..b3536a8 100644 --- a/v8js_exceptions.h +++ b/v8js_exceptions.h @@ -2,12 +2,13 @@ +----------------------------------------------------------------------+ | PHP Version 5 | +----------------------------------------------------------------------+ - | Copyright (c) 1997-2013 The PHP Group | + | Copyright (c) 1997-2015 The PHP Group | +----------------------------------------------------------------------+ | http://www.opensource.org/licenses/mit-license.php MIT License | +----------------------------------------------------------------------+ | Author: Jani Taskinen | | Author: Patrick Reilly | + | Author: Stefan Siegl | +----------------------------------------------------------------------+ */ @@ -19,8 +20,8 @@ extern zend_class_entry *php_ce_v8js_script_exception; extern zend_class_entry *php_ce_v8js_time_limit_exception; extern zend_class_entry *php_ce_v8js_memory_limit_exception; -void v8js_create_script_exception(zval *return_value, v8::TryCatch *try_catch TSRMLS_DC); -void v8js_throw_script_exception(v8::TryCatch *try_catch TSRMLS_DC); +void v8js_create_script_exception(zval *return_value, v8::Isolate *isolate, v8::TryCatch *try_catch TSRMLS_DC); +void v8js_throw_script_exception(v8::Isolate *isolate, v8::TryCatch *try_catch TSRMLS_DC); PHP_MINIT_FUNCTION(v8js_exceptions); diff --git a/v8js_v8.cc b/v8js_v8.cc index 4251888..18d866d 100644 --- a/v8js_v8.cc +++ b/v8js_v8.cc @@ -2,12 +2,13 @@ +----------------------------------------------------------------------+ | PHP Version 5 | +----------------------------------------------------------------------+ - | Copyright (c) 1997-2013 The PHP Group | + | Copyright (c) 1997-2015 The PHP Group | +----------------------------------------------------------------------+ | http://www.opensource.org/licenses/mit-license.php MIT License | +----------------------------------------------------------------------+ | Author: Jani Taskinen | | Author: Patrick Reilly | + | Author: Stefan Siegl | +----------------------------------------------------------------------+ */ @@ -175,14 +176,14 @@ void v8js_v8_call(v8js_ctx *c, zval **return_value, /* Report immediately if report_uncaught is true */ if (c->report_uncaught) { - v8js_throw_script_exception(&try_catch TSRMLS_CC); + v8js_throw_script_exception(c->isolate, &try_catch TSRMLS_CC); return; } /* Exception thrown from JS, preserve it for future execution */ if (result.IsEmpty()) { MAKE_STD_ZVAL(c->pending_exception); - v8js_create_script_exception(c->pending_exception, &try_catch TSRMLS_CC); + v8js_create_script_exception(c->pending_exception, c->isolate, &try_catch TSRMLS_CC); return; } } From b7dde1b1db5a9d3909ccc3daf38b752e2977ad3b Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 23 Aug 2015 17:40:27 +0200 Subject: [PATCH 05/11] Handle thrown PHP objects, that are no exceptions --- tests/php_exceptions_005.phpt | 43 +++++++++++++++++++++++++++++++++++ tests/php_exceptions_006.phpt | 40 ++++++++++++++++++++++++++++++++ v8js_exceptions.cc | 6 ++++- 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 tests/php_exceptions_005.phpt create mode 100644 tests/php_exceptions_006.phpt diff --git a/tests/php_exceptions_005.phpt b/tests/php_exceptions_005.phpt new file mode 100644 index 0000000..94f3c0d --- /dev/null +++ b/tests/php_exceptions_005.phpt @@ -0,0 +1,43 @@ +--TEST-- +Test V8::executeString() : PHP Exception handling (JS throw PHP-exception) +--SKIPIF-- + +--FILE-- +foo = new \Foo(); + +$JS = <<< EOT +var ex = PHP.foo.getException(); +print("after getException\\n"); +throw ex; +print("after throw\\n"); +EOT; + +try { + $v8->executeString($JS, 'php_exceptions_005'); +} +catch(V8JsScriptException $e) { + echo "Got V8JsScriptException\n"; + var_dump($e->getMessage()); + var_dump($e->getPrevious()->getMessage()); +} +?> +===EOF=== +--EXPECTF-- +after getException +Got V8JsScriptException +string(329) "php_exceptions_005:3: exception 'Exception' with message 'Test-Exception' in %s +Stack trace: +#0 [internal function]: Foo->getException() +#1 %s: V8Js->executeString('var ex = PHP.fo...', 'php_exceptions_...') +#2 {main}" +string(14) "Test-Exception" +===EOF=== diff --git a/tests/php_exceptions_006.phpt b/tests/php_exceptions_006.phpt new file mode 100644 index 0000000..9ccc7d5 --- /dev/null +++ b/tests/php_exceptions_006.phpt @@ -0,0 +1,40 @@ +--TEST-- +Test V8::executeString() : PHP Exception handling (JS throws normal PHP-object) +--SKIPIF-- + +--FILE-- +foo = new \Foo(); + +$JS = <<< EOT +var ex = PHP.foo.getNonExceptionObject(); +print("after getNonExceptionObject\\n"); +throw ex; +print("after throw\\n"); +EOT; + +try { + $v8->executeString($JS, 'php_exceptions_006'); +} +catch(V8JsScriptException $e) { + echo "Got V8JsScriptException\n"; + var_dump($e->getMessage()); + // previous exception should be NULL, as it is *not* a php exception + var_dump($e->getPrevious()); +} +?> +===EOF=== +--EXPECTF-- +after getNonExceptionObject +Got V8JsScriptException +string(34) "php_exceptions_006:3: [object Foo]" +NULL +===EOF=== diff --git a/v8js_exceptions.cc b/v8js_exceptions.cc index c2e2379..0b5e6c7 100644 --- a/v8js_exceptions.cc +++ b/v8js_exceptions.cc @@ -89,7 +89,11 @@ void v8js_create_script_exception(zval *return_value, v8::Isolate *isolate, v8:: if(!php_ref.IsEmpty()) { assert(php_ref->IsExternal()); zval *php_exception = reinterpret_cast(v8::External::Cast(*php_ref)->Value()); - zend_exception_set_previous(return_value, php_exception TSRMLS_CC); + + zend_class_entry *exception_ce = zend_exception_get_default(TSRMLS_C); + if (Z_TYPE_P(php_exception) == IS_OBJECT && instanceof_function(Z_OBJCE_P(php_exception), exception_ce TSRMLS_CC)) { + zend_exception_set_previous(return_value, php_exception TSRMLS_CC); + } } } From 829a98e0645e3d0a9ce1db8fe82be02c1e53bade Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 23 Aug 2015 17:56:26 +0200 Subject: [PATCH 06/11] Mention (new) exception behaviour in README --- README.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/README.md b/README.md index 462fb21..7b2f322 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,7 @@ class V8Js const FLAG_NONE = 1; const FLAG_FORCE_ARRAY = 2; + const FLAG_PROPAGATE_PHP_EXCEPTIONS = 4; const DEBUG_AUTO_BREAK_NEVER = 1; const DEBUG_AUTO_BREAK_ONCE = 2; @@ -297,3 +298,21 @@ PHP Objects implementing ArrayAccess, Countable The above rule that PHP objects are generally converted to JavaScript objects also applies to PHP objects of `ArrayObject` type or other classes, that implement both the `ArrayAccess` and the `Countable` interface -- even so they behave like PHP arrays. This behaviour can be changed by enabling the php.ini flag `v8js.use_array_access`. If set, objects of PHP classes that implement the aforementioned interfaces are converted to JavaScript Array-like objects. This is by-index access of this object results in immediate calls to the `offsetGet` or `offsetSet` PHP methods (effectively this is live-binding of JavaScript against the PHP object). Such an Array-esque object also supports calling every attached public method of the PHP object + methods of JavaScript's native Array.prototype methods (as long as they are not overloaded by PHP methods). + +Exceptions +---------- + +If the JavaScript code throws (without catching), causes errors or doesn't +compile, `V8JsScriptException` exceptions are thrown unless the `V8Js` object +is constructed with `report_uncaught_exceptions` set `FALSE`. + +PHP exceptions that occur due to calls from JavaScript code by default are +*not* re-thrown into JavaScript context but cause the JavaScript execution to +be stopped immediately. + +This behaviour can be changed by setting the `FLAG_PROPAGATE_PHP_EXCEPTIONS` +flag. If it is set, PHP exception (objects) are converted to JavaScript +objects obeying the above rules and re-thrown in JavaScript context. If they +are not caught by JavaScript code the execution stops and a +`V8JsScriptException` is thrown, which has the original PHP exception accessible +via `getPrevious` method. \ No newline at end of file From c033000aea632a847d42d38a9a3756ba2516e082 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 23 Aug 2015 18:05:46 +0200 Subject: [PATCH 07/11] Fix tests/exception_propagation_2.phpt The test relied on weird behaviour that PHP exceptions shouldn't stop JavaScript code execution. Since JavaScript execution is now stopped, the JavaScript catch handler is not executed anymore. --- tests/exception_propagation_2.phpt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/exception_propagation_2.phpt b/tests/exception_propagation_2.phpt index c12830a..173e335 100644 --- a/tests/exception_propagation_2.phpt +++ b/tests/exception_propagation_2.phpt @@ -1,7 +1,6 @@ --TEST-- Test V8::executeString() : Exception propagation test 2 --SKIPIF-- -SKIP needs discussion, see issue #144 --FILE-- v8->foo = $this; $this->v8->executeString('fooobar', 'throw_0'); var_dump($this->v8->getPendingException()); + // the exception is not cleared before the next executeString call, + // hence the next *exiting* executeString will throw. + // In this case this is the executeString call in bar() function. $this->v8->executeString('try { PHP.foo.bar(); } catch (e) { print(e + " caught!\n"); }', 'trycatch1'); $this->v8->executeString('try { PHP.foo.bar(); } catch (e) { print(e + " caught!\n"); }', 'trycatch2'); } @@ -22,6 +24,8 @@ class Foo { public function bar() { echo "To Bar!\n"; + // This executeString call throws a PHP exception, not propagated + // to JS, hence immediately triggering the top-level catch handler. $this->v8->executeString('throw new Error();', 'throw_1'); } } @@ -72,7 +76,7 @@ object(V8JsScriptException)#%d (13) { ["file"]=> string(%d) "%s" ["line"]=> - int(24) + int(29) ["function"]=> string(11) "__construct" ["class"]=> @@ -101,6 +105,5 @@ object(V8JsScriptException)#%d (13) { at throw_0:1:1" } To Bar! -Error caught! PHP Exception: throw_0:1: ReferenceError: fooobar is not defined ===EOF=== From 790735f04a404f25938662904f598971c8ad1460 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 23 Aug 2015 19:30:47 +0200 Subject: [PATCH 08/11] Fix tests/php_exceptions_005.phpt --- tests/php_exceptions_005.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php_exceptions_005.phpt b/tests/php_exceptions_005.phpt index 94f3c0d..296cb45 100644 --- a/tests/php_exceptions_005.phpt +++ b/tests/php_exceptions_005.phpt @@ -34,7 +34,7 @@ catch(V8JsScriptException $e) { --EXPECTF-- after getException Got V8JsScriptException -string(329) "php_exceptions_005:3: exception 'Exception' with message 'Test-Exception' in %s +string(%d) "php_exceptions_005:3: exception 'Exception' with message 'Test-Exception' in %s Stack trace: #0 [internal function]: Foo->getException() #1 %s: V8Js->executeString('var ex = PHP.fo...', 'php_exceptions_...') From 5f6d9aee2d30e055ea8ecd12e5fe8ca949c0727a Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Tue, 22 Sep 2015 22:36:50 +0200 Subject: [PATCH 09/11] Don't terminate execution repeatedly --- v8js_v8.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/v8js_v8.cc b/v8js_v8.cc index 18d866d..a72376c 100644 --- a/v8js_v8.cc +++ b/v8js_v8.cc @@ -202,6 +202,13 @@ void v8js_v8_call(v8js_ctx *c, zval **return_value, void v8js_terminate_execution(v8::Isolate *isolate) /* {{{ */ { + if(v8::V8::IsExecutionTerminating(isolate)) { + /* Execution already terminating, needn't trigger it again and + * especially must not execute the spinning loop (which would cause + * crashes in V8 itself, at least with 4.2 and 4.3 version lines). */ + return; + } + /* Unfortunately just calling TerminateExecution on the isolate is not * enough, since v8 just marks the thread as "to be aborted" and doesn't * immediately do so. Hence we enter an endless loop after signalling From 8934db6dec59ddfefc43d94411a958ae477f8487 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Wed, 23 Sep 2015 19:19:11 +0200 Subject: [PATCH 10/11] Add v8js.compat_php_exceptions INI switch --- php_v8js_macros.h | 1 + tests/issue_156_001.phpt | 33 +++++++++++++++++++++++++++++++++ v8js.cc | 18 ++++++++++++++++++ v8js_object_export.cc | 2 +- 4 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tests/issue_156_001.phpt diff --git a/php_v8js_macros.h b/php_v8js_macros.h index 61f30c6..555dca6 100644 --- a/php_v8js_macros.h +++ b/php_v8js_macros.h @@ -121,6 +121,7 @@ ZEND_BEGIN_MODULE_GLOBALS(v8js) char *v8_flags; /* V8 command line flags */ bool use_date; /* Generate JS Date objects instead of PHP DateTime */ bool use_array_access; /* Convert ArrayAccess, Countable objects to array-like objects */ + bool compat_php_exceptions; /* Don't stop JS execution on PHP exception */ // Timer thread globals std::deque timer_stack; diff --git a/tests/issue_156_001.phpt b/tests/issue_156_001.phpt new file mode 100644 index 0000000..dc9ed68 --- /dev/null +++ b/tests/issue_156_001.phpt @@ -0,0 +1,33 @@ +--TEST-- +Test V8::executeString() : Backwards compatibility for issue #156 +--SKIPIF-- + +--INI-- +v8js.compat_php_exceptions = 1 +--FILE-- +throwPHPException = function () { + echo "throwing PHP exception now ...\n"; + throw new \Exception('foo'); +}; + +$JS = <<< EOT +PHP.throwPHPException(); +print("... old behaviour was to not stop JS execution on PHP exceptions\\n"); +EOT; + +try { + $v8->executeString($JS, 'issue_156_001.js'); +} catch(Exception $e) { + var_dump($e->getMessage()); +} +?> +===EOF=== +--EXPECT-- +throwing PHP exception now ... +... old behaviour was to not stop JS execution on PHP exceptions +string(3) "foo" +===EOF=== diff --git a/v8js.cc b/v8js.cc index 6236ed0..333cd81 100755 --- a/v8js.cc +++ b/v8js.cc @@ -82,10 +82,28 @@ static ZEND_INI_MH(v8js_OnUpdateUseArrayAccess) /* {{{ */ } /* }}} */ +static ZEND_INI_MH(v8js_OnUpdateCompatExceptions) /* {{{ */ +{ + bool value; + if (new_value_length==2 && strcasecmp("on", new_value)==0) { + value = (bool) 1; + } else if (new_value_length==3 && strcasecmp("yes", new_value)==0) { + value = (bool) 1; + } else if (new_value_length==4 && strcasecmp("true", new_value)==0) { + value = (bool) 1; + } else { + value = (bool) atoi(new_value); + } + V8JSG(compat_php_exceptions) = value; + return SUCCESS; +} +/* }}} */ + ZEND_INI_BEGIN() /* {{{ */ ZEND_INI_ENTRY("v8js.flags", NULL, ZEND_INI_ALL, v8js_OnUpdateV8Flags) ZEND_INI_ENTRY("v8js.use_date", "0", ZEND_INI_ALL, v8js_OnUpdateUseDate) ZEND_INI_ENTRY("v8js.use_array_access", "0", ZEND_INI_ALL, v8js_OnUpdateUseArrayAccess) + ZEND_INI_ENTRY("v8js.compat_php_exceptions", "0", ZEND_INI_ALL, v8js_OnUpdateCompatExceptions) ZEND_INI_END() /* }}} */ diff --git a/v8js_object_export.cc b/v8js_object_export.cc index ec11263..fe1ccb2 100644 --- a/v8js_object_export.cc +++ b/v8js_object_export.cc @@ -149,7 +149,7 @@ failure: efree(fci.params); } - if(EG(exception)) { + if(EG(exception) && !V8JSG(compat_php_exceptions)) { if(ctx->flags & V8JS_FLAG_PROPAGATE_PHP_EXCEPTIONS) { return_value = isolate->ThrowException(zval_to_v8js(EG(exception), isolate TSRMLS_CC)); zend_clear_exception(TSRMLS_C); From 6d31da2ab7708fb4649c231db652a921d79ad803 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Wed, 23 Sep 2015 19:39:41 +0200 Subject: [PATCH 11/11] Mention compat_php_exceptions php.ini switch --- README.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 7b2f322..eb42610 100644 --- a/README.md +++ b/README.md @@ -300,7 +300,7 @@ The above rule that PHP objects are generally converted to JavaScript objects al This behaviour can be changed by enabling the php.ini flag `v8js.use_array_access`. If set, objects of PHP classes that implement the aforementioned interfaces are converted to JavaScript Array-like objects. This is by-index access of this object results in immediate calls to the `offsetGet` or `offsetSet` PHP methods (effectively this is live-binding of JavaScript against the PHP object). Such an Array-esque object also supports calling every attached public method of the PHP object + methods of JavaScript's native Array.prototype methods (as long as they are not overloaded by PHP methods). Exceptions ----------- +========== If the JavaScript code throws (without catching), causes errors or doesn't compile, `V8JsScriptException` exceptions are thrown unless the `V8Js` object @@ -308,11 +308,18 @@ is constructed with `report_uncaught_exceptions` set `FALSE`. PHP exceptions that occur due to calls from JavaScript code by default are *not* re-thrown into JavaScript context but cause the JavaScript execution to -be stopped immediately. +be stopped immediately and then are reported at the location calling the JS code. This behaviour can be changed by setting the `FLAG_PROPAGATE_PHP_EXCEPTIONS` flag. If it is set, PHP exception (objects) are converted to JavaScript objects obeying the above rules and re-thrown in JavaScript context. If they are not caught by JavaScript code the execution stops and a `V8JsScriptException` is thrown, which has the original PHP exception accessible -via `getPrevious` method. \ No newline at end of file +via `getPrevious` method. + +V8Js versions 0.2.4 and before did not stop JS code execution on PHP exceptions, +but silently ignored them (even so succeeding PHP calls from within the same piece +of JS code were not executed by the PHP engine). This behaviour is considered as +a bug and hence was fixed with 0.2.5 release. Nevertheless there is a +compatibility php.ini switch (`v8js.compat_php_exceptions`) which turns previous +behaviour back on.