From ac2b1cb23852466442dbefc17cf628a6062d5c61 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Tue, 31 May 2022 14:21:32 +0200 Subject: [PATCH 01/16] 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 02/16] 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 03/16] 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 04/16] 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 05/16] 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 06/16] 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 07/16] 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 08/16] 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 09/16] 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) {} /** From 225c8cde6380721691d0668e4c8874b3de6fa770 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 24 Jun 2022 21:27:33 +0200 Subject: [PATCH 10/16] test compiler for c++17 support --- config.m4 | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/config.m4 b/config.m4 index bb823f4..0cccd8c 100644 --- a/config.m4 +++ b/config.m4 @@ -39,11 +39,15 @@ if test "$PHP_V8JS" != "no"; then AC_CACHE_CHECK(for C standard version, ac_cv_v8_cstd, [ - ac_cv_v8_cstd="c++14" + ac_cv_v8_cstd="c++17" old_CPPFLAGS=$CPPFLAGS AC_LANG_PUSH([C++]) CPPFLAGS="-std="$ac_cv_v8_cstd - AC_RUN_IFELSE([AC_LANG_SOURCE([[int main() { return 0; }]])],[],[ac_cv_v8_cstd="c++1y"],[]) + AC_RUN_IFELSE([AC_LANG_SOURCE([[int main() { return 0; }]])],[],[ + ac_cv_v8_cstd="c++14" + CPPFLAGS="-std="$ac_cv_v8_cstd + AC_RUN_IFELSE([AC_LANG_SOURCE([[int main() { return 0; }]])],[],[ ac_cv_v8_cstd="c++1y" ],[]) + ],[]) AC_LANG_POP([C++]) CPPFLAGS=$old_CPPFLAGS ]); From 19111d65147f063cd27e75d09948a95fa73fe896 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 24 Jun 2022 21:35:58 +0200 Subject: [PATCH 11/16] ShutdownPlatform is DisposePlatform() now --- v8js_main.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v8js_main.cc b/v8js_main.cc index 66b58ef..5fa08e7 100644 --- a/v8js_main.cc +++ b/v8js_main.cc @@ -162,7 +162,7 @@ static PHP_MSHUTDOWN_FUNCTION(v8js) if(v8_initialized) { v8::V8::Dispose(); - v8::V8::ShutdownPlatform(); + v8::V8::DisposePlatform(); // @fixme call virtual destructor somehow //delete v8js_process_globals.v8_platform; } From 883c9000c3ed6ae293006af967dd7f2ae41b6d41 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 24 Jun 2022 21:47:28 +0200 Subject: [PATCH 12/16] accessor signature checks are no longer supported --- v8js_variables.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v8js_variables.cc b/v8js_variables.cc index 5d6d9d8..f546f79 100644 --- a/v8js_variables.cc +++ b/v8js_variables.cc @@ -80,7 +80,7 @@ void v8js_register_accessors(std::vector *accessor_list, v8: ctx->isolate = isolate; /* Set the variable fetch callback for given symbol on named property */ - php_obj->SetAccessor(V8JS_STRL(ZSTR_VAL(property_name), static_cast(ZSTR_LEN(property_name))), v8js_fetch_php_variable, NULL, v8::External::New(isolate, ctx), v8::PROHIBITS_OVERWRITING, v8::ReadOnly, v8::AccessorSignature::New(isolate, php_obj_t)); + php_obj->SetAccessor(V8JS_STRL(ZSTR_VAL(property_name), static_cast(ZSTR_LEN(property_name))), v8js_fetch_php_variable, NULL, v8::External::New(isolate, ctx), v8::PROHIBITS_OVERWRITING, v8::ReadOnly); /* record the context so we can free it later */ accessor_list->push_back(ctx); From 23cc21c5b86ec1250dd2e6ca7b9d9abb9ac9a0cd Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 24 Jun 2022 21:54:08 +0200 Subject: [PATCH 13/16] call InitializeSandbox --- v8js_v8.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/v8js_v8.cc b/v8js_v8.cc index 8d8f99a..81bfb09 100644 --- a/v8js_v8.cc +++ b/v8js_v8.cc @@ -71,6 +71,10 @@ void v8js_v8_init() /* {{{ */ v8js_process_globals.v8_platform = v8::platform::NewDefaultPlatform(); v8::V8::InitializePlatform(v8js_process_globals.v8_platform.get()); +#ifdef V8_ENABLE_SANDBOX + v8::V8::InitializeSandbox(); +#endif + /* Set V8 command line flags (must be done before V8::Initialize()!) */ if (v8js_process_globals.v8_flags) { size_t flags_len = strlen(v8js_process_globals.v8_flags); From 32d8dd8dec073b000ee42570e60fecb5881b0f72 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 24 Jun 2022 22:24:52 +0200 Subject: [PATCH 14/16] conditionally call DisposePlatform or ShutdownPlatform --- v8js_main.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/v8js_main.cc b/v8js_main.cc index 5fa08e7..51efd71 100644 --- a/v8js_main.cc +++ b/v8js_main.cc @@ -162,7 +162,11 @@ static PHP_MSHUTDOWN_FUNCTION(v8js) if(v8_initialized) { v8::V8::Dispose(); +#if PHP_V8_API_VERSION >= 10000000 v8::V8::DisposePlatform(); +#else + v8::V8::ShutdownPlatform(); +#endif // @fixme call virtual destructor somehow //delete v8js_process_globals.v8_platform; } From 26de750e8e568c9c3c33018352807efdbb60c0d2 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Mon, 27 Jun 2022 18:00:49 +0200 Subject: [PATCH 15/16] auto-detect V8 sandbox and call InitializeSandbox if needed --- config.m4 | 18 ++++++++++++++++++ v8js_v8.cc | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/config.m4 b/config.m4 index 0cccd8c..9cb3b16 100644 --- a/config.m4 +++ b/config.m4 @@ -177,6 +177,24 @@ int main () V8_SEARCH_BLOB([snapshot_blob.bin], [PHP_V8_SNAPSHOT_BLOB_PATH]) + dnl + dnl Check for v8::V8::InitializeSandbox + dnl + AC_CACHE_CHECK([for v8::V8::InitializeSandbox], ac_cv_has_initialize_sandbox, [ + AC_LINK_IFELSE([AC_LANG_PROGRAM([ + #define V8_ENABLE_SANDBOX 1 + #include + ], [ v8::V8::InitializeSandbox(); ])], [ + ac_cv_has_initialize_sandbox=yes + ], [ + ac_cv_has_initialize_sandbox=no + ]) + ]) + if test "x$ac_cv_has_initialize_sandbox" = "xyes"; then + AC_DEFINE([V8_HAS_INITIALIZE_SANDBOX], [1], + [Define if V8::InitializeSandbox must be called.]) + fi + dnl dnl Check for v8::ArrayBuffer::Allocator::NewDefaultAllocator dnl diff --git a/v8js_v8.cc b/v8js_v8.cc index 81bfb09..4437801 100644 --- a/v8js_v8.cc +++ b/v8js_v8.cc @@ -71,7 +71,7 @@ void v8js_v8_init() /* {{{ */ v8js_process_globals.v8_platform = v8::platform::NewDefaultPlatform(); v8::V8::InitializePlatform(v8js_process_globals.v8_platform.get()); -#ifdef V8_ENABLE_SANDBOX +#ifdef V8_HAS_INITIALIZE_SANDBOX v8::V8::InitializeSandbox(); #endif From fa264c9ec5a6229d9b2a8f30f23734ff7ce7261d Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 3 Feb 2023 19:04:56 +0100 Subject: [PATCH 16/16] define V8_ENABLE_SANDBOX if needed --- php_v8js_macros.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/php_v8js_macros.h b/php_v8js_macros.h index eb6d811..a56c165 100644 --- a/php_v8js_macros.h +++ b/php_v8js_macros.h @@ -52,6 +52,10 @@ extern "C" { #undef COMPILER #endif +#ifdef V8_HAS_INITIALIZE_SANDBOX +#define V8_ENABLE_SANDBOX 1 +#endif + #include #include