Introduce access control in propertyIsEnumerable.

Also, fix JSObject::getPropertyAttribute() so it deals correctly with access control modifiers.
Review URL: http://codereview.chromium.org/8834

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

src/objects.cc

@@ -266,6 +266,65 @@ Object* JSObject::GetPropertyWithFailedAccessCheck(Object* receiver,
 }
 
 
+PropertyAttributes JSObject::GetPropertyAttributeWithFailedAccessCheck(
+    Object* receiver,
+    LookupResult* result,
+    String* name,
+    bool continue_search) {
+  if (result->IsValid()) {
+    switch (result->type()) {
+      case CALLBACKS: {
+        // Only allow API accessors.
+        Object* obj = result->GetCallbackObject();
+        if (obj->IsAccessorInfo()) {
+          AccessorInfo* info = AccessorInfo::cast(obj);
+          if (info->all_can_read()) {
+            return result->GetAttributes();
+          }
+        }
+        break;
+      }
+
+      case NORMAL:
+      case FIELD:
+      case CONSTANT_FUNCTION: {
+        // Search ALL_CAN_READ accessors in prototype chain.
+        LookupResult r;
+        result->holder()->LookupRealNamedPropertyInPrototypes(name, &r);
+        if (r.IsValid() && continue_search) {
+          return GetPropertyAttributeWithFailedAccessCheck(receiver,
+                                                           &r,
+                                                           name,
+                                                           continue_search);
+        }
+        break;
+      }
+
+      case INTERCEPTOR: {
+        // If the object has an interceptor, try real named properties.
+        // No access check in GetPropertyAttributeWithInterceptor.
+        LookupResult r;
+        result->holder()->LookupRealNamedProperty(name, &r);
+        if (r.IsValid() && continue_search) {
+          return GetPropertyAttributeWithFailedAccessCheck(receiver,
+                                                           &r,
+                                                           name,
+                                                           continue_search);
+        }
+        break;
+      }
+
+      default: {
+        break;
+      }
+    }
+  }
+
+  Top::ReportFailedAccessCheck(this, v8::ACCESS_HAS);
+  return ABSENT;
+}
+
+
 Object* JSObject::GetLazyProperty(Object* receiver,
                                   LookupResult* result,
                                   String* name,
@@ -1720,8 +1779,10 @@ PropertyAttributes JSObject::GetPropertyAttribute(JSObject* receiver,
   // Check access rights if needed.
   if (IsAccessCheckNeeded() &&
       !Top::MayNamedAccess(this, name, v8::ACCESS_HAS)) {
-    Top::ReportFailedAccessCheck(this, v8::ACCESS_HAS);
-    return ABSENT;
+    return GetPropertyAttributeWithFailedAccessCheck(receiver,
+                                                     result,
+                                                     name,
+                                                     continue_search);
   }
   if (result->IsValid()) {
     switch (result->type()) {

src/objects.h

@@ -1453,6 +1453,11 @@ class JSObject: public HeapObject {
   PropertyAttributes GetPropertyAttributeWithInterceptor(JSObject* receiver,
                                                          String* name,
                                                          bool continue_search);
+  PropertyAttributes GetPropertyAttributeWithFailedAccessCheck(
+      Object* receiver,
+      LookupResult* result,
+      String* name,
+      bool continue_search);
   PropertyAttributes GetPropertyAttribute(JSObject* receiver,
                                           LookupResult* result,
                                           String* name,

src/runtime.cc

@@ -1933,10 +1933,8 @@ static Object* Runtime_IsPropertyEnumerable(Arguments args) {
     return Heap::ToBoolean(object->HasElement(index));
   }
 
-  LookupResult result;
-  object->LocalLookup(key, &result);
-  if (!result.IsProperty()) return Heap::false_value();
-  return Heap::ToBoolean(!result.IsDontEnum());
+  PropertyAttributes att = object->GetLocalPropertyAttribute(key);
+  return Heap::ToBoolean(att != ABSENT && att != DONT_ENUM);
 }
 
 

test/cctest/test-api.cc

@@ -3186,6 +3186,41 @@ THREADED_TEST(CrossDomainDelete) {
 }
 
 
+THREADED_TEST(CrossDomainIsPropertyEnumerable) {
+  v8::HandleScope handle_scope;
+  LocalContext env1;
+  v8::Persistent<Context> env2 = Context::New();
+
+  Local<Value> foo = v8_str("foo");
+  Local<Value> bar = v8_str("bar");
+
+  // Set to the same domain.
+  env1->SetSecurityToken(foo);
+  env2->SetSecurityToken(foo);
+
+  env1->Global()->Set(v8_str("prop"), v8_num(3));
+  env2->Global()->Set(v8_str("env1"), env1->Global());
+
+  // env1.prop is enumerable in env2.
+  Local<String> test = v8_str("propertyIsEnumerable.call(env1, 'prop')");
+  {
+    Context::Scope scope_env2(env2);
+    Local<Value> result = Script::Compile(test)->Run();
+    CHECK(result->IsTrue());
+  }
+
+  // Change env2 to a different domain and test again.
+  env2->SetSecurityToken(bar);
+  {
+    Context::Scope scope_env2(env2);
+    Local<Value> result = Script::Compile(test)->Run();
+    CHECK(result->IsFalse());
+  }
+
+  env2.Dispose();
+}
+
+
 THREADED_TEST(CrossDomainForIn) {
   v8::HandleScope handle_scope;
   LocalContext env1;
@@ -3342,7 +3377,7 @@ THREADED_TEST(AccessControl) {
       v8::AccessControl(v8::ALL_CAN_READ | v8::ALL_CAN_WRITE));
 
   // Add an accessor that is not accessible by cross-domain JS code.
-  global_template->SetAccessor(v8_str("blocked_access_prop"),
+  global_template->SetAccessor(v8_str("blocked_prop"),
                                UnreachableGetter, UnreachableSetter,
                                v8::Handle<Value>(),
                                v8::DEFAULT);
@@ -3368,6 +3403,9 @@ THREADED_TEST(AccessControl) {
   value = v8_compile("other.blocked_prop")->Run();
   CHECK(value->IsUndefined());
 
+  value = v8_compile("propertyIsEnumerable.call(other, 'blocked_prop')")->Run();
+  CHECK(value->IsFalse());
+
   // Access accessible property
   value = v8_compile("other.accessible_prop = 3")->Run();
   CHECK(value->IsNumber());
@@ -3377,6 +3415,21 @@ THREADED_TEST(AccessControl) {
   CHECK(value->IsNumber());
   CHECK_EQ(3, value->Int32Value());
 
+  value =
+    v8_compile("propertyIsEnumerable.call(other, 'accessible_prop')")->Run();
+  CHECK(value->IsTrue());
+
+  // Enumeration doesn't enumerate accessors from inaccessible objects in
+  // the prototype chain even if the accessors are in themselves accessible.
+  Local<Value> result =
+      CompileRun("(function(){var obj = {'__proto__':other};"
+                 "for (var p in obj)"
+                 "   if (p == 'accessible_prop' || p == 'blocked_prop') {"
+                 "     return false;"
+                 "   }"
+                 "return true;})()");
+  CHECK(result->IsTrue());
+
   context1->Exit();
   context0->Exit();
   context1.Dispose();