Revision: 17257
Author: [email protected]
Date: Thu Oct 17 20:49:45 2013 UTC
Log: Prevent changes to hidden properties from being observable via
Object.observe
This addresses the leak that mstarzinger points out
(https://codereview.chromium.org/26390003/) and includes the test.
Note that this adds a test that observing changes to the empty-string
property remains possible.
BUG=
[email protected], [email protected]
Review URL: https://codereview.chromium.org/26592012
http://code.google.com/p/v8/source/detail?r=17257
Modified:
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/test/cctest/test-object-observe.cc
/branches/bleeding_edge/test/mjsunit/harmony/object-observe.js
=======================================
--- /branches/bleeding_edge/src/objects.cc Wed Oct 16 11:16:49 2013 UTC
+++ /branches/bleeding_edge/src/objects.cc Thu Oct 17 20:49:45 2013 UTC
@@ -2137,7 +2137,9 @@
AddSlowProperty(object, name, value, attributes);
}
- if (FLAG_harmony_observation && object->map()->is_observed()) {
+ if (FLAG_harmony_observation &&
+ object->map()->is_observed() &&
+ *name != isolate->heap()->hidden_string()) {
Handle<Object> old_value = isolate->factory()->the_hole_value();
EnqueueChangeRecord(object, "new", name, old_value);
}
@@ -4015,8 +4017,10 @@
}
Handle<Object> old_value = isolate->factory()->the_hole_value();
- if (FLAG_harmony_observation &&
- object->map()->is_observed() && lookup->IsDataProperty()) {
+ bool is_observed = FLAG_harmony_observation &&
+ object->map()->is_observed() &&
+ *name != isolate->heap()->hidden_string();
+ if (is_observed && lookup->IsDataProperty()) {
old_value = Object::GetProperty(object, name);
}
@@ -4055,7 +4059,7 @@
RETURN_IF_EMPTY_HANDLE_VALUE(isolate, result, Handle<Object>());
- if (FLAG_harmony_observation && object->map()->is_observed()) {
+ if (is_observed) {
if (lookup->IsTransition()) {
EnqueueChangeRecord(object, "new", name, old_value);
} else {
@@ -4156,7 +4160,9 @@
Handle<Object> old_value = isolate->factory()->the_hole_value();
PropertyAttributes old_attributes = ABSENT;
- bool is_observed = FLAG_harmony_observation &&
object->map()->is_observed();
+ bool is_observed = FLAG_harmony_observation &&
+ object->map()->is_observed() &&
+ *name != isolate->heap()->hidden_string();
if (is_observed && lookup.IsProperty()) {
if (lookup.IsDataProperty()) old_value =
Object::GetProperty(object, name);
@@ -5220,7 +5226,9 @@
}
Handle<Object> old_value = isolate->factory()->the_hole_value();
- bool is_observed = FLAG_harmony_observation &&
object->map()->is_observed();
+ bool is_observed = FLAG_harmony_observation &&
+ object->map()->is_observed() &&
+ *name != isolate->heap()->hidden_string();
if (is_observed && lookup.IsDataProperty()) {
old_value = Object::GetProperty(object, name);
}
@@ -6292,7 +6300,9 @@
bool is_element = name->AsArrayIndex(&index);
Handle<Object> old_value = isolate->factory()->the_hole_value();
- bool is_observed = FLAG_harmony_observation &&
object->map()->is_observed();
+ bool is_observed = FLAG_harmony_observation &&
+ object->map()->is_observed() &&
+ *name != isolate->heap()->hidden_string();
bool preexists = false;
if (is_observed) {
if (is_element) {
=======================================
--- /branches/bleeding_edge/test/cctest/test-object-observe.cc Wed Oct 2
18:06:20 2013 UTC
+++ /branches/bleeding_edge/test/cctest/test-object-observe.cc Thu Oct 17
20:49:45 2013 UTC
@@ -720,3 +720,18 @@
}
CHECK(CompileRun("records")->IsNull());
}
+
+
+TEST(HiddenPropertiesLeakage) {
+ HarmonyIsolate isolate;
+ HandleScope scope(isolate.GetIsolate());
+ LocalContext context(isolate.GetIsolate());
+ CompileRun("var obj = {};"
+ "var records = null;"
+ "var observer = function(r) { records = r };"
+ "Object.observe(obj, observer);");
+ Handle<Value> obj = context->Global()->Get(String::New("obj"));
+ Handle<Object>::Cast(obj)->SetHiddenValue(String::New("foo"), Null());
+ CompileRun(""); // trigger delivery
+ CHECK(CompileRun("records")->IsNull());
+}
=======================================
--- /branches/bleeding_edge/test/mjsunit/harmony/object-observe.js Fri Sep
13 08:13:02 2013 UTC
+++ /branches/bleeding_edge/test/mjsunit/harmony/object-observe.js Thu Oct
17 20:49:45 2013 UTC
@@ -286,6 +286,20 @@
{ object: obj, type: 'new', name: 'id' },
]);
+// The empty-string property is observable
+reset();
+var obj = {};
+Object.observe(obj, observer.callback);
+obj[''] = '';
+obj[''] = ' ';
+delete obj[''];
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+ { object: obj, type: 'new', name: '' },
+ { object: obj, type: 'updated', name: '', oldValue: '' },
+ { object: obj, type: 'deleted', name: '', oldValue: ' ' },
+]);
+
// Observing a continuous stream of changes, while itermittantly
unobserving.
reset();
Object.observe(obj, observer.callback);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
