Merge pull request #487 from stesie/exception-proxy-class

introduce V8Js::setExceptionProxyFactory

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 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 $filter)
+    {}
+
     /**
      * 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,10 @@ 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 `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.

tests/exception_filter_001.phpt

@@ -0,0 +1,35 @@
+--TEST--
+Test V8::setExceptionFilter() : String conversion
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+class myv8 extends V8Js
+{
+	public function throwException(string $message) {
+		throw new Exception($message);
+	}
+}
+
+$v8 = new myv8();
+$v8->setExceptionFilter(function (Throwable $ex) {
+	echo "exception filter 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 filter called.
+string(6) "string"
+string(4) "Oops"
+===EOF===

tests/exception_filter_002.phpt

@@ -0,0 +1,32 @@
+--TEST--
+Test V8::setExceptionFilter() : Filter handling on exception in setModuleLoader
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+$v8 = new V8Js();
+$v8->setModuleLoader(function ($path) {
+	throw new Error('moep');
+});
+
+$v8->setExceptionFilter(function (Throwable $ex) {
+	echo "exception filter called.\n";
+	return $ex->getMessage();
+});
+
+$v8->executeString('
+	try {
+		require("file");
+	} catch(e) {
+		var_dump(e);
+	}
+', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS);
+
+?>
+===EOF===
+--EXPECT--
+exception filter called.
+string(4) "moep"
+===EOF===
+

tests/exception_filter_003.phpt

@@ -0,0 +1,34 @@
+--TEST--
+Test V8::setExceptionFilter() : Filter handling on exception in setModuleNormaliser
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+$v8 = new V8Js();
+$v8->setModuleNormaliser(function ($path) {
+	throw new Error('blarg');
+});
+$v8->setModuleLoader(function ($path) {
+	throw new Error('moep');
+});
+
+$v8->setExceptionFilter(function (Throwable $ex) {
+	echo "exception filter called.\n";
+	return $ex->getMessage();
+});
+
+$v8->executeString('
+	try {
+		require("file");
+	} catch(e) {
+		var_dump(e);
+	}
+', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS);
+
+?>
+===EOF===
+--EXPECT--
+exception filter called.
+string(5) "blarg"
+===EOF===

tests/exception_filter_004.phpt

@@ -0,0 +1,37 @@
+--TEST--
+Test V8::setExceptionFilter() : Filter handling on exception in converter
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+class myv8 extends V8Js
+{
+        public function throwException(string $message) {
+                throw new Exception($message);
+        }
+}
+
+$v8 = new myv8();
+$v8->setExceptionFilter(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===

tests/exception_filter_005.phpt

@@ -0,0 +1,53 @@
+--TEST--
+Test V8::setExceptionFilter() : Uninstall filter on NULL
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+class myv8 extends V8Js
+{
+	public function throwException(string $message) {
+		throw new Exception($message);
+	}
+}
+
+$v8 = new myv8();
+$v8->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===

tests/exception_filter_006.phpt

@@ -0,0 +1,38 @@
+--TEST--
+Test V8::setExceptionFilter() : re-throw exception in exception filter
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+class myv8 extends V8Js
+{
+        public function throwException(string $message) {
+                throw new Exception($message);
+        }
+}
+
+$v8 = new myv8();
+$v8->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===

tests/exception_filter_basic.phpt

@@ -0,0 +1,55 @@
+--TEST--
+Test V8::setExceptionFilter() : Simple test
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+class myv8 extends V8Js
+{
+	public function throwException(string $message) {
+		throw new Exception($message);
+	}
+}
+
+class ExceptionFilter {
+	private $ex;
+
+	public function __construct(Throwable $ex) {
+		echo "ExceptionFilter::__construct called!\n";
+		var_dump($ex->getMessage());
+
+		$this->ex = $ex;
+	}
+
+	public function getMessage() {
+		echo "getMessage called\n";
+		return $this->ex->getMessage();
+	}
+}
+
+$v8 = new myv8();
+$v8->setExceptionFilter(function (Throwable $ex) {
+	echo "exception filter called.\n";
+	return new ExceptionFilter($ex);
+});
+
+$v8->executeString('
+	try {
+		PHP.throwException("Oops");
+	}
+	catch (e) {
+		var_dump(e.getMessage()); // calls ExceptionFilter::getMessage
+		var_dump(typeof e.getTrace);
+	}
+', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS);
+?>
+===EOF===
+--EXPECT--
+exception filter called.
+ExceptionFilter::__construct called!
+string(4) "Oops"
+getMessage called
+string(4) "Oops"
+string(9) "undefined"
+===EOF===
+

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_filter);
 
 	/* 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_filter);
 
 	/* 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::setExceptionFilter(callable factory)
+ */
+static PHP_METHOD(V8Js, setExceptionFilter)
+{
+	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_filter, 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_setexceptionfilter, 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,	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)

v8js_class.h

@@ -55,6 +55,7 @@ struct v8js_ctx {
 
   zval module_normaliser;
   zval module_loader;
+  zval exception_filter;
 
   std::vector<char *> modules_stack;
   std::map<char *, v8js_persistent_value_t, cmp_str> modules_loaded;

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) {
+			info.GetReturnValue().Set(v8js_propagate_exception(c));
-				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);
-			}
 			return;
 		}
 
@@ -466,15 +460,7 @@ V8JS_METHOD(require)
 		efree(normalised_module_id);
 		efree(normalised_path);
 
-		if (c->flags & V8JS_FLAG_PROPAGATE_PHP_EXCEPTIONS) {
+		info.GetReturnValue().Set(v8js_propagate_exception(c));
-			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);
-		}
-
 		return;
 	}
 	
@@ -485,7 +471,7 @@ V8JS_METHOD(require)
 
 		efree(normalised_path);
 		return;
-        }
+	}
 
 	if(Z_TYPE(module_code) == IS_OBJECT) {
 		v8::Local<v8::Object> newobj = zval_to_v8js(&module_code, isolate)->ToObject(isolate->GetEnteredOrMicrotaskContext()).ToLocalChecked();

v8js_object_export.cc

@@ -34,6 +34,41 @@ extern "C" {
 
 static void v8js_weak_object_callback(const v8::WeakCallbackInfo<zend_object> &data);
 
+v8::Local<v8::Value> v8js_propagate_exception(v8js_ctx *ctx) /* {{{ */
+{
+	v8::Local<v8::Value> 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_filter) != IS_NULL) {
+		zval params[1];
+		ZVAL_OBJ(&params[0], EG(exception));
+		Z_ADDREF_P(&params[0]);
+		zend_clear_exception();
+		call_user_function(EG(function_table), NULL, &ctx->exception_filter, &tmp_zv, 1, params);
+		zval_ptr_dtor(&params[0]);
+
+		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));
+		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<v8::Value>& info) /* {{{ */
 {
@@ -175,14 +210,7 @@ failure:
 	}
 
 	if(EG(exception)) {
-		if(ctx->flags & V8JS_FLAG_PROPAGATE_PHP_EXCEPTIONS) {
+		return_value = v8js_propagate_exception(ctx);
-			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);
-		}
 	} else if (Z_TYPE(retval) == IS_OBJECT && Z_OBJ(retval) == object) {
 		// special case: "return $this"
 		return_value = info.Holder();

v8js_object_export.h

@@ -15,6 +15,7 @@
 #define V8JS_OBJECT_EXPORT_H
 
 v8::Local<v8::Value> v8js_hash_to_jsobj(zval *value, v8::Isolate *isolate);
+v8::Local<v8::Value> v8js_propagate_exception(v8js_ctx *ctx);
 
 
 typedef enum {