From 2f0b8e28735aa505bde934cb56a6d74b805053ba Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 4 Dec 2015 21:37:51 +0100 Subject: [PATCH 1/9] Make var_dump command available to modules as well --- v8js_methods.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/v8js_methods.cc b/v8js_methods.cc index c16eda5..ed488b2 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -297,6 +297,7 @@ V8JS_METHOD(require) // Create a template for the global object and set the built-in global functions v8::Handle global = v8::ObjectTemplate::New(); global->Set(V8JS_SYM("print"), v8::FunctionTemplate::New(isolate, V8JS_MN(print)), v8::ReadOnly); + global->Set(V8JS_SYM("var_dump"), v8::FunctionTemplate::New(isolate, V8JS_MN(var_dump)), v8::ReadOnly); global->Set(V8JS_SYM("sleep"), v8::FunctionTemplate::New(isolate, V8JS_MN(sleep)), v8::ReadOnly); global->Set(V8JS_SYM("require"), v8::FunctionTemplate::New(isolate, V8JS_MN(require), v8::External::New(isolate, c)), v8::ReadOnly); From 39fff2301e9fdfd5d7e0833bedf101bedac0a15e Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 4 Dec 2015 21:46:54 +0100 Subject: [PATCH 2/9] Use module id as JsFileName for V8 This way the information to V8JsScriptException instances are way more clear since they contain the name of the module that caused the exception. --- tests/commonjs_source_naming.phpt | 27 +++++++++++++++++++++++++++ v8js_methods.cc | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 tests/commonjs_source_naming.phpt diff --git a/tests/commonjs_source_naming.phpt b/tests/commonjs_source_naming.phpt new file mode 100644 index 0000000..b57acfa --- /dev/null +++ b/tests/commonjs_source_naming.phpt @@ -0,0 +1,27 @@ +--TEST-- +Test V8Js::setModuleLoader : Module source naming +--SKIPIF-- + +--FILE-- +setModuleLoader(function($module) { + // return code with syntax errors to provoke script exception + return "foo(blar);"; +}); + +try { + $v8->executeString($JS, 'commonjs_source_naming.js'); +} catch (V8JsScriptException $e) { + var_dump($e->getJsFileName()); +} +?> +===EOF=== +--EXPECT-- +string(7) "foo/bar" +===EOF=== diff --git a/v8js_methods.cc b/v8js_methods.cc index ed488b2..889a707 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -326,7 +326,7 @@ V8JS_METHOD(require) // Enter the module context v8::Context::Scope scope(context); // Set script identifier - v8::Local sname = V8JS_SYM("require"); + v8::Local sname = V8JS_SYM(normalised_module_id); v8::Local source = V8JS_STRL(Z_STRVAL_P(module_code), Z_STRLEN_P(module_code)); zval_ptr_dtor(&module_code); From bf58fe67c13e3a9a06af9128cd2d44fe502a45c8 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 4 Dec 2015 22:09:04 +0100 Subject: [PATCH 3/9] wrap module loading in zend_try/zend_catch, closes #178 --- tests/commonjs_fatal_error.phpt | 17 +++++++++++++++++ v8js_methods.cc | 29 +++++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 tests/commonjs_fatal_error.phpt diff --git a/tests/commonjs_fatal_error.phpt b/tests/commonjs_fatal_error.phpt new file mode 100644 index 0000000..f246adf --- /dev/null +++ b/tests/commonjs_fatal_error.phpt @@ -0,0 +1,17 @@ +--TEST-- +Test V8Js::setModuleLoader : Handle fatal errors gracefully +--SKIPIF-- + +--FILE-- +setModuleLoader(function() { + trigger_error('some fatal error', E_USER_ERROR); +}); + +$v8->executeString(' require("foo"); '); +?> +===EOF=== +--EXPECTF-- +Fatal error: some fatal error in %s%ecommonjs_fatal_error.php on line 5 diff --git a/v8js_methods.cc b/v8js_methods.cc index 889a707..afa0cd8 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -257,13 +257,34 @@ V8JS_METHOD(require) MAKE_STD_ZVAL(normalised_path_zend); ZVAL_STRING(normalised_path_zend, normalised_module_id, 1); - zval **params[1] = {&normalised_path_zend}; - if (FAILURE == call_user_function_ex(EG(function_table), NULL, c->module_loader, &module_code, 1, params, 0, NULL TSRMLS_CC)) { + int call_result; + + zend_try { + { + isolate->Exit(); + v8::Unlocker unlocker(isolate); + + zval **params[1] = {&normalised_path_zend}; + call_result = call_user_function_ex(EG(function_table), NULL, c->module_loader, &module_code, 1, params, 0, NULL TSRMLS_CC); + } + + isolate->Enter(); + + if (call_result == FAILURE) { + info.GetReturnValue().Set(isolate->ThrowException(V8JS_SYM("Module loader callback failed"))); + } + } + zend_catch { + v8js_terminate_execution(isolate); + V8JSG(fatal_error_abort) = 1; + call_result = FAILURE; + } + zend_end_try(); + + if (call_result == FAILURE) { zval_ptr_dtor(&normalised_path_zend); efree(normalised_module_id); efree(normalised_path); - - info.GetReturnValue().Set(isolate->ThrowException(V8JS_SYM("Module loader callback failed"))); return; } zval_ptr_dtor(&normalised_path_zend); From 4853c6d17f40d86b137105744708be424fa218e0 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 6 Dec 2015 13:23:14 +0100 Subject: [PATCH 4/9] Set script identifier as String, not Symbol Otherwise long module identifiers might get cut off. --- v8js_methods.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v8js_methods.cc b/v8js_methods.cc index afa0cd8..4395559 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -347,7 +347,7 @@ V8JS_METHOD(require) // Enter the module context v8::Context::Scope scope(context); // Set script identifier - v8::Local sname = V8JS_SYM(normalised_module_id); + v8::Local sname = V8JS_STR(normalised_module_id); v8::Local source = V8JS_STRL(Z_STRVAL_P(module_code), Z_STRLEN_P(module_code)); zval_ptr_dtor(&module_code); From f2589803997ba675f00f57937eff9d687040f339 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 6 Dec 2015 13:25:43 +0100 Subject: [PATCH 5/9] Accept empty string as module source This might be perfectly valid, if you're using a third-party module, which requires a module yet doesn't use it in the code paths hit and hence you just want to stub it out. --- v8js_methods.cc | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/v8js_methods.cc b/v8js_methods.cc index 4395559..7757c0e 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -305,16 +305,6 @@ V8JS_METHOD(require) convert_to_string(module_code); } - // Check that some code has been returned - if (Z_STRLEN_P(module_code)==0) { - zval_ptr_dtor(&module_code); - efree(normalised_module_id); - efree(normalised_path); - - info.GetReturnValue().Set(isolate->ThrowException(V8JS_SYM("Module loader callback did not return code"))); - return; - } - // Create a template for the global object and set the built-in global functions v8::Handle global = v8::ObjectTemplate::New(); global->Set(V8JS_SYM("print"), v8::FunctionTemplate::New(isolate, V8JS_MN(print)), v8::ReadOnly); From 67a9de01bdd6f6057c5c9e136fa7fd1919a4bbca Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 6 Dec 2015 13:55:13 +0100 Subject: [PATCH 6/9] Allow custom module normalisation --- README.md | 13 ++++ tests/commonjs_cust_normalise_001.phpt | 31 ++++++++ tests/commonjs_cust_normalise_002.phpt | 33 ++++++++ tests/commonjs_cust_normalise_003.phpt | 41 ++++++++++ tests/commonjs_cust_normalise_004.phpt | 42 ++++++++++ v8js_class.cc | 29 +++++++ v8js_class.h | 1 + v8js_methods.cc | 103 ++++++++++++++++++++++++- 8 files changed, 289 insertions(+), 4 deletions(-) create mode 100644 tests/commonjs_cust_normalise_001.phpt create mode 100644 tests/commonjs_cust_normalise_002.phpt create mode 100644 tests/commonjs_cust_normalise_003.phpt create mode 100644 tests/commonjs_cust_normalise_004.phpt diff --git a/README.md b/README.md index 8cf45a3..18ce894 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,19 @@ class V8Js public function setModuleLoader(callable $loader) {} + /** + * Provide a function or method to be used to normalise module paths. This can be any valid PHP callable. + * This can be used in combination with setModuleLoader to influence normalisation of the module path (which + * is normally done by V8Js itself but can be overriden this way). + * The normaliser function will receive the base path of the current module (if any; otherwise an empty string) + * and the literate string provided to the require method and should return an array of two strings (the new + * module base path as well as the normalised name). Both are joined by a '/' and then passed on to the + * module loader (unless the module was cached before). + * @param callable $normaliser + */ + public function setModuleNormaliser(callable $normaliser) + {} + /** * 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. diff --git a/tests/commonjs_cust_normalise_001.phpt b/tests/commonjs_cust_normalise_001.phpt new file mode 100644 index 0000000..c9b9edf --- /dev/null +++ b/tests/commonjs_cust_normalise_001.phpt @@ -0,0 +1,31 @@ +--TEST-- +Test V8Js::setModuleNormaliser : Custom normalisation #001 +--SKIPIF-- + +--FILE-- +setModuleNormaliser(function($base, $module) { + var_dump($base, $module); + return [ "", "test" ]; +}); + +$v8->setModuleLoader(function($module) { + print("setModuleLoader called for ".$module."\n"); + return 'exports.bar = 23;'; +}); + +$v8->executeString($JS, 'module.js'); +?> +===EOF=== +--EXPECT-- +string(0) "" +string(6) "./test" +setModuleLoader called for test +===EOF=== diff --git a/tests/commonjs_cust_normalise_002.phpt b/tests/commonjs_cust_normalise_002.phpt new file mode 100644 index 0000000..792ebaa --- /dev/null +++ b/tests/commonjs_cust_normalise_002.phpt @@ -0,0 +1,33 @@ +--TEST-- +Test V8Js::setModuleNormaliser : Custom normalisation #002 +--SKIPIF-- + +--FILE-- +setModuleNormaliser(function($base, $module) { + var_dump($base, $module); + return [ "path/to", "test-foo" ]; +}); + +$v8->setModuleLoader(function($module) { + print("setModuleLoader called for ".$module."\n"); + return 'exports.bar = 23;'; +}); + +$v8->executeString($JS, 'module.js'); +?> +===EOF=== +--EXPECT-- +string(0) "" +string(6) "./test" +setModuleLoader called for path/to/test-foo +===EOF=== diff --git a/tests/commonjs_cust_normalise_003.phpt b/tests/commonjs_cust_normalise_003.phpt new file mode 100644 index 0000000..c417eb6 --- /dev/null +++ b/tests/commonjs_cust_normalise_003.phpt @@ -0,0 +1,41 @@ +--TEST-- +Test V8Js::setModuleNormaliser : Custom normalisation #003 +--SKIPIF-- + +--FILE-- +setModuleNormaliser(function($base, $module) { + var_dump($base, $module); + return [ "path/to", "test-foo" ]; +}); + +$v8->setModuleLoader(function($module) { + print("setModuleLoader called for ".$module."\n"); + if($module != "path/to/test-foo") { + throw new \Exception("module caching fails"); + } + return 'exports.bar = 23;'; +}); + +$v8->executeString($JS, 'module.js'); +?> +===EOF=== +--EXPECT-- +string(0) "" +string(6) "./test" +setModuleLoader called for path/to/test-foo +string(0) "" +string(4) "test" +===EOF=== diff --git a/tests/commonjs_cust_normalise_004.phpt b/tests/commonjs_cust_normalise_004.phpt new file mode 100644 index 0000000..3086396 --- /dev/null +++ b/tests/commonjs_cust_normalise_004.phpt @@ -0,0 +1,42 @@ +--TEST-- +Test V8Js::setModuleNormaliser : Custom normalisation #004 +--SKIPIF-- + +--FILE-- +setModuleNormaliser(function($base, $module) { + var_dump($base, $module); + return [ "path/to", $module ]; +}); + +$v8->setModuleLoader(function($module) { + print("setModuleLoader called for ".$module."\n"); + switch($module) { + case "path/to/foo": + return "require('bar');"; + + case "path/to/bar": + return 'exports.bar = 23;'; + } +}); + +$v8->executeString($JS, 'module.js'); +?> +===EOF=== +--EXPECT-- +string(0) "" +string(3) "foo" +setModuleLoader called for path/to/foo +string(7) "path/to" +string(3) "bar" +setModuleLoader called for path/to/bar +===EOF=== diff --git a/v8js_class.cc b/v8js_class.cc index 8faec51..f538a82 100644 --- a/v8js_class.cc +++ b/v8js_class.cc @@ -89,6 +89,10 @@ static void v8js_free_storage(void *object TSRMLS_DC) /* {{{ */ zval_ptr_dtor(&c->pending_exception); } + if (c->module_normaliser) { + zval_ptr_dtor(&c->module_normaliser); + } + if (c->module_loader) { zval_ptr_dtor(&c->module_loader); } @@ -362,6 +366,7 @@ static PHP_METHOD(V8Js, __construct) c->memory_limit = 0; c->memory_limit_hit = false; + c->module_normaliser = NULL; c->module_loader = NULL; /* Include extensions used by this context */ @@ -687,6 +692,24 @@ static PHP_METHOD(V8Js, clearPendingException) } /* }}} */ +/* {{{ proto void V8Js::setModuleNormaliser(string base, string module_id) + */ +static PHP_METHOD(V8Js, setModuleNormaliser) +{ + v8js_ctx *c; + zval *callable; + + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "z", &callable) == FAILURE) { + return; + } + + c = (v8js_ctx *) zend_object_store_get_object(getThis() TSRMLS_CC); + + c->module_normaliser = callable; + Z_ADDREF_P(c->module_normaliser); +} +/* }}} */ + /* {{{ proto void V8Js::setModuleLoader(string module) */ static PHP_METHOD(V8Js, setModuleLoader) @@ -1005,6 +1028,11 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO(arginfo_v8js_clearpendingexception, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_INFO_EX(arginfo_v8js_setmodulenormaliser, 0, 0, 2) + ZEND_ARG_INFO(0, base) + ZEND_ARG_INFO(0, module_id) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_INFO_EX(arginfo_v8js_setmoduleloader, 0, 0, 1) ZEND_ARG_INFO(0, callable) ZEND_END_ARG_INFO() @@ -1038,6 +1066,7 @@ static const zend_function_entry v8js_methods[] = { /* {{{ */ PHP_ME(V8Js, checkString, arginfo_v8js_checkstring, ZEND_ACC_PUBLIC|ZEND_ACC_DEPRECATED) PHP_ME(V8Js, getPendingException, arginfo_v8js_getpendingexception, ZEND_ACC_PUBLIC) PHP_ME(V8Js, clearPendingException, arginfo_v8js_clearpendingexception, ZEND_ACC_PUBLIC) + PHP_ME(V8Js, setModuleNormaliser, arginfo_v8js_setmodulenormaliser, ZEND_ACC_PUBLIC) PHP_ME(V8Js, setModuleLoader, arginfo_v8js_setmoduleloader, ZEND_ACC_PUBLIC) PHP_ME(V8Js, registerExtension, arginfo_v8js_registerextension, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) PHP_ME(V8Js, getExtensions, arginfo_v8js_getextensions, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) diff --git a/v8js_class.h b/v8js_class.h index 873591a..ab7b7aa 100644 --- a/v8js_class.h +++ b/v8js_class.h @@ -50,6 +50,7 @@ struct v8js_ctx { v8js_tmpl_t global_template; v8js_tmpl_t array_tmpl; + zval *module_normaliser; zval *module_loader; std::vector modules_stack; std::vector modules_base; diff --git a/v8js_methods.cc b/v8js_methods.cc index 7757c0e..8832df2 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -8,6 +8,7 @@ +----------------------------------------------------------------------+ | Author: Jani Taskinen | | Author: Patrick Reilly | + | Author: Stefan Siegl | +----------------------------------------------------------------------+ */ @@ -207,12 +208,106 @@ V8JS_METHOD(require) } v8::String::Utf8Value module_id_v8(info[0]); - const char *module_id = ToCString(module_id_v8); - char *normalised_path = (char *)emalloc(PATH_MAX); - char *module_name = (char *)emalloc(PATH_MAX); + char *normalised_path, *module_name; - v8js_commonjs_normalise_identifier(c->modules_base.back(), module_id, normalised_path, module_name); + if (c->module_normaliser == NULL) { + // No custom normalisation routine registered, use internal one + normalised_path = (char *)emalloc(PATH_MAX); + module_name = (char *)emalloc(PATH_MAX); + + v8js_commonjs_normalise_identifier(c->modules_base.back(), module_id, normalised_path, module_name); + } + else { + // Call custom normaliser + int call_result; + zval *z_base, *z_module_id, *normaliser_result; + + MAKE_STD_ZVAL(z_base); + MAKE_STD_ZVAL(z_module_id); + + zend_try { + { + isolate->Exit(); + v8::Unlocker unlocker(isolate); + + ZVAL_STRING(z_base, c->modules_base.back(), 1); + ZVAL_STRING(z_module_id, module_id, 1); + + zval **params[2] = {&z_base, &z_module_id}; + call_result = call_user_function_ex(EG(function_table), NULL, c->module_normaliser, + &normaliser_result, 2, params, 0, NULL TSRMLS_CC); + } + + isolate->Enter(); + + if (call_result == FAILURE) { + info.GetReturnValue().Set(isolate->ThrowException(V8JS_SYM("Module normaliser callback failed"))); + } + } + zend_catch { + v8js_terminate_execution(isolate); + V8JSG(fatal_error_abort) = 1; + call_result = FAILURE; + } + zend_end_try(); + + zval_ptr_dtor(&z_base); + zval_ptr_dtor(&z_module_id); + + if(call_result == FAILURE) { + return; + } + + // Check if an exception was thrown + if (EG(exception)) { + // Clear the PHP exception and throw it in V8 instead + zend_clear_exception(TSRMLS_C); + info.GetReturnValue().Set(isolate->ThrowException(V8JS_SYM("Module normaliser callback exception"))); + return; + } + + if (Z_TYPE_P(normaliser_result) != IS_ARRAY) { + zval_ptr_dtor(&normaliser_result); + info.GetReturnValue().Set(isolate->ThrowException(V8JS_SYM("Module normaliser didn't return an array"))); + return; + } + + HashTable *ht = HASH_OF(normaliser_result); + int num_elements = zend_hash_num_elements(ht); + + if(num_elements != 2) { + zval_ptr_dtor(&normaliser_result); + info.GetReturnValue().Set(isolate->ThrowException(V8JS_SYM("Module normaliser expected to return array of 2 strings"))); + return; + } + + zval **data; + ulong index = 0; + HashPosition pos; + + for (zend_hash_internal_pointer_reset_ex(ht, &pos); + SUCCESS == zend_hash_get_current_data_ex(ht, (void **) &data, &pos); + zend_hash_move_forward_ex(ht, &pos) + ) { + + if (Z_TYPE_P(*data) != IS_STRING) { + convert_to_string(*data); + } + + switch(index++) { + case 0: // normalised path + normalised_path = estrndup(Z_STRVAL_PP(data), Z_STRLEN_PP(data)); + break; + + case 1: // normalised module id + module_name = estrndup(Z_STRVAL_PP(data), Z_STRLEN_PP(data)); + break; + } + } + + zval_ptr_dtor(&normaliser_result); + } char *normalised_module_id = (char *)emalloc(strlen(normalised_path)+1+strlen(module_name)+1); *normalised_module_id = 0; From d9e4ae5abe1f50a80c9b425ec909e971519fc8d7 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Mon, 7 Dec 2015 13:26:44 +0100 Subject: [PATCH 7/9] Bump version to 0.4.0 --- package.xml | 63 ++++++++++++++++++++++++++++++++++++----------- php_v8js_macros.h | 2 +- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/package.xml b/package.xml index 13873f9..4f2fde6 100644 --- a/package.xml +++ b/package.xml @@ -16,11 +16,11 @@ stesie@php.net yes - 2015-10-11 - + 2015-12-07 + - 0.3.0 - 0.3.0 + 0.4.0 + 0.4.0 stable @@ -28,8 +28,13 @@ The MIT License (MIT) -- Fix multi-threading with pthreads extension -- Remove v8 debug agent support (which is unsupported by V8 since 3.28.4) +- Improve -Wno-c++11-narrowing/-Wno-narrowing flag detection (clang/gcc5 support) +- Added ability to set properties on V8Function +- CommonJS modules now have access to V8Js' var_dump function +- V8JsScriptExtensions now reference the normalised module id (instead of just "require") +- fatal PHP errors triggered in setModuleLoader callback are now handled gracefully +- setModuleLoader callback is now allowed to return an empty string as source of module +- V8Js' internal module path normalisation may now be overrode using setModuleNormaliser @@ -67,6 +72,11 @@ + + + + + @@ -74,6 +84,7 @@ + @@ -109,6 +120,7 @@ + @@ -171,38 +183,38 @@ - + - - - - + + + + - - + + - + - + @@ -441,5 +453,26 @@ - Remove v8 debug agent support (which is unsupported by V8 since 3.28.4) + + + 0.4.0 + 0.4.0 + + + stable + stable + + 2015-12-07 + The MIT License (MIT) + +- Improve -Wno-c++11-narrowing/-Wno-narrowing flag detection (clang/gcc5 support) +- Added ability to set properties on V8Function +- CommonJS modules now have access to V8Js' var_dump function +- V8JsScriptExtensions now reference the normalised module id (instead of just "require") +- fatal PHP errors triggered in setModuleLoader callback are now handled gracefully +- setModuleLoader callback is now allowed to return an empty string as source of module +- V8Js' internal module path normalisation may now be overrode using setModuleNormaliser + + diff --git a/php_v8js_macros.h b/php_v8js_macros.h index cf8e465..9a2b9e8 100644 --- a/php_v8js_macros.h +++ b/php_v8js_macros.h @@ -53,7 +53,7 @@ extern "C" { #endif /* V8Js Version */ -#define PHP_V8JS_VERSION "0.3.0" +#define PHP_V8JS_VERSION "0.4.0" /* Hidden field name used to link JS wrappers with underlying PHP object */ #define PHPJS_OBJECT_KEY "phpjs::object" From 3c5508b956e0e859f6f6bd933ad716abbe676093 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sat, 26 Dec 2015 12:16:17 +0100 Subject: [PATCH 8/9] Provide correct "this" on V8Object method invocation, closes #185 --- tests/issue_185_001.phpt | 36 ++++++++++++++++++++++++++++++++++++ tests/issue_185_002.phpt | 28 ++++++++++++++++++++++++++++ tests/issue_185_basic.phpt | 32 ++++++++++++++++++++++++++++++++ v8js_v8object_class.cc | 12 +++++++++++- 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 tests/issue_185_001.phpt create mode 100644 tests/issue_185_002.phpt create mode 100644 tests/issue_185_basic.phpt diff --git a/tests/issue_185_001.phpt b/tests/issue_185_001.phpt new file mode 100644 index 0000000..166e448 --- /dev/null +++ b/tests/issue_185_001.phpt @@ -0,0 +1,36 @@ +--TEST-- +Test V8::executeString() : Issue #185 this on direct invocation of method +--SKIPIF-- + +--FILE-- +executeString($JS); + +// now fetch `inst` from V8 and call method from PHP +$fn = $v8->executeString('(inst.tell)'); +$fn(); +?> +===EOF=== +--EXPECT-- +NULL +string(8) "function" +NULL +string(8) "function" +===EOF=== diff --git a/tests/issue_185_002.phpt b/tests/issue_185_002.phpt new file mode 100644 index 0000000..f2aa723 --- /dev/null +++ b/tests/issue_185_002.phpt @@ -0,0 +1,28 @@ +--TEST-- +Test V8::executeString() : Issue #185 this on function invocation +--SKIPIF-- + +--FILE-- +executeString($JS); + +// now fetch `inst` from V8 and call method from PHP +$fn = $v8->executeString('(fn)'); +$fn(); +?> +===EOF=== +--EXPECT-- +string(8) "function" +string(8) "function" +===EOF=== diff --git a/tests/issue_185_basic.phpt b/tests/issue_185_basic.phpt new file mode 100644 index 0000000..9bb562a --- /dev/null +++ b/tests/issue_185_basic.phpt @@ -0,0 +1,32 @@ +--TEST-- +Test V8::executeString() : Issue #185 Wrong this on V8Object method invocation +--SKIPIF-- + +--FILE-- +executeString($JS); + +// now fetch `inst` from V8 and call method from PHP +$inst = $v8->executeString('(inst)'); +$inst->tell(); +?> +===EOF=== +--EXPECT-- +int(23) +int(23) +===EOF=== diff --git a/v8js_v8object_class.cc b/v8js_v8object_class.cc index c51929d..20f01dc 100644 --- a/v8js_v8object_class.cc +++ b/v8js_v8object_class.cc @@ -302,6 +302,7 @@ static int v8js_v8object_call_method(char *method, INTERNAL_FUNCTION_PARAMETERS) v8::Local method_name = V8JS_SYML(method, strlen(method)); v8::Local v8obj = v8::Local::New(isolate, obj->v8obj)->ToObject(); + v8::Local thisObj; v8::Local cb; if (method_name->Equals(V8JS_SYM(V8JS_V8_INVOKE_FUNC_NAME))) { @@ -310,6 +311,15 @@ static int v8js_v8object_call_method(char *method, INTERNAL_FUNCTION_PARAMETERS) cb = v8::Local::Cast(v8obj->Get(method_name)); } + // If a method is invoked on V8Object, then set the object itself as + // "this" on JS side. Otherwise fall back to global object. + if (obj->std.ce == php_ce_v8object) { + thisObj = v8obj; + } + else { + thisObj = V8JS_GLOBAL(isolate); + } + v8::Local *jsArgv = static_cast *>(alloca(sizeof(v8::Local) * argc)); v8::Local js_retval; @@ -318,7 +328,7 @@ static int v8js_v8object_call_method(char *method, INTERNAL_FUNCTION_PARAMETERS) jsArgv[i] = v8::Local::New(isolate, zval_to_v8js(*argv[i], isolate TSRMLS_CC)); } - return cb->Call(V8JS_GLOBAL(isolate), argc, jsArgv); + return cb->Call(thisObj, argc, jsArgv); }; v8js_v8_call(obj->ctx, &return_value, obj->flags, obj->ctx->time_limit, obj->ctx->memory_limit, v8_call TSRMLS_CC); From 3108d3947d92df5bc3fdfe899b96bd193fcce73e Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Wed, 30 Dec 2015 14:59:37 +0100 Subject: [PATCH 9/9] Fix output of var_dump on regexp (V8 > 4.8) Newer V8 versions' toString() converts RegExp objects just to [object RegExp] (instead of the actual regexp as before). Work-around by calling GetSource() on the regexp and create former outhway that way. --- v8js_methods.cc | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/v8js_methods.cc b/v8js_methods.cc index 8832df2..9cde0cf 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -92,10 +92,20 @@ static void v8js_dumper(v8::Isolate *isolate, v8::Local var, int leve } v8::TryCatch try_catch; /* object.toString() can throw an exception */ - v8::Local details = var->ToDetailString(); - if (try_catch.HasCaught()) { - details = V8JS_SYM(""); + v8::Local details; + + if(var->IsRegExp()) { + v8::RegExp *re = v8::RegExp::Cast(*var); + details = re->GetSource(); } + else { + details = var->ToDetailString(); + + if (try_catch.HasCaught()) { + details = V8JS_SYM(""); + } + } + v8::String::Utf8Value str(details); const char *valstr = ToCString(str); size_t valstr_len = details->ToString()->Utf8Length(); @@ -113,7 +123,7 @@ static void v8js_dumper(v8::Isolate *isolate, v8::Local var, int leve } else if (var->IsRegExp()) { - php_printf("regexp(%s)\n", valstr); + php_printf("regexp(/%s/)\n", valstr); } else if (var->IsArray()) {