Re-enable Object.observe and add enforcement for security invariants.

This patch reverts r21062 which disabled Object.observe and the relevant tests.

It also adds enforcement for the following three invariants:

1) No observer may receive a change record describing changes to an object which is in different security origin (context have differing security tokens)

2) No observer may receive a change record whose context's security token is different from that of the object described by the change.

3) Object.getNotifier will return null if the caller and the provided object are in differing security origins

Further, it ensures that the global object can never be observed nor a notifier retrieved for it.

Tests are included.
R=verwaest@chromium.org, rossberg
LOG=Y

Review URL: https://codereview.chromium.org/265503002

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@21122 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/api.cc

@@ -3567,28 +3567,13 @@ Local<v8::Object> v8::Object::Clone() {
 }
 
 
-static i::Context* GetCreationContext(i::JSObject* object) {
-  i::Object* constructor = object->map()->constructor();
-  i::JSFunction* function;
-  if (!constructor->IsJSFunction()) {
-    // Functions have null as a constructor,
-    // but any JSFunction knows its context immediately.
-    ASSERT(object->IsJSFunction());
-    function = i::JSFunction::cast(object);
-  } else {
-    function = i::JSFunction::cast(constructor);
-  }
-  return function->context()->native_context();
-}
-
-
 Local<v8::Context> v8::Object::CreationContext() {
   i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
   ON_BAILOUT(isolate,
              "v8::Object::CreationContext()", return Local<v8::Context>());
   ENTER_V8(isolate);
   i::Handle<i::JSObject> self = Utils::OpenHandle(this);
-  i::Context* context = GetCreationContext(*self);
+  i::Context* context = self->GetCreationContext();
   return Utils::ToLocal(i::Handle<i::Context>(context));
 }
 

src/messages.js

@@ -77,6 +77,7 @@ var kMessages = {
   observe_perform_non_string:    ["Invalid non-string changeType"],
   observe_perform_non_function:  ["Cannot perform non-function"],
   observe_notify_non_notifier:   ["notify called on non-notifier object"],
+  observe_global_proxy:          ["%0", " cannot be called on the global proxy object"],
   not_typed_array:               ["this is not a typed array."],
   invalid_argument:              ["invalid_argument"],
   data_view_not_array_buffer:    ["First argument to DataView constructor must be an ArrayBuffer"],

src/object-observe.js

@@ -355,6 +355,8 @@ function CallbackInfoNormalize(callback) {
 function ObjectObserve(object, callback, acceptList) {
   if (!IS_SPEC_OBJECT(object))
     throw MakeTypeError("observe_non_object", ["observe"]);
+  if (%IsJSGlobalProxy(object))
+    throw MakeTypeError("observe_global_proxy", ["observe"]);
   if (!IS_SPEC_FUNCTION(callback))
     throw MakeTypeError("observe_non_function", ["observe"]);
   if (ObjectIsFrozen(callback))
@@ -370,6 +372,8 @@ function ObjectObserve(object, callback, acceptList) {
 function ObjectUnobserve(object, callback) {
   if (!IS_SPEC_OBJECT(object))
     throw MakeTypeError("observe_non_object", ["unobserve"]);
+  if (%IsJSGlobalProxy(object))
+    throw MakeTypeError("observe_global_proxy", ["unobserve"]);
   if (!IS_SPEC_FUNCTION(callback))
     throw MakeTypeError("observe_non_function", ["unobserve"]);
 
@@ -392,19 +396,15 @@ function ArrayUnobserve(object, callback) {
   return ObjectUnobserve(object, callback);
 }
 
-function ObserverEnqueueIfActive(observer, objectInfo, changeRecord,
-                                 needsAccessCheck) {
+function ObserverEnqueueIfActive(observer, objectInfo, changeRecord) {
   if (!ObserverIsActive(observer, objectInfo) ||
       !TypeMapHasType(ObserverGetAcceptTypes(observer), changeRecord.type)) {
     return;
   }
 
   var callback = ObserverGetCallback(observer);
-  if (needsAccessCheck &&
-      // Drop all splice records on the floor for access-checked objects
-      (changeRecord.type == 'splice' ||
-       !%IsAccessAllowedForObserver(
-           callback, changeRecord.object, changeRecord.name))) {
+  if (!%ObserverObjectAndRecordHaveSameOrigin(callback, changeRecord.object,
+                                              changeRecord)) {
     return;
   }
 
@@ -433,22 +433,16 @@ function ObjectInfoEnqueueExternalChangeRecord(objectInfo, changeRecord, type) {
   }
   ObjectFreeze(newRecord);
 
-  ObjectInfoEnqueueInternalChangeRecord(objectInfo, newRecord,
-                                        true /* skip access check */);
+  ObjectInfoEnqueueInternalChangeRecord(objectInfo, newRecord);
 }
 
-function ObjectInfoEnqueueInternalChangeRecord(objectInfo, changeRecord,
-                                               skipAccessCheck) {
+function ObjectInfoEnqueueInternalChangeRecord(objectInfo, changeRecord) {
   // TODO(rossberg): adjust once there is a story for symbols vs proxies.
   if (IS_SYMBOL(changeRecord.name)) return;
 
-  var needsAccessCheck = !skipAccessCheck &&
-      %IsAccessCheckNeeded(changeRecord.object);
-
   if (ChangeObserversIsOptimized(objectInfo.changeObservers)) {
     var observer = objectInfo.changeObservers;
-    ObserverEnqueueIfActive(observer, objectInfo, changeRecord,
-                            needsAccessCheck);
+    ObserverEnqueueIfActive(observer, objectInfo, changeRecord);
     return;
   }
 
@@ -456,8 +450,7 @@ function ObjectInfoEnqueueInternalChangeRecord(objectInfo, changeRecord,
     var observer = objectInfo.changeObservers[priority];
     if (IS_NULL(observer))
       continue;
-    ObserverEnqueueIfActive(observer, objectInfo, changeRecord,
-                            needsAccessCheck);
+    ObserverEnqueueIfActive(observer, objectInfo, changeRecord);
   }
 }
 
@@ -558,9 +551,13 @@ function ObjectNotifierPerformChange(changeType, changeFn) {
 function ObjectGetNotifier(object) {
   if (!IS_SPEC_OBJECT(object))
     throw MakeTypeError("observe_non_object", ["getNotifier"]);
+  if (%IsJSGlobalProxy(object))
+    throw MakeTypeError("observe_global_proxy", ["getNotifier"]);
 
   if (ObjectIsFrozen(object)) return null;
 
+  if (!%ObjectWasCreatedInCurrentOrigin(object)) return null;
+
   var objectInfo = ObjectInfoGetOrCreate(object);
   return ObjectInfoGetNotifier(objectInfo);
 }
@@ -622,5 +619,4 @@ function SetupObjectObserve() {
   ));
 }
 
-// Disable Object.observe API for M35.
-// SetupObjectObserve();
+SetupObjectObserve();

src/objects.cc

@@ -1957,6 +1957,21 @@ MaybeHandle<Object> JSObject::AddProperty(
 }
 
 
+Context* JSObject::GetCreationContext() {
+  Object* constructor = this->map()->constructor();
+  JSFunction* function;
+  if (!constructor->IsJSFunction()) {
+    // Functions have null as a constructor,
+    // but any JSFunction knows its context immediately.
+    function = JSFunction::cast(this);
+  } else {
+    function = JSFunction::cast(constructor);
+  }
+
+  return function->context()->native_context();
+}
+
+
 void JSObject::EnqueueChangeRecord(Handle<JSObject> object,
                                    const char* type_str,
                                    Handle<Name> name,

src/objects.h

@@ -2654,6 +2654,8 @@ class JSObject: public JSReceiver {
     static inline int SizeOf(Map* map, HeapObject* object);
   };
 
+  Context* GetCreationContext();
+
   // Enqueue change record for Object.observe. May cause GC.
   static void EnqueueChangeRecord(Handle<JSObject> object,
                                   const char* type,

src/runtime.cc

@@ -14872,11 +14872,11 @@ RUNTIME_FUNCTION(Runtime_HaveSameMap) {
 }
 
 
-RUNTIME_FUNCTION(Runtime_IsAccessCheckNeeded) {
+RUNTIME_FUNCTION(Runtime_IsJSGlobalProxy) {
   SealHandleScope shs(isolate);
   ASSERT(args.length() == 1);
-  CONVERT_ARG_CHECKED(HeapObject, obj, 0);
-  return isolate->heap()->ToBoolean(obj->IsAccessCheckNeeded());
+  CONVERT_ARG_CHECKED(Object, obj, 0);
+  return isolate->heap()->ToBoolean(obj->IsJSGlobalProxy());
 }
 
 
@@ -14961,32 +14961,38 @@ RUNTIME_FUNCTION(Runtime_ObservationWeakMapCreate) {
 }
 
 
-RUNTIME_FUNCTION(Runtime_IsAccessAllowedForObserver) {
+static bool ContextsHaveSameOrigin(Handle<Context> context1,
+                                   Handle<Context> context2) {
+  return context1->security_token() == context2->security_token();
+}
+
+
+RUNTIME_FUNCTION(Runtime_ObserverObjectAndRecordHaveSameOrigin) {
   HandleScope scope(isolate);
   ASSERT(args.length() == 3);
   CONVERT_ARG_HANDLE_CHECKED(JSFunction, observer, 0);
   CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 1);
-  RUNTIME_ASSERT(object->map()->is_access_check_needed());
-  CONVERT_ARG_HANDLE_CHECKED(Object, key, 2);
-  SaveContext save(isolate);
-  isolate->set_context(observer->context());
-  if (!isolate->MayNamedAccess(
-          object, isolate->factory()->undefined_value(), v8::ACCESS_KEYS)) {
-    return isolate->heap()->false_value();
-  }
-  bool access_allowed = false;
-  uint32_t index = 0;
-  if (key->ToArrayIndex(&index) ||
-      (key->IsString() && String::cast(*key)->AsArrayIndex(&index))) {
-    access_allowed =
-        isolate->MayIndexedAccess(object, index, v8::ACCESS_GET) &&
-        isolate->MayIndexedAccess(object, index, v8::ACCESS_HAS);
-  } else {
-    access_allowed =
-        isolate->MayNamedAccess(object, key, v8::ACCESS_GET) &&
-        isolate->MayNamedAccess(object, key, v8::ACCESS_HAS);
-  }
-  return isolate->heap()->ToBoolean(access_allowed);
+  CONVERT_ARG_HANDLE_CHECKED(JSObject, record, 2);
+
+  Handle<Context> observer_context(observer->context()->native_context(),
+      isolate);
+  Handle<Context> object_context(object->GetCreationContext());
+  Handle<Context> record_context(record->GetCreationContext());
+
+  return isolate->heap()->ToBoolean(
+      ContextsHaveSameOrigin(object_context, observer_context) &&
+      ContextsHaveSameOrigin(object_context, record_context));
+}
+
+
+RUNTIME_FUNCTION(Runtime_ObjectWasCreatedInCurrentOrigin) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 1);
+  CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0);
+
+  Handle<Context> creation_context(object->GetCreationContext(), isolate);
+  return isolate->heap()->ToBoolean(
+      ContextsHaveSameOrigin(creation_context, isolate->native_context()));
 }
 
 

src/runtime.h

@@ -311,7 +311,8 @@ namespace internal {
   F(SetIsObserved, 1, 1) \
   F(GetObservationState, 0, 1) \
   F(ObservationWeakMapCreate, 0, 1) \
-  F(IsAccessAllowedForObserver, 3, 1) \
+  F(ObserverObjectAndRecordHaveSameOrigin, 3, 1) \
+  F(ObjectWasCreatedInCurrentOrigin, 1, 1) \
   \
   /* Harmony typed arrays */ \
   F(ArrayBufferInitialize, 2, 1)\
@@ -397,7 +398,7 @@ namespace internal {
   F(HasFastProperties, 1, 1) \
   F(TransitionElementsKind, 2, 1) \
   F(HaveSameMap, 2, 1) \
-  F(IsAccessCheckNeeded, 1, 1)
+  F(IsJSGlobalProxy, 1, 1)
 
 
 #define RUNTIME_FUNCTION_LIST_DEBUGGER(F) \

test/cctest/cctest.status

@@ -84,10 +84,6 @@
   'test-api/Threading3': [PASS, ['mode == debug', SLOW]],
   'test-api/Threading4': [PASS, ['mode == debug', SLOW]],
   'test-strings/StringOOM*': [PASS, ['mode == debug', SKIP]],
-
-  # Object.observe is disabled.
-  'test-object-observe/*': [SKIP],
-  'test-microtask-delivery/*': [SKIP],
 }],  # ALWAYS
 
 ##############################################################################

test/mjsunit/es7/object-observe.js

@@ -113,6 +113,8 @@ Object.defineProperty(changeRecordWithAccessor, 'name', {
 // Object.observe
 assertThrows(function() { Object.observe("non-object", observer.callback); },
              TypeError);
+assertThrows(function() { Object.observe(this, observer.callback); },
+             TypeError);
 assertThrows(function() { Object.observe(obj, nonFunction); }, TypeError);
 assertThrows(function() { Object.observe(obj, frozenFunction); }, TypeError);
 assertEquals(obj, Object.observe(obj, observer.callback, [1]));
@@ -127,6 +129,8 @@ assertEquals(obj, Object.observe(obj, observer.callback));
 
 // Object.unobserve
 assertThrows(function() { Object.unobserve(4, observer.callback); }, TypeError);
+assertThrows(function() { Object.unobserve(this, observer.callback); },
+             TypeError);
 assertThrows(function() { Object.unobserve(obj, nonFunction); }, TypeError);
 assertEquals(obj, Object.unobserve(obj, observer.callback));
 
@@ -135,6 +139,7 @@ assertEquals(obj, Object.unobserve(obj, observer.callback));
 var notifier = Object.getNotifier(obj);
 assertSame(notifier, Object.getNotifier(obj));
 assertEquals(null, Object.getNotifier(Object.freeze({})));
+assertThrows(function() { Object.getNotifier(this) }, TypeError);
 assertFalse(notifier.hasOwnProperty('notify'));
 assertEquals([], Object.keys(notifier));
 var notifyDesc = Object.getOwnPropertyDescriptor(notifier.__proto__, 'notify');
@@ -1074,6 +1079,8 @@ function TestObserveNonConfigurable(obj, prop, desc) {
   Object.unobserve(obj, observer.callback);
 }
 
+// TODO(rafaelw) Enable when ES6 Proxies are implemented
+/*
 function createProxy(create, x) {
   var handler = {
     getPropertyDescriptor: function(k) {
@@ -1113,11 +1120,11 @@ function createProxy(create, x) {
   Object.observe(handler.target, handler.callback);
   return handler.proxy = create(handler, x);
 }
+*/
 
 var objects = [
   {},
   [],
-  this,  // global object
   function(){},
   (function(){ return arguments })(),
   (function(){ "use strict"; return arguments })(),
@@ -1125,9 +1132,10 @@ var objects = [
   new Date(),
   Object, Function, Date, RegExp,
   new Set, new Map, new WeakMap,
-  new ArrayBuffer(10), new Int32Array(5),
-  createProxy(Proxy.create, null),
-  createProxy(Proxy.createFunction, function(){}),
+  new ArrayBuffer(10), new Int32Array(5)
+// TODO(rafaelw) Enable when ES6 Proxies are implemented.
+//  createProxy(Proxy.create, null),
+//  createProxy(Proxy.createFunction, function(){}),
 ];
 var properties = ["a", "1", 1, "length", "setPrototype", "name", "caller"];
 

test/mjsunit/mjsunit.status

@@ -127,14 +127,6 @@
   # array buffer.
   'nans': [PASS, ['arch == mips', SKIP]],
 
-  # Object.observe is disabled.
-  'es6/promises': [SKIP],
-  'array-push7': [SKIP],
-  'harmony/microtask-delivery': [SKIP],
-  'es7/object-observe': [SKIP],
-  'harmony/regress/regress-observe-empty-double-array': [SKIP],
-  'regress/regress-356589': [SKIP],
-  'regress/regress-observe-map-cache': [SKIP],
 }],  # ALWAYS
 
 ##############################################################################