From ac2b1cb23852466442dbefc17cf628a6062d5c61 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Tue, 31 May 2022 14:21:32 +0200 Subject: [PATCH 1/9] introduce setExceptionProxyFactory --- tests/exception_proxy_basic.phpt | 55 ++++++++++++++++++++++++++++++++ v8js_class.cc | 22 +++++++++++++ v8js_class.h | 1 + v8js_object_export.cc | 39 +++++++++++++++++----- 4 files changed, 109 insertions(+), 8 deletions(-) create mode 100644 tests/exception_proxy_basic.phpt diff --git a/tests/exception_proxy_basic.phpt b/tests/exception_proxy_basic.phpt new file mode 100644 index 0000000..e7f8b3b --- /dev/null +++ b/tests/exception_proxy_basic.phpt @@ -0,0 +1,55 @@ +--TEST-- +Test V8::setExceptionProxyFactory() : Simple test +--SKIPIF-- + +--FILE-- +getMessage()); + + $this->ex = $ex; + } + + public function getMessage() { + echo "getMessage called\n"; + return $this->ex->getMessage(); + } +} + +$v8 = new myv8(); +$v8->setExceptionProxyFactory(function (Throwable $ex) { + echo "exception proxy factory called.\n"; + return new ExceptionProxy($ex); +}); + +$v8->executeString(' + try { + PHP.throwException("Oops"); + } + catch (e) { + var_dump(e.getMessage()); // calls ExceptionProxy::getMessage + var_dump(typeof e.getTrace); + } +', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS); +?> +===EOF=== +--EXPECT-- +exception proxy factory called. +ExceptionProxy::__construct called! +string(4) "Oops" +getMessage called +string(4) "Oops" +string(9) "undefined" +===EOF=== + diff --git a/v8js_class.cc b/v8js_class.cc index 4e9be67..57f5571 100644 --- a/v8js_class.cc +++ b/v8js_class.cc @@ -92,6 +92,7 @@ static void v8js_free_storage(zend_object *object) /* {{{ */ zval_ptr_dtor(&c->pending_exception); zval_ptr_dtor(&c->module_normaliser); zval_ptr_dtor(&c->module_loader); + zval_ptr_dtor(&c->exception_proxy_factory); /* Delete PHP global object from JavaScript */ if (!c->context.IsEmpty()) { @@ -400,6 +401,7 @@ static PHP_METHOD(V8Js, __construct) ZVAL_NULL(&c->module_normaliser); ZVAL_NULL(&c->module_loader); + ZVAL_NULL(&c->exception_proxy_factory); /* Include extensions used by this context */ /* Note: Extensions registered with auto_enable do not need to be added separately like this. */ @@ -880,6 +882,21 @@ static PHP_METHOD(V8Js, setModuleLoader) } /* }}} */ +/* {{{ proto void V8Js::setExceptionProxyFactory(callable factory) + */ +static PHP_METHOD(V8Js, setExceptionProxyFactory) +{ + zval *callable; + + if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &callable) == FAILURE) { + return; + } + + v8js_ctx *c = Z_V8JS_CTX_OBJ_P(getThis()); + ZVAL_COPY(&c->exception_proxy_factory, callable); +} +/* }}} */ + /* {{{ proto void V8Js::setTimeLimit(int time_limit) */ static PHP_METHOD(V8Js, setTimeLimit) @@ -1254,6 +1271,10 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_v8js_setmoduleloader, 0, 0, 1) ZEND_ARG_INFO(0, callable) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_INFO_EX(arginfo_v8js_setexceptionproxyfactory, 0, 0, 1) + ZEND_ARG_INFO(0, callable) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_INFO_EX(arginfo_v8js_setaverageobjectsize, 0, 0, 1) ZEND_ARG_INFO(0, average_object_size) ZEND_END_ARG_INFO() @@ -1293,6 +1314,7 @@ const zend_function_entry v8js_methods[] = { /* {{{ */ PHP_ME(V8Js, clearPendingException, arginfo_v8js_clearpendingexception, ZEND_ACC_PUBLIC|ZEND_ACC_DEPRECATED) PHP_ME(V8Js, setModuleNormaliser, arginfo_v8js_setmodulenormaliser, ZEND_ACC_PUBLIC) PHP_ME(V8Js, setModuleLoader, arginfo_v8js_setmoduleloader, ZEND_ACC_PUBLIC) + PHP_ME(V8Js, setExceptionProxyFactory, arginfo_v8js_setexceptionproxyfactory, ZEND_ACC_PUBLIC) PHP_ME(V8Js, setTimeLimit, arginfo_v8js_settimelimit, ZEND_ACC_PUBLIC) PHP_ME(V8Js, setMemoryLimit, arginfo_v8js_setmemorylimit, ZEND_ACC_PUBLIC) PHP_ME(V8Js, setAverageObjectSize, arginfo_v8js_setaverageobjectsize, ZEND_ACC_PUBLIC) diff --git a/v8js_class.h b/v8js_class.h index 8bcdb6b..0fcdcd5 100644 --- a/v8js_class.h +++ b/v8js_class.h @@ -55,6 +55,7 @@ struct v8js_ctx { zval module_normaliser; zval module_loader; + zval exception_proxy_factory; std::vector modules_stack; std::map modules_loaded; diff --git a/v8js_object_export.cc b/v8js_object_export.cc index b36a347..d78fd51 100644 --- a/v8js_object_export.cc +++ b/v8js_object_export.cc @@ -34,6 +34,36 @@ extern "C" { static void v8js_weak_object_callback(const v8::WeakCallbackInfo &data); +v8::Local v8js_propagate_exception(v8js_ctx *ctx) /* {{{ */ +{ + v8::Local return_value = v8::Null(ctx->isolate); + + if (!(ctx->flags & V8JS_FLAG_PROPAGATE_PHP_EXCEPTIONS)) { + v8js_terminate_execution(ctx->isolate); + return return_value; + } + + zval tmp_zv; + + if (Z_TYPE(ctx->exception_proxy_factory) != IS_NULL) { + zval params[1]; + ZVAL_OBJ(¶ms[0], EG(exception)); + Z_ADDREF_P(¶ms[0]); + zend_clear_exception(); + call_user_function(EG(function_table), NULL, &ctx->exception_proxy_factory, &tmp_zv, 1, params); + zval_ptr_dtor(¶ms[0]); + + return_value = ctx->isolate->ThrowException(zval_to_v8js(&tmp_zv, ctx->isolate)); + } else { + ZVAL_OBJ(&tmp_zv, EG(exception)); + return_value = ctx->isolate->ThrowException(zval_to_v8js(&tmp_zv, ctx->isolate)); + zend_clear_exception(); + } + + return return_value; +} +/* }}} */ + /* Callback for PHP methods and functions */ static void v8js_call_php_func(zend_object *object, zend_function *method_ptr, const v8::FunctionCallbackInfo& info) /* {{{ */ { @@ -175,14 +205,7 @@ failure: } if(EG(exception)) { - if(ctx->flags & V8JS_FLAG_PROPAGATE_PHP_EXCEPTIONS) { - zval tmp_zv; - ZVAL_OBJ(&tmp_zv, EG(exception)); - return_value = isolate->ThrowException(zval_to_v8js(&tmp_zv, isolate)); - zend_clear_exception(); - } else { - v8js_terminate_execution(isolate); - } + return_value = v8js_propagate_exception(ctx); } else if (Z_TYPE(retval) == IS_OBJECT && Z_OBJ(retval) == object) { // special case: "return $this" return_value = info.Holder(); From 456814703c7ee334bc93459309182ff123be4a2e Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Tue, 31 May 2022 15:11:44 +0200 Subject: [PATCH 2/9] add Throwable to string conversion testcase --- tests/exception_proxy_001.phpt | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 tests/exception_proxy_001.phpt diff --git a/tests/exception_proxy_001.phpt b/tests/exception_proxy_001.phpt new file mode 100644 index 0000000..52c8286 --- /dev/null +++ b/tests/exception_proxy_001.phpt @@ -0,0 +1,35 @@ +--TEST-- +Test V8::setExceptionProxyFactory() : String conversion +--SKIPIF-- + +--FILE-- +setExceptionProxyFactory(function (Throwable $ex) { + echo "exception proxy factory called.\n"; + return $ex->getMessage(); +}); + +$v8->executeString(' + try { + PHP.throwException("Oops"); + } + catch (e) { + var_dump(typeof e); // string + var_dump(e); + } +', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS); +?> +===EOF=== +--EXPECT-- +exception proxy factory called. +string(6) "string" +string(4) "Oops" +===EOF=== From 7422ef2383d255b8ef1c39904e28026dc6b6b5e6 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Tue, 31 May 2022 15:58:47 +0200 Subject: [PATCH 3/9] run exceptions thrown in require() through proxy factory as well --- tests/exception_proxy_002.phpt | 32 ++++++++++++++++++++++++++++++++ tests/exception_proxy_003.phpt | 34 ++++++++++++++++++++++++++++++++++ v8js_methods.cc | 22 ++++------------------ v8js_object_export.h | 1 + 4 files changed, 71 insertions(+), 18 deletions(-) create mode 100644 tests/exception_proxy_002.phpt create mode 100644 tests/exception_proxy_003.phpt diff --git a/tests/exception_proxy_002.phpt b/tests/exception_proxy_002.phpt new file mode 100644 index 0000000..9174eb2 --- /dev/null +++ b/tests/exception_proxy_002.phpt @@ -0,0 +1,32 @@ +--TEST-- +Test V8::setExceptionProxyFactory() : Proxy handling on exception in setModuleLoader +--SKIPIF-- + +--FILE-- +setModuleLoader(function ($path) { + throw new Error('moep'); +}); + +$v8->setExceptionProxyFactory(function (Throwable $ex) { + echo "exception proxy factory called.\n"; + return $ex->getMessage(); +}); + +$v8->executeString(' + try { + require("file"); + } catch(e) { + var_dump(e); + } +', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS); + +?> +===EOF=== +--EXPECT-- +exception proxy factory called. +string(4) "moep" +===EOF=== + diff --git a/tests/exception_proxy_003.phpt b/tests/exception_proxy_003.phpt new file mode 100644 index 0000000..fe70877 --- /dev/null +++ b/tests/exception_proxy_003.phpt @@ -0,0 +1,34 @@ +--TEST-- +Test V8::setExceptionProxyFactory() : Proxy handling on exception in setModuleNormaliser +--SKIPIF-- + +--FILE-- +setModuleNormaliser(function ($path) { + throw new Error('blarg'); +}); +$v8->setModuleLoader(function ($path) { + throw new Error('moep'); +}); + +$v8->setExceptionProxyFactory(function (Throwable $ex) { + echo "exception proxy factory called.\n"; + return $ex->getMessage(); +}); + +$v8->executeString(' + try { + require("file"); + } catch(e) { + var_dump(e); + } +', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS); + +?> +===EOF=== +--EXPECT-- +exception proxy factory called. +string(5) "blarg" +===EOF=== diff --git a/v8js_methods.cc b/v8js_methods.cc index 7f7f926..0fdd550 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -19,6 +19,7 @@ #include "php_v8js_macros.h" #include "v8js_commonjs.h" #include "v8js_exceptions.h" +#include "v8js_object_export.h" extern "C" { #include "zend_exceptions.h" @@ -337,14 +338,7 @@ V8JS_METHOD(require) // Check if an exception was thrown if (EG(exception)) { - if (c->flags & V8JS_FLAG_PROPAGATE_PHP_EXCEPTIONS) { - zval tmp_zv; - ZVAL_OBJ(&tmp_zv, EG(exception)); - info.GetReturnValue().Set(isolate->ThrowException(zval_to_v8js(&tmp_zv, isolate))); - zend_clear_exception(); - } else { - v8js_terminate_execution(isolate); - } + info.GetReturnValue().Set(v8js_propagate_exception(c)); return; } @@ -466,15 +460,7 @@ V8JS_METHOD(require) efree(normalised_module_id); efree(normalised_path); - if (c->flags & V8JS_FLAG_PROPAGATE_PHP_EXCEPTIONS) { - zval tmp_zv; - ZVAL_OBJ(&tmp_zv, EG(exception)); - info.GetReturnValue().Set(isolate->ThrowException(zval_to_v8js(&tmp_zv, isolate))); - zend_clear_exception(); - } else { - v8js_terminate_execution(isolate); - } - + info.GetReturnValue().Set(v8js_propagate_exception(c)); return; } @@ -485,7 +471,7 @@ V8JS_METHOD(require) efree(normalised_path); return; - } + } if(Z_TYPE(module_code) == IS_OBJECT) { v8::Local newobj = zval_to_v8js(&module_code, isolate)->ToObject(isolate->GetEnteredOrMicrotaskContext()).ToLocalChecked(); diff --git a/v8js_object_export.h b/v8js_object_export.h index 3bbc840..d562541 100644 --- a/v8js_object_export.h +++ b/v8js_object_export.h @@ -15,6 +15,7 @@ #define V8JS_OBJECT_EXPORT_H v8::Local v8js_hash_to_jsobj(zval *value, v8::Isolate *isolate); +v8::Local v8js_propagate_exception(v8js_ctx *ctx); typedef enum { From ca38f724c8e4efc26d4aeef175109c6f1927dcda Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Tue, 31 May 2022 16:07:57 +0200 Subject: [PATCH 4/9] handle exceptions thrown in proxy factory --- tests/exception_proxy_004.phpt | 37 ++++++++++++++++++++++++++++++++++ v8js_object_export.cc | 7 ++++++- 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tests/exception_proxy_004.phpt diff --git a/tests/exception_proxy_004.phpt b/tests/exception_proxy_004.phpt new file mode 100644 index 0000000..dfa4287 --- /dev/null +++ b/tests/exception_proxy_004.phpt @@ -0,0 +1,37 @@ +--TEST-- +Test V8::setExceptionProxyFactory() : Proxy handling on exception in factory +--SKIPIF-- + +--FILE-- +setExceptionProxyFactory(function (Throwable $ex) { + throw new Exception('moep'); +}); + +try { + $v8->executeString(' + try { + PHP.throwException("Oops"); + print("done\\n"); + } + catch (e) { + print("caught\\n"); + var_dump(e); + } + ', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS); +} catch (Exception $ex) { + echo "caught in php: " . $ex->getMessage() . PHP_EOL; +} +?> +===EOF=== +--EXPECT-- +caught in php: moep +===EOF=== diff --git a/v8js_object_export.cc b/v8js_object_export.cc index d78fd51..e6e8354 100644 --- a/v8js_object_export.cc +++ b/v8js_object_export.cc @@ -53,7 +53,12 @@ v8::Local v8js_propagate_exception(v8js_ctx *ctx) /* {{{ */ call_user_function(EG(function_table), NULL, &ctx->exception_proxy_factory, &tmp_zv, 1, params); zval_ptr_dtor(¶ms[0]); - return_value = ctx->isolate->ThrowException(zval_to_v8js(&tmp_zv, ctx->isolate)); + if(EG(exception)) { + // exception proxy threw exception itself, don't forward, just stop execution. + v8js_terminate_execution(ctx->isolate); + } else { + return_value = ctx->isolate->ThrowException(zval_to_v8js(&tmp_zv, ctx->isolate)); + } } else { ZVAL_OBJ(&tmp_zv, EG(exception)); return_value = ctx->isolate->ThrowException(zval_to_v8js(&tmp_zv, ctx->isolate)); From abb8b6df148f84212decc76cd6d590bdacb8cbf6 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Tue, 31 May 2022 16:16:30 +0200 Subject: [PATCH 5/9] Mention setExceptionProxyFactory in README --- README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/README.md b/README.md index 64182da..bca66e5 100644 --- a/README.md +++ b/README.md @@ -104,6 +104,15 @@ class V8Js public function setModuleNormaliser(callable $normaliser) {} + /** + * Provate a function or method to be used to convert/proxy PHP exceptions to JS. + * This can be any valid PHP callable. + * The factory function will receive the PHP Exception instance that has not been caught and is + * due to be forwarded to JS. + */ + public function setExceptionProxyFactory(callable $factory) + {} + /** * Compiles and executes script in object's context with optional identifier string. * A time limit (milliseconds) and/or memory limit (bytes) can be provided to restrict execution. These options will throw a V8JsTimeLimitException or V8JsMemoryLimitException. @@ -401,3 +410,8 @@ 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. + +Consider that the JS code has access to methods like `getTrace` on the exception +object. This might be unwanted behaviour, if you execute untrusted code. +Using `setExceptionProxyFactory` method a callable can be provided, that converts +the PHP exception to not expose unwanted information. From 4bfe2ef3cef6c8f231fbdfb3b3cc37e872aa8f6e Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 24 Jun 2022 12:04:11 +0200 Subject: [PATCH 6/9] rename to setExceptionFilter --- README.md | 4 ++-- ...roxy_001.phpt => exception_filter_001.phpt} | 8 ++++---- ...roxy_002.phpt => exception_filter_002.phpt} | 8 ++++---- ...roxy_003.phpt => exception_filter_003.phpt} | 8 ++++---- ...roxy_004.phpt => exception_filter_004.phpt} | 4 ++-- ..._basic.phpt => exception_filter_basic.phpt} | 18 +++++++++--------- v8js_class.cc | 14 +++++++------- v8js_class.h | 2 +- v8js_object_export.cc | 4 ++-- 9 files changed, 35 insertions(+), 35 deletions(-) rename tests/{exception_proxy_001.phpt => exception_filter_001.phpt} (72%) rename tests/{exception_proxy_002.phpt => exception_filter_002.phpt} (64%) rename tests/{exception_proxy_003.phpt => exception_filter_003.phpt} (67%) rename tests/{exception_proxy_004.phpt => exception_filter_004.phpt} (85%) rename tests/{exception_proxy_basic.phpt => exception_filter_basic.phpt} (64%) diff --git a/README.md b/README.md index bca66e5..eab1a7d 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ class V8Js * The factory function will receive the PHP Exception instance that has not been caught and is * due to be forwarded to JS. */ - public function setExceptionProxyFactory(callable $factory) + public function setExceptionFilter(callable $factory) {} /** @@ -413,5 +413,5 @@ via `getPrevious` method. Consider that the JS code has access to methods like `getTrace` on the exception object. This might be unwanted behaviour, if you execute untrusted code. -Using `setExceptionProxyFactory` method a callable can be provided, that converts +Using `setExceptionFilter` method a callable can be provided, that converts the PHP exception to not expose unwanted information. diff --git a/tests/exception_proxy_001.phpt b/tests/exception_filter_001.phpt similarity index 72% rename from tests/exception_proxy_001.phpt rename to tests/exception_filter_001.phpt index 52c8286..115642e 100644 --- a/tests/exception_proxy_001.phpt +++ b/tests/exception_filter_001.phpt @@ -1,5 +1,5 @@ --TEST-- -Test V8::setExceptionProxyFactory() : String conversion +Test V8::setExceptionFilter() : String conversion --SKIPIF-- --FILE-- @@ -12,8 +12,8 @@ class myv8 extends V8Js } $v8 = new myv8(); -$v8->setExceptionProxyFactory(function (Throwable $ex) { - echo "exception proxy factory called.\n"; +$v8->setExceptionFilter(function (Throwable $ex) { + echo "exception filter called.\n"; return $ex->getMessage(); }); @@ -29,7 +29,7 @@ $v8->executeString(' ?> ===EOF=== --EXPECT-- -exception proxy factory called. +exception filter called. string(6) "string" string(4) "Oops" ===EOF=== diff --git a/tests/exception_proxy_002.phpt b/tests/exception_filter_002.phpt similarity index 64% rename from tests/exception_proxy_002.phpt rename to tests/exception_filter_002.phpt index 9174eb2..add31ee 100644 --- a/tests/exception_proxy_002.phpt +++ b/tests/exception_filter_002.phpt @@ -1,5 +1,5 @@ --TEST-- -Test V8::setExceptionProxyFactory() : Proxy handling on exception in setModuleLoader +Test V8::setExceptionFilter() : Filter handling on exception in setModuleLoader --SKIPIF-- --FILE-- @@ -10,8 +10,8 @@ $v8->setModuleLoader(function ($path) { throw new Error('moep'); }); -$v8->setExceptionProxyFactory(function (Throwable $ex) { - echo "exception proxy factory called.\n"; +$v8->setExceptionFilter(function (Throwable $ex) { + echo "exception filter called.\n"; return $ex->getMessage(); }); @@ -26,7 +26,7 @@ $v8->executeString(' ?> ===EOF=== --EXPECT-- -exception proxy factory called. +exception filter called. string(4) "moep" ===EOF=== diff --git a/tests/exception_proxy_003.phpt b/tests/exception_filter_003.phpt similarity index 67% rename from tests/exception_proxy_003.phpt rename to tests/exception_filter_003.phpt index fe70877..1caaab4 100644 --- a/tests/exception_proxy_003.phpt +++ b/tests/exception_filter_003.phpt @@ -1,5 +1,5 @@ --TEST-- -Test V8::setExceptionProxyFactory() : Proxy handling on exception in setModuleNormaliser +Test V8::setExceptionFilter() : Filter handling on exception in setModuleNormaliser --SKIPIF-- --FILE-- @@ -13,8 +13,8 @@ $v8->setModuleLoader(function ($path) { throw new Error('moep'); }); -$v8->setExceptionProxyFactory(function (Throwable $ex) { - echo "exception proxy factory called.\n"; +$v8->setExceptionFilter(function (Throwable $ex) { + echo "exception filter called.\n"; return $ex->getMessage(); }); @@ -29,6 +29,6 @@ $v8->executeString(' ?> ===EOF=== --EXPECT-- -exception proxy factory called. +exception filter called. string(5) "blarg" ===EOF=== diff --git a/tests/exception_proxy_004.phpt b/tests/exception_filter_004.phpt similarity index 85% rename from tests/exception_proxy_004.phpt rename to tests/exception_filter_004.phpt index dfa4287..a5dec89 100644 --- a/tests/exception_proxy_004.phpt +++ b/tests/exception_filter_004.phpt @@ -1,5 +1,5 @@ --TEST-- -Test V8::setExceptionProxyFactory() : Proxy handling on exception in factory +Test V8::setExceptionFilter() : Filter handling on exception in factory --SKIPIF-- --FILE-- @@ -12,7 +12,7 @@ class myv8 extends V8Js } $v8 = new myv8(); -$v8->setExceptionProxyFactory(function (Throwable $ex) { +$v8->setExceptionFilter(function (Throwable $ex) { throw new Exception('moep'); }); diff --git a/tests/exception_proxy_basic.phpt b/tests/exception_filter_basic.phpt similarity index 64% rename from tests/exception_proxy_basic.phpt rename to tests/exception_filter_basic.phpt index e7f8b3b..fca5305 100644 --- a/tests/exception_proxy_basic.phpt +++ b/tests/exception_filter_basic.phpt @@ -1,5 +1,5 @@ --TEST-- -Test V8::setExceptionProxyFactory() : Simple test +Test V8::setExceptionFilter() : Simple test --SKIPIF-- --FILE-- @@ -11,11 +11,11 @@ class myv8 extends V8Js } } -class ExceptionProxy { +class ExceptionFilter { private $ex; public function __construct(Throwable $ex) { - echo "ExceptionProxy::__construct called!\n"; + echo "ExceptionFilter::__construct called!\n"; var_dump($ex->getMessage()); $this->ex = $ex; @@ -28,9 +28,9 @@ class ExceptionProxy { } $v8 = new myv8(); -$v8->setExceptionProxyFactory(function (Throwable $ex) { - echo "exception proxy factory called.\n"; - return new ExceptionProxy($ex); +$v8->setExceptionFilter(function (Throwable $ex) { + echo "exception filter called.\n"; + return new ExceptionFilter($ex); }); $v8->executeString(' @@ -38,15 +38,15 @@ $v8->executeString(' PHP.throwException("Oops"); } catch (e) { - var_dump(e.getMessage()); // calls ExceptionProxy::getMessage + var_dump(e.getMessage()); // calls ExceptionFilter::getMessage var_dump(typeof e.getTrace); } ', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS); ?> ===EOF=== --EXPECT-- -exception proxy factory called. -ExceptionProxy::__construct called! +exception filter called. +ExceptionFilter::__construct called! string(4) "Oops" getMessage called string(4) "Oops" diff --git a/v8js_class.cc b/v8js_class.cc index 57f5571..044a8c5 100644 --- a/v8js_class.cc +++ b/v8js_class.cc @@ -92,7 +92,7 @@ static void v8js_free_storage(zend_object *object) /* {{{ */ zval_ptr_dtor(&c->pending_exception); zval_ptr_dtor(&c->module_normaliser); zval_ptr_dtor(&c->module_loader); - zval_ptr_dtor(&c->exception_proxy_factory); + zval_ptr_dtor(&c->exception_filter); /* Delete PHP global object from JavaScript */ if (!c->context.IsEmpty()) { @@ -401,7 +401,7 @@ static PHP_METHOD(V8Js, __construct) ZVAL_NULL(&c->module_normaliser); ZVAL_NULL(&c->module_loader); - ZVAL_NULL(&c->exception_proxy_factory); + ZVAL_NULL(&c->exception_filter); /* Include extensions used by this context */ /* Note: Extensions registered with auto_enable do not need to be added separately like this. */ @@ -882,9 +882,9 @@ static PHP_METHOD(V8Js, setModuleLoader) } /* }}} */ -/* {{{ proto void V8Js::setExceptionProxyFactory(callable factory) +/* {{{ proto void V8Js::setExceptionFilter(callable factory) */ -static PHP_METHOD(V8Js, setExceptionProxyFactory) +static PHP_METHOD(V8Js, setExceptionFilter) { zval *callable; @@ -893,7 +893,7 @@ static PHP_METHOD(V8Js, setExceptionProxyFactory) } v8js_ctx *c = Z_V8JS_CTX_OBJ_P(getThis()); - ZVAL_COPY(&c->exception_proxy_factory, callable); + ZVAL_COPY(&c->exception_filter, callable); } /* }}} */ @@ -1271,7 +1271,7 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_v8js_setmoduleloader, 0, 0, 1) ZEND_ARG_INFO(0, callable) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_INFO_EX(arginfo_v8js_setexceptionproxyfactory, 0, 0, 1) +ZEND_BEGIN_ARG_INFO_EX(arginfo_v8js_setexceptionfilter, 0, 0, 1) ZEND_ARG_INFO(0, callable) ZEND_END_ARG_INFO() @@ -1314,7 +1314,7 @@ const zend_function_entry v8js_methods[] = { /* {{{ */ PHP_ME(V8Js, clearPendingException, arginfo_v8js_clearpendingexception, ZEND_ACC_PUBLIC|ZEND_ACC_DEPRECATED) PHP_ME(V8Js, setModuleNormaliser, arginfo_v8js_setmodulenormaliser, ZEND_ACC_PUBLIC) PHP_ME(V8Js, setModuleLoader, arginfo_v8js_setmoduleloader, ZEND_ACC_PUBLIC) - PHP_ME(V8Js, setExceptionProxyFactory, arginfo_v8js_setexceptionproxyfactory, ZEND_ACC_PUBLIC) + PHP_ME(V8Js, setExceptionFilter, arginfo_v8js_setexceptionfilter, ZEND_ACC_PUBLIC) PHP_ME(V8Js, setTimeLimit, arginfo_v8js_settimelimit, ZEND_ACC_PUBLIC) PHP_ME(V8Js, setMemoryLimit, arginfo_v8js_setmemorylimit, ZEND_ACC_PUBLIC) PHP_ME(V8Js, setAverageObjectSize, arginfo_v8js_setaverageobjectsize, ZEND_ACC_PUBLIC) diff --git a/v8js_class.h b/v8js_class.h index 0fcdcd5..38a3ce5 100644 --- a/v8js_class.h +++ b/v8js_class.h @@ -55,7 +55,7 @@ struct v8js_ctx { zval module_normaliser; zval module_loader; - zval exception_proxy_factory; + zval exception_filter; std::vector modules_stack; std::map modules_loaded; diff --git a/v8js_object_export.cc b/v8js_object_export.cc index e6e8354..a657ff1 100644 --- a/v8js_object_export.cc +++ b/v8js_object_export.cc @@ -45,12 +45,12 @@ v8::Local v8js_propagate_exception(v8js_ctx *ctx) /* {{{ */ zval tmp_zv; - if (Z_TYPE(ctx->exception_proxy_factory) != IS_NULL) { + if (Z_TYPE(ctx->exception_filter) != IS_NULL) { zval params[1]; ZVAL_OBJ(¶ms[0], EG(exception)); Z_ADDREF_P(¶ms[0]); zend_clear_exception(); - call_user_function(EG(function_table), NULL, &ctx->exception_proxy_factory, &tmp_zv, 1, params); + call_user_function(EG(function_table), NULL, &ctx->exception_filter, &tmp_zv, 1, params); zval_ptr_dtor(¶ms[0]); if(EG(exception)) { From b635a16789be20a26b1d022fed3ba6126ab87f09 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 24 Jun 2022 12:55:08 +0200 Subject: [PATCH 7/9] add test on uninstallation of exception filter --- tests/exception_filter_005.phpt | 53 +++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 tests/exception_filter_005.phpt diff --git a/tests/exception_filter_005.phpt b/tests/exception_filter_005.phpt new file mode 100644 index 0000000..7813691 --- /dev/null +++ b/tests/exception_filter_005.phpt @@ -0,0 +1,53 @@ +--TEST-- +Test V8::setExceptionFilter() : Uninstall filter on NULL +--SKIPIF-- + +--FILE-- +setExceptionFilter(function (Throwable $ex) { + echo "exception filter called.\n"; + return "moep"; +}); + +$v8->executeString(' + try { + PHP.throwException("Oops"); + } + catch (e) { + var_dump(e); + } +', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS); + +$v8->setExceptionFilter(null); + +try { + $v8->executeString(' + try { + PHP.throwException("Oops"); + print("done\\n"); + } + catch (e) { + print("caught\\n"); + var_dump(e.getMessage()); + } + ', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS); +} catch (Exception $ex) { + echo "caught in php: " . $ex->getMessage() . PHP_EOL; +} + +?> +===EOF=== +--EXPECT-- +exception filter called. +string(4) "moep" +caught +string(4) "Oops" +===EOF=== From bf51bf52a90d2cfbbb957d9d9e2719e344df547e Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 24 Jun 2022 13:15:40 +0200 Subject: [PATCH 8/9] document (re-)throwing behaviour from exception filter --- README.md | 6 ++++-- tests/exception_filter_004.phpt | 2 +- tests/exception_filter_006.phpt | 38 +++++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 tests/exception_filter_006.phpt diff --git a/README.md b/README.md index eab1a7d..0155f2c 100644 --- a/README.md +++ b/README.md @@ -413,5 +413,7 @@ via `getPrevious` method. Consider that the JS code has access to methods like `getTrace` on the exception object. This might be unwanted behaviour, if you execute untrusted code. -Using `setExceptionFilter` method a callable can be provided, that converts -the PHP exception to not expose unwanted information. +Using `setExceptionFilter` method a callable can be provided, that may convert +the PHP exception to some other value that is safe to expose. The filter may +also decide not to propagate the exception to JS at all by either re-throwing +the passed exception or throwing another exception. diff --git a/tests/exception_filter_004.phpt b/tests/exception_filter_004.phpt index a5dec89..b35e3fc 100644 --- a/tests/exception_filter_004.phpt +++ b/tests/exception_filter_004.phpt @@ -1,5 +1,5 @@ --TEST-- -Test V8::setExceptionFilter() : Filter handling on exception in factory +Test V8::setExceptionFilter() : Filter handling on exception in converter --SKIPIF-- --FILE-- diff --git a/tests/exception_filter_006.phpt b/tests/exception_filter_006.phpt new file mode 100644 index 0000000..bf62fce --- /dev/null +++ b/tests/exception_filter_006.phpt @@ -0,0 +1,38 @@ +--TEST-- +Test V8::setExceptionFilter() : re-throw exception in exception filter +--SKIPIF-- + +--FILE-- +setExceptionFilter(function (Throwable $ex) { + // re-throw exception so it is not forwarded + throw $ex; +}); + +try { + $v8->executeString(' + try { + PHP.throwException("Oops"); + print("done\\n"); + } + catch (e) { + print("caught\\n"); + var_dump(e); + } + ', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS); +} catch (Exception $ex) { + echo "caught in php: " . $ex->getMessage() . PHP_EOL; +} +?> +===EOF=== +--EXPECT-- +caught in php: Oops +===EOF=== From dcb583267a9e135577b32fb559acfb0b61b3be71 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 24 Jun 2022 13:17:10 +0200 Subject: [PATCH 9/9] document how to uninstall a filter --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0155f2c..fab78da 100644 --- a/README.md +++ b/README.md @@ -107,10 +107,10 @@ class V8Js /** * Provate a function or method to be used to convert/proxy PHP exceptions to JS. * This can be any valid PHP callable. - * The factory function will receive the PHP Exception instance that has not been caught and is - * due to be forwarded to JS. + * The converter function will receive the PHP Exception instance that has not been caught and + * is due to be forwarded to JS. Pass NULL as $filter to uninstall an existing filter. */ - public function setExceptionFilter(callable $factory) + public function setExceptionFilter(callable $filter) {} /**