Merge pull request #90 from stesie/fix-property-visibility

Fix visibility of PHP properties in JS

tests/property_visibility-delete.phpt

@@ -0,0 +1,75 @@
+--TEST--
+Test V8::executeString() : Property visibility - delete
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class Foo {
+	private $privBar = "privBar";
+	protected $protBar = "protBar";
+	public $pubBar = "pubBar";
+
+	public function dump($a)
+	{
+		var_dump(@$this->$a);
+	}
+}
+
+$js = new V8Js();
+$js->foo = new Foo();
+
+$script = <<<END
+
+var_dump(PHP.foo.privBar);
+delete PHP.foo.privBar;
+var_dump(PHP.foo.privBar);
+
+PHP.foo.privBar = 42;
+
+var_dump(PHP.foo.privBar);
+delete PHP.foo.privBar;
+var_dump(PHP.foo.privBar);
+
+var_dump(PHP.foo.protBar);
+delete PHP.foo.protBar;
+var_dump(PHP.foo.protBar);
+
+var_dump(PHP.foo.pubBar);
+delete PHP.foo.pubBar;
+var_dump(PHP.foo.pubBar);
+
+END;
+
+$js->foo->dump('privBar');
+$js->foo->dump('protBar');
+$js->foo->dump('pubBar');
+
+echo "--- JS ---\n";
+$js->executeString($script);
+
+echo "--- PHP ---\n";
+$js->foo->dump('privBar');
+$js->foo->dump('protBar');
+$js->foo->dump('pubBar');
+
+?>
+===EOF===
+--EXPECT--
+string(7) "privBar"
+string(7) "protBar"
+string(6) "pubBar"
+--- JS ---
+NULL
+NULL
+int(42)
+NULL
+NULL
+NULL
+string(6) "pubBar"
+NULL
+--- PHP ---
+string(7) "privBar"
+string(7) "protBar"
+NULL
+===EOF===

tests/property_visibility-enumerate.phpt

@@ -0,0 +1,56 @@
+--TEST--
+Test V8::executeString() : Property visibility - enumerate
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class Foo {
+	private $privBar = "privBar";
+	protected $protBar = "protBar";
+	public $pubBar = "pubBar";
+
+	public function dump() {
+		foreach($this as $key => $value) {
+			echo("$key => ");
+			var_dump($value);
+		}
+	}
+}
+
+$js = new V8Js();
+$js->foo = new Foo();
+
+$script = <<<END
+
+for(var key in PHP.foo) {
+	if(PHP.foo.hasOwnProperty(key)) {
+		var_dump(key);
+
+		if(key[0] === '$') {
+			var_dump(PHP.foo[key]);
+		}
+		else {
+			var_dump("function");
+		}
+	}
+}
+
+END;
+
+$js->executeString($script);
+
+echo "--- PHP ---\n";
+$js->foo->dump();
+?>
+===EOF===
+--EXPECT--
+string(4) "dump"
+string(8) "function"
+string(7) "$pubBar"
+string(6) "pubBar"
+--- PHP ---
+privBar => string(7) "privBar"
+protBar => string(7) "protBar"
+pubBar => string(6) "pubBar"
+===EOF===

tests/property_visibility-has-property.phpt

@@ -0,0 +1,49 @@
+--TEST--
+Test V8::executeString() : Property visibility - has property
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class Foo {
+	private $privBar = "privBar";
+	protected $protBar = "protBar";
+	public $pubBar = "pubBar";
+}
+
+$js = new V8Js();
+$js->foo = new Foo();
+
+$script = <<<END
+
+var_dump(PHP.foo.hasOwnProperty("privBar"));
+var_dump(PHP.foo.hasOwnProperty("protBar"));
+var_dump(PHP.foo.hasOwnProperty("pubBar"));
+var_dump(PHP.foo.hasOwnProperty("unknownBar"));
+
+PHP.foo.privBar = 23;
+PHP.foo.protBar = 23;
+PHP.foo.pubBar = 23;
+PHP.foo.unknownBar = 23;
+
+var_dump(PHP.foo.hasOwnProperty("privBar"));
+var_dump(PHP.foo.hasOwnProperty("protBar"));
+var_dump(PHP.foo.hasOwnProperty("pubBar"));
+var_dump(PHP.foo.hasOwnProperty("unknownBar"));
+
+END;
+
+$js->executeString($script);
+
+?>
+===EOF===
+--EXPECT--
+bool(false)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(true)
+bool(true)
+bool(true)
+===EOF===

tests/property_visibility-set.phpt

@@ -0,0 +1,51 @@
+--TEST--
+Test V8::executeString() : Property visibility - set
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class Foo {
+	private $privBar = "privBar";
+	protected $protBar = "protBar";
+	public $pubBar = "pubBar";
+
+	public function dump() {
+		var_dump($this->privBar);
+		var_dump($this->protBar);
+		var_dump($this->pubBar);
+		var_dump($this->unknownBar);
+	}
+}
+
+$js = new V8Js();
+$js->foo = new Foo();
+
+$script = <<<END
+
+PHP.foo.privBar = 'jsPriv';
+PHP.foo.protBar = 'jsProt';
+PHP.foo.pubBar = 'jsPub';
+PHP.foo.unknownBar = 'jsUnknown';
+
+var_dump(PHP.foo.privBar);
+var_dump(PHP.foo.protBar);
+var_dump(PHP.foo.pubBar);
+var_dump(PHP.foo.unknownBar);
+
+END;
+
+$js->executeString($script);
+$js->foo->dump();
+?>
+===EOF===
+--EXPECT--
+string(6) "jsPriv"
+string(6) "jsProt"
+string(5) "jsPub"
+string(9) "jsUnknown"
+string(7) "privBar"
+string(7) "protBar"
+string(5) "jsPub"
+string(9) "jsUnknown"
+===EOF===

tests/property_visibility.phpt

@@ -0,0 +1,33 @@
+--TEST--
+Test V8::executeString() : Property visibility
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class Foo {
+	private $privBar = "privBar";
+	protected $protBar = "protBar";
+	public $pubBar = "pubBar";
+}
+
+$js = new V8Js();
+
+$js->foo = new Foo();
+
+$script = <<<END
+
+var_dump(PHP.foo.privBr);
+var_dump(PHP.foo.protBar);
+var_dump(PHP.foo.pubBar);
+
+END;
+
+$js->executeString($script);
+?>
+===EOF===
+--EXPECT--
+NULL
+NULL
+string(6) "pubBar"
+===EOF===

tests/property_visibility__get.phpt

@@ -0,0 +1,44 @@
+--TEST--
+Test V8::executeString() : Property visibility __get
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class Foo {
+	private $privBar = "privBar";
+	protected $protBar = "protBar";
+	public $pubBar = "pubBar";
+
+	public function __get($key)
+	{
+		var_dump($key);
+		return 42;
+	}
+}
+
+$js = new V8Js();
+
+$js->foo = new Foo();
+
+$script = <<<END
+
+var_dump(PHP.foo.unknownBar);
+var_dump(PHP.foo.privBar);
+var_dump(PHP.foo.protBar);
+var_dump(PHP.foo.pubBar);
+
+END;
+
+$js->executeString($script);
+?>
+===EOF===
+--EXPECT--
+string(10) "unknownBar"
+int(42)
+string(7) "privBar"
+int(42)
+string(7) "protBar"
+int(42)
+string(6) "pubBar"
+===EOF===

tests/property_visibility__set.phpt

@@ -0,0 +1,67 @@
+--TEST--
+Test V8::executeString() : Property visibility __set
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class Foo {
+	private $privBar = "privBar";
+	protected $protBar = "protBar";
+	public $pubBar = "pubBar";
+
+	public function __set($name, $value) {
+		echo "$name <- $value\n";
+	}
+
+	public function dump() {
+		var_dump($this->privBar);
+		var_dump($this->protBar);
+		var_dump($this->pubBar);
+		var_dump(isset($this->unknownBar));
+		var_dump(isset($this->phpBar));
+	}
+}
+
+$js = new V8Js();
+
+$js->foo = new Foo();
+$js->foo->protBar = 'piet';
+$js->foo->phpBar = 'phpValue';
+
+$script = <<<END
+
+PHP.foo.privBar = 'jsPriv';
+PHP.foo.protBar = 'jsProt';
+PHP.foo.pubBar = 'jsPub';
+PHP.foo.unknownBar = 'jsUnknown';
+
+var_dump(PHP.foo.privBar);
+var_dump(PHP.foo.protBar);
+var_dump(PHP.foo.pubBar);
+var_dump(PHP.foo.unknownBar);
+var_dump(PHP.foo.phpBar);
+
+END;
+
+$js->executeString($script);
+$js->foo->dump();
+?>
+===EOF===
+--EXPECT--
+protBar <- piet
+phpBar <- phpValue
+privBar <- jsPriv
+protBar <- jsProt
+unknownBar <- jsUnknown
+NULL
+NULL
+string(5) "jsPub"
+NULL
+NULL
+string(7) "privBar"
+string(7) "protBar"
+string(5) "jsPub"
+bool(false)
+bool(false)
+===EOF===

v8js.cc

@@ -932,6 +932,7 @@ static PHP_METHOD(V8Js, __construct)
 		}
 
 		zval zmember;
+		INIT_ZVAL(zmember);
 		ZVAL_STRING(&zmember, member, 0);
 
 		zend_property_info *property_info = zend_get_property_info(c->std.ce, &zmember, 1 TSRMLS_CC);

v8js_convert.cc

@@ -48,6 +48,7 @@ static void php_v8js_call_php_func(zval *value, zend_class_entry *ce, zend_funct
 	max_num_args = method_ptr->common.num_args;
 
 	/* Function name to call */
+	INIT_ZVAL(fname);
 	ZVAL_STRING(&fname, method_ptr->common.function_name, 0);
 
 	/* zend_fcall_info */
@@ -604,15 +605,56 @@ static inline v8::Local<v8::Value> php_v8js_named_property_callback(v8::Local<v8
 		}
 		if (callback_type == V8JS_PROP_GETTER) {
 			/* Nope, not a method -- must be a (case-sensitive) property */
-			php_value = zend_read_property(scope, object, V8JS_CONST name, name_len, true TSRMLS_CC);
-			// special case uninitialized_zval_ptr and return an empty value
-			// (indicating that we don't intercept this property) if the
-			// property doesn't exist.
-			if (php_value == EG(uninitialized_zval_ptr)) {
-				ret_value = v8::Handle<v8::Value>();
-			} else {
-				// wrap it
+			zval zname;
+			INIT_ZVAL(zname);
+			ZVAL_STRINGL(&zname, name, name_len, 0);
+			zend_property_info *property_info = zend_get_property_info(ce, &zname, 1 TSRMLS_CC);
+
+			if(property_info && property_info->flags & ZEND_ACC_PUBLIC) {
+				php_value = zend_read_property(NULL, object, V8JS_CONST name, name_len, true TSRMLS_CC);
+				// special case uninitialized_zval_ptr and return an empty value
+				// (indicating that we don't intercept this property) if the
+				// property doesn't exist.
+				if (php_value == EG(uninitialized_zval_ptr)) {
+					ret_value = v8::Handle<v8::Value>();
+				} else {
+					// wrap it
+					ret_value = zval_to_v8js(php_value, isolate TSRMLS_CC);
+					/* We don't own the reference to php_value... unless the
+					 * returned refcount was 0, in which case the below code
+					 * will free it. */
+					zval_add_ref(&php_value);
+					zval_ptr_dtor(&php_value);
+				}
+			}
+			else if (zend_hash_find(&ce->function_table, "__get", 6, (void**)&method_ptr) == SUCCESS
+					 /* Allow only public methods */
+					 && ((method_ptr->common.fn_flags & ZEND_ACC_PUBLIC) != 0)) {
+				/* Okay, let's call __get. */
+				zend_fcall_info fci;
+
+				zval fmember;
+				INIT_ZVAL(fmember);
+				ZVAL_STRING(&fmember, "__get", 0);
+
+				fci.size = sizeof(fci);
+				fci.function_table = &ce->function_table;
+				fci.function_name = &fmember;
+				fci.symbol_table = NULL;
+				fci.retval_ptr_ptr = &php_value;
+
+				zval *zname_ptr = &zname;
+				zval **zname_ptr_ptr = &zname_ptr;
+				fci.param_count = 1;
+				fci.params = &zname_ptr_ptr;
+
+				fci.object_ptr = object;
+				fci.no_separation = 0;
+
+				zend_call_function(&fci, NULL TSRMLS_CC);
+
 				ret_value = zval_to_v8js(php_value, isolate TSRMLS_CC);
+
 				/* We don't own the reference to php_value... unless the
 				 * returned refcount was 0, in which case the below code
 				 * will free it. */
@@ -621,12 +663,59 @@ static inline v8::Local<v8::Value> php_v8js_named_property_callback(v8::Local<v8
 			}
 		} else if (callback_type == V8JS_PROP_SETTER) {
 			MAKE_STD_ZVAL(php_value);
-			if (v8js_to_zval(set_value, php_value, 0, isolate TSRMLS_CC) == SUCCESS) {
-				zend_update_property(scope, object, V8JS_CONST name, name_len, php_value TSRMLS_CC);
-				ret_value = set_value;
-			} else {
+			if (v8js_to_zval(set_value, php_value, 0, isolate TSRMLS_CC) != SUCCESS) {
 				ret_value = v8::Handle<v8::Value>();
 			}
+			else {
+				zval zname;
+				INIT_ZVAL(zname);
+				ZVAL_STRINGL(&zname, name, name_len, 0);
+				zend_property_info *property_info = zend_get_property_info(ce, &zname, 1 TSRMLS_CC);
+
+				if(property_info && property_info->flags & ZEND_ACC_PUBLIC) {
+					zend_update_property(scope, object, V8JS_CONST name, name_len, php_value TSRMLS_CC);
+					ret_value = set_value;
+				}
+				else if (zend_hash_find(&ce->function_table, "__set", 6, (void**)&method_ptr) == SUCCESS
+						 /* Allow only public methods */
+						 && ((method_ptr->common.fn_flags & ZEND_ACC_PUBLIC) != 0)) {
+					/* Okay, let's call __set. */
+					zend_fcall_info fci;
+
+					zval fmember;
+					INIT_ZVAL(fmember);
+					ZVAL_STRING(&fmember, "__set", 0);
+
+					zval *php_ret_value;
+
+					fci.size = sizeof(fci);
+					fci.function_table = &ce->function_table;
+					fci.function_name = &fmember;
+					fci.symbol_table = NULL;
+					fci.retval_ptr_ptr = &php_ret_value;
+
+					zval *zname_ptr = &zname;
+					zval **params[2];
+					fci.param_count = 2;
+					fci.params = params;
+					fci.params[0] = &zname_ptr;
+					fci.params[1] = &php_value;
+
+					fci.object_ptr = object;
+					fci.no_separation = 1;
+
+					zend_call_function(&fci, NULL TSRMLS_CC);
+
+					ret_value = zval_to_v8js(php_ret_value, isolate TSRMLS_CC);
+
+					/* We don't own the reference to php_ret_value... unless the
+					 * returned refcount was 0, in which case the below code
+					 * will free it. */
+					zval_add_ref(&php_ret_value);
+					zval_ptr_dtor(&php_ret_value);
+				}
+			}
+
 			// if PHP wanted to hold on to this value, update_property would
 			// have bumped the refcount
 			zval_ptr_dtor(&php_value);
@@ -636,6 +725,7 @@ static inline v8::Local<v8::Value> php_v8js_named_property_callback(v8::Local<v8
 			zval *prop;
 			MAKE_STD_ZVAL(prop);
 			ZVAL_STRINGL(prop, name, name_len, 1);
+
 			if (callback_type == V8JS_PROP_QUERY) {
 				if (h->has_property(object, prop, 0 ZEND_HASH_KEY_NULL TSRMLS_CC)) {
 					ret_value = V8JS_UINT(v8::None);
@@ -643,8 +733,15 @@ static inline v8::Local<v8::Value> php_v8js_named_property_callback(v8::Local<v8
 					ret_value = v8::Handle<v8::Value>(); // empty handle
 				}
 			} else {
-				h->unset_property(object, prop ZEND_HASH_KEY_NULL TSRMLS_CC);
-				ret_value = V8JS_BOOL(true);
+				zend_property_info *property_info = zend_get_property_info(ce, prop, 1 TSRMLS_CC);
+
+				if(property_info && property_info->flags & ZEND_ACC_PUBLIC) {
+					h->unset_property(object, prop ZEND_HASH_KEY_NULL TSRMLS_CC);
+					ret_value = V8JS_BOOL(true);
+				}
+				else {
+					ret_value = v8::Handle<v8::Value>(); // empty handle
+				}
 			}
 			zval_ptr_dtor(&prop);
 		} else {