Bug 905493. Fix setting document.location via an Xray to not enter the content compartment. r=peterv

dom/bindings/BindingUtils.cpp

@@ -872,6 +872,15 @@ GetNativePropertyHooks(JSContext *cx, JS::Handle<JSObject*> obj,
   return ifaceAndProtoJSClass->mNativeHooks;
 }
 
+// Try to resolve a property as an unforgeable property from the given
+// NativeProperties, if it's there.  nativeProperties is allowed to be null (in
+// which case we of course won't resolve anything).
+static bool
+XrayResolveUnforgeableProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
+                               JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
+                               JS::MutableHandle<JSPropertyDescriptor> desc,
+                               const NativeProperties* nativeProperties);
+
 bool
 XrayResolveOwnProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                        JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
@@ -881,7 +890,29 @@ XrayResolveOwnProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
   const NativePropertyHooks *nativePropertyHooks =
     GetNativePropertyHooks(cx, obj, type);
 
-  return type != eInstance || !nativePropertyHooks->mResolveOwnProperty ||
+  if (type != eInstance) {
+    return true;
+  }
+
+  // Check for unforgeable properties before doing mResolveOwnProperty weirdness
+  const NativePropertiesHolder& nativeProperties =
+    nativePropertyHooks->mNativeProperties;
+  if (!XrayResolveUnforgeableProperty(cx, wrapper, obj, id, desc,
+                                      nativeProperties.regular)) {
+    return false;
+  }
+  if (desc.object()) {
+    return true;
+  }
+  if (!XrayResolveUnforgeableProperty(cx, wrapper, obj, id, desc,
+                                      nativeProperties.chromeOnly)) {
+    return false;
+  }
+  if (desc.object()) {
+    return true;
+  }
+
+  return !nativePropertyHooks->mResolveOwnProperty ||
          nativePropertyHooks->mResolveOwnProperty(cx, wrapper, obj, id, desc, flags);
 }
 
@@ -936,6 +967,20 @@ XrayResolveAttribute(JSContext* cx, JS::Handle<JSObject*> wrapper,
   return true;
 }
 
+/* static */ bool
+XrayResolveUnforgeableProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
+                               JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
+                               JS::MutableHandle<JSPropertyDescriptor> desc,
+                               const NativeProperties* nativeProperties)
+{
+  return !nativeProperties || !nativeProperties->unforgeableAttributes ||
+         XrayResolveAttribute(cx, wrapper, obj, id,
+                              nativeProperties->unforgeableAttributes,
+                              nativeProperties->unforgeableAttributeIds,
+                              nativeProperties->unforgeableAttributeSpecs,
+                              desc);
+}
+
 static bool
 XrayResolveProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                     JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
@@ -1008,18 +1053,6 @@ XrayResolveProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
         return true;
       }
     }
-    if (nativeProperties->unforgeableAttributes) {
-      if (!XrayResolveAttribute(cx, wrapper, obj, id,
-                                nativeProperties->unforgeableAttributes,
-                                nativeProperties->unforgeableAttributeIds,
-                                nativeProperties->unforgeableAttributeSpecs,
-                                desc)) {
-        return false;
-      }
-      if (desc.object()) {
-        return true;
-      }
-    }
   }
 
   if (nativeProperties->constants) {

dom/bindings/Codegen.py

@@ -7280,22 +7280,6 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(ClassMethod):
                    CGIndenter(CGProxyIndexedGetter(self.descriptor, templateValues)).define() + "\n" +
                    "}\n") % (self.descriptor.nativeType)
 
-        if UseHolderForUnforgeable(self.descriptor):
-            getUnforgeable = """if (!JS_GetPropertyDescriptorById(cx, ${holder}, id, flags, desc)) {
-  return false;
-}
-MOZ_ASSERT_IF(desc.object(), desc.object() == ${holder});"""
-            getUnforgeable = CallOnUnforgeableHolder(self.descriptor,
-                                                     getUnforgeable, "isXray")
-            getUnforgeable += """if (desc.object()) {
-  desc.object().set(proxy);
-  return !isXray || JS_WrapPropertyDescriptor(cx, desc);
-}
-
-"""
-        else:
-            getUnforgeable = ""
-
         if indexedSetter or self.descriptor.operations['NamedSetter']:
             setOrIndexedGet += "if (flags & JSRESOLVE_ASSIGNING) {\n"
             if indexedSetter:
@@ -7309,7 +7293,6 @@ MOZ_ASSERT_IF(desc.object(), desc.object() == ${holder});"""
                 setOrIndexedGet += ("    FillPropertyDescriptor(desc, proxy, JSVAL_VOID, false);\n" +
                                     "    return true;\n" +
                                     "  }\n")
-            setOrIndexedGet += CGIndenter(CGGeneric(getUnforgeable)).define()
             if self.descriptor.operations['NamedSetter']:
                 if not 'NamedCreator' in self.descriptor.operations:
                     # FIXME need to check that this is a 'supported property name'
@@ -7324,11 +7307,7 @@ MOZ_ASSERT_IF(desc.object(), desc.object() == ${holder});"""
             setOrIndexedGet += "}"
             if indexedGetter:
                 setOrIndexedGet += (" else {\n" +
-                                    CGIndenter(CGGeneric(get + "\n" + getUnforgeable)).define() +
-                                    "}")
-            else:
-                setOrIndexedGet += (" else {\n" +
-                                    CGIndenter(CGGeneric(getUnforgeable)).define() +
+                                    CGIndenter(CGGeneric(get)).define() +
                                     "}")
             setOrIndexedGet += "\n\n"
         else:
@@ -7336,7 +7315,6 @@ MOZ_ASSERT_IF(desc.object(), desc.object() == ${holder});"""
                 setOrIndexedGet += ("if (!(flags & JSRESOLVE_ASSIGNING)) {\n" +
                                     CGIndenter(CGGeneric(get)).define() +
                                     "}\n\n")
-            setOrIndexedGet += getUnforgeable
 
         if self.descriptor.supportsNamedProperties():
             readonly = toStringBool(self.descriptor.operations['NamedSetter'] is None)

dom/bindings/test/Makefile.in

@@ -68,11 +68,13 @@ MOCHITEST_FILES := \
   test_exception_messages.html \
   test_bug707564.html \
   test_defineProperty.html \
+  file_document_location_set_via_xray.html \
   $(NULL)
 
 MOCHITEST_CHROME_FILES = \
   test_bug775543.html \
   test_bug707564-chrome.html \
+  test_document_location_set_via_xray.html \
   $(NULL)
 
 ifdef GNU_CC

dom/bindings/test/file_document_location_set_via_xray.html

@@ -0,0 +1,5 @@
+<body>
+<script>
+document.x = 5;
+</script>
+</body>

dom/bindings/test/test_document_location_set_via_xray.html

@@ -0,0 +1,49 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=905493
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 905493</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=905493">Mozilla Bug 905493</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+<iframe id="t" src="http://example.org/tests/dom/bindings/test/file_document_location_set_via_xray.html"></iframe>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 905493 **/
+
+function test()
+{
+  var doc = document.getElementById("t").contentWindow.document;
+  ok(!("x" in doc), "Should have an Xray here");
+  is(doc.x, undefined, "Really should have an Xray here");
+  is(doc.wrappedJSObject.x, 5, "And wrapping the right thing");
+  document.getElementById("t").onload = function() {
+    ok(true, "Load happened");
+    SimpleTest.finish();
+  };
+  try {
+    // Test the forwarding location setter
+    doc.location = "chrome://mochikit/content/tests/SimpleTest/test.css";
+  } catch (e) {
+    // Load failed
+    ok(false, "Load failed");
+    SimpleTest.finish();
+  }
+}
+
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(test);
+
+</script>
+</pre>
+</body>
+</html>