Bug 1465602 part 2. Fix the interaction of default toJSON with Func-controlled exposure that examines the object's global. r=qdot
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 01 Jun 2018 12:17:10 -0400
changeset 420887 7c76daa75842e03504329d23ecdfe0b723d02494
parent 420886 916e5914d84acd420f61b5dd48027ddce9465e5d
child 420888 f63e09b914570efce8e047f4f51aaf58ead46f75
push id103916
push userbzbarsky@mozilla.com
push dateFri, 01 Jun 2018 16:17:39 +0000
treeherdermozilla-inbound@7c76daa75842 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot
bugs1465602
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1465602 part 2. Fix the interaction of default toJSON with Func-controlled exposure that examines the object's global. r=qdot
dom/bindings/Codegen.py
dom/bindings/DOMJSClass.h
dom/bindings/test/TestFunctions.cpp
dom/bindings/test/TestFunctions.h
dom/bindings/test/mochitest.ini
dom/bindings/test/test_toJSON.html
dom/webidl/TestFunctions.webidl
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -2864,68 +2864,85 @@ class CGNativeProperties(CGList):
         return CGList.define(self)
 
 
 class CGCollectJSONAttributesMethod(CGAbstractMethod):
     """
     Generate the CollectJSONAttributes method for an interface descriptor
     """
     def __init__(self, descriptor, toJSONMethod):
-        args = [Argument('JSContext*', 'aCx'),
+        args = [Argument('JSContext*', 'cx'),
                 Argument('JS::Handle<JSObject*>', 'obj'),
                 Argument('%s*' % descriptor.nativeType, 'self'),
-                Argument('JS::Rooted<JSObject*>&', 'aResult')]
+                Argument('JS::Rooted<JSObject*>&', 'result')]
         CGAbstractMethod.__init__(self, descriptor, 'CollectJSONAttributes',
                                   'bool', args, canRunScript=True)
         self.toJSONMethod = toJSONMethod
 
     def definition_body(self):
         ret = ''
         interface = self.descriptor.interface
         toJSONCondition = PropertyDefiner.getControllingCondition(self.toJSONMethod,
                                                                   self.descriptor)
+        needUnwrappedObj = False
         for m in interface.members:
             if m.isAttr() and not m.isStatic() and m.type.isJSONType():
                 getAndDefine = fill(
                     """
-                    JS::Rooted<JS::Value> temp(aCx);
-                    if (!get_${name}(aCx, obj, self, JSJitGetterCallArgs(&temp))) {
+                    JS::Rooted<JS::Value> temp(cx);
+                    if (!get_${name}(cx, obj, self, JSJitGetterCallArgs(&temp))) {
                       return false;
                     }
-                    if (!JS_DefineProperty(aCx, aResult, "${name}", temp, JSPROP_ENUMERATE)) {
+                    if (!JS_DefineProperty(cx, result, "${name}", temp, JSPROP_ENUMERATE)) {
                       return false;
                     }
                     """,
                     name=IDLToCIdentifier(m.identifier.name))
                 # Make sure we don't include things which are supposed to be
                 # disabled.  Things that either don't have disablers or whose
                 # disablers match the disablers for our toJSON method can't
                 # possibly be disabled, but other things might be.
                 condition = PropertyDefiner.getControllingCondition(m, self.descriptor)
                 if condition.hasDisablers() and condition != toJSONCondition:
+                    needUnwrappedObj = True;
                     ret += fill(
                         """
                         // This is unfortunately a linear scan through sAttributes, but we
                         // only do it for things which _might_ be disabled, which should
                         // help keep the performance problems down.
-                        if (IsGetterEnabled(aCx, obj, (JSJitGetterOp)get_${name}, sAttributes)) {
+                        if (IsGetterEnabled(cx, unwrappedObj, (JSJitGetterOp)get_${name}, sAttributes)) {
                           $*{getAndDefine}
                         }
                         """,
                         name=IDLToCIdentifier(m.identifier.name),
                         getAndDefine=getAndDefine)
                 else:
                     ret += fill(
                         """
                         { // scope for "temp"
                           $*{getAndDefine}
                         }
                         """,
                         getAndDefine=getAndDefine)
         ret += 'return true;\n'
+
+        if needUnwrappedObj:
+            ret= fill(
+                """
+                JS::Rooted<JSObject*> unwrappedObj(cx, js::CheckedUnwrap(obj));
+                if (!unwrappedObj) {
+                  // How did that happen?  We managed to get called with that
+                  // object as "this"!  Just give up on sanity.
+                  return false;
+                }
+
+                $*{ret}
+                """,
+                ret=ret);
+
         return ret
 
 
 class CGCreateInterfaceObjectsMethod(CGAbstractMethod):
     """
     Generate the CreateInterfaceObjects method for an interface descriptor.
 
     properties should be a PropertyArrays instance.
--- a/dom/bindings/DOMJSClass.h
+++ b/dom/bindings/DOMJSClass.h
@@ -3,16 +3,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_dom_DOMJSClass_h
 #define mozilla_dom_DOMJSClass_h
 
 #include "jsfriendapi.h"
+#include "js/Wrapper.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/Likely.h"
 
 #include "mozilla/dom/PrototypeList.h" // auto-generated
 
 #include "mozilla/dom/JSSlots.h"
 
@@ -149,16 +150,17 @@ struct PrefableDisablers {
   // even if "enabled" is set to true.  If the pointer is null the value of
   // "enabled" is used as-is.
   const PropertyEnabled enabledFunc;
 };
 
 template<typename T>
 struct Prefable {
   inline bool isEnabled(JSContext* cx, JS::Handle<JSObject*> obj) const {
+    MOZ_ASSERT(!js::IsWrapper(obj));
     if (MOZ_LIKELY(!disablers)) {
       return true;
     }
     return disablers->isEnabled(cx, obj);
   }
 
   // Things that can disable this set of specs. |nullptr| means "cannot be
   // disabled".
--- a/dom/bindings/test/TestFunctions.cpp
+++ b/dom/bindings/test/TestFunctions.cpp
@@ -1,14 +1,15 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include "mozilla/dom/BindingUtils.h"
 #include "mozilla/dom/TestFunctions.h"
 #include "mozilla/dom/TestFunctionsBinding.h"
 #include "nsStringBuffer.h"
 #include "mozITestInterfaceJS.h"
 #include "nsComponentManagerUtils.h"
 
 namespace mozilla {
 namespace dom {
@@ -103,16 +104,47 @@ TestFunctions::TestThrowNsresultFromNati
 
 already_AddRefed<Promise>
 TestFunctions::ThrowToRejectPromise(GlobalObject& aGlobal, ErrorResult& aError)
 {
   aError.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
   return nullptr;
 }
 
+int32_t
+TestFunctions::One() const
+{
+  return 1;
+}
+
+int32_t
+TestFunctions::Two() const
+{
+  return 2;
+}
+
+bool
+TestFunctions::ObjectFromAboutBlank(JSContext* aCx, JSObject* aObj)
+{
+  // We purposefully don't use WindowOrNull here, because we want to
+  // demonstrate the incorrect behavior we get, not just fail some asserts.
+  RefPtr<nsGlobalWindowInner> win;
+  UNWRAP_OBJECT(Window, aObj, win);
+  if (!win) {
+    return false;
+  }
+
+  nsIDocument* doc = win->GetDoc();
+  if (!doc) {
+    return false;
+  }
+
+  return doc->GetDocumentURI()->GetSpecOrDefault().EqualsLiteral("about:blank");
+}
+
 bool
 TestFunctions::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto,
                           JS::MutableHandle<JSObject*> aWrapper)
 {
   return TestFunctionsBinding::Wrap(aCx, this, aGivenProto, aWrapper);
 }
 
 }
--- a/dom/bindings/test/TestFunctions.h
+++ b/dom/bindings/test/TestFunctions.h
@@ -41,16 +41,21 @@ public:
                                 DOMString& aString);
 
   void TestThrowNsresult(ErrorResult& aError);
   void TestThrowNsresultFromNative(ErrorResult& aError);
   static already_AddRefed<Promise>
   ThrowToRejectPromise(GlobalObject& aGlobal,
                        ErrorResult& aError);
 
+  int32_t One() const;
+  int32_t Two() const;
+
+  static bool ObjectFromAboutBlank(JSContext* aCx, JSObject* aObj);
+
   bool WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto,
                   JS::MutableHandle<JSObject*> aWrapper);
 private:
   nsString mStringData;
 };
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/bindings/test/mochitest.ini
+++ b/dom/bindings/test/mochitest.ini
@@ -79,8 +79,10 @@ skip-if = debug == false
 skip-if = debug == false
 [test_oom_reporting.html]
 [test_domProxyArrayLengthGetter.html]
 [test_exceptionSanitization.html]
 skip-if = debug == false
 [test_stringBindings.html]
 skip-if = debug == false
 [test_jsimplemented_subclassing.html]
+[test_toJSON.html]
+skip-if = debug == false
new file mode 100644
--- /dev/null
+++ b/dom/bindings/test/test_toJSON.html
@@ -0,0 +1,59 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1465602
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1465602</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1465602">Mozilla Bug 1465602</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  <iframe></iframe>
+</div>
+<pre id="test">
+</pre>
+  <script type="application/javascript">
+
+  /** Test for Bug 1465602 **/
+
+  SimpleTest.waitForExplicitFinish();
+  SpecialPowers.pushPrefEnv({set: [['dom.expose_test_interfaces', true]]}, go);
+
+  function go() {
+    var ourObj = new TestFunctions();
+    is(ourObj.one, 1, "Basic sanity check for our 'one'");
+    is(ourObj.two, undefined, "Basic sanity check for our 'two'");
+
+    var otherObj = new frames[0].TestFunctions();
+    is(otherObj.one, 1, "Basic sanity check for subframe 'one'");
+    is(otherObj.two, 2, "Basic sanity check for subframe 'two'");
+
+    var ourToJSON = ourObj.toJSON();
+    is(ourToJSON.one, 1, "We should have correct value for 'one'");
+    is(ourToJSON.two, undefined, "We should have correct value for 'two'");
+    ok(!Object.hasOwnProperty(ourToJSON, "two"),
+       "We should not have a property named 'two'");
+
+    var otherToJSON = otherObj.toJSON();
+    is(otherToJSON.one, 1, "Subframe should have correct value for 'one'");
+    is(otherToJSON.two, 2, "Subframe should have correct value for 'two'");
+
+    var mixedToJSON = ourObj.toJSON.call(otherObj);
+    is(mixedToJSON.one, 1, "First test should have correct value for 'one'");
+    is(mixedToJSON.two, 2, "First test should have correct value for 'two'");
+
+    mixedToJSON = otherObj.toJSON.call(ourObj);
+    is(mixedToJSON.one, 1, "Second test should have correct value for 'one'");
+    is(mixedToJSON.two, undefined,
+       "Second test should have correct value for 'two'");
+
+    SimpleTest.finish();
+  }
+  </script>
+</body>
+</html>
--- a/dom/webidl/TestFunctions.webidl
+++ b/dom/webidl/TestFunctions.webidl
@@ -43,9 +43,17 @@ interface TestFunctions {
   [Throws]
   void testThrowNsresult();
   [Throws]
   void testThrowNsresultFromNative();
 
   // Throws an InvalidStateError to auto-create a rejected promise.
   [Throws]
   static Promise<any> throwToRejectPromise();
+
+  // Some attributes for the toJSON to work with.
+  readonly attribute long one;
+  [Func="mozilla::dom::TestFunctions::ObjectFromAboutBlank"]
+  readonly attribute long two;
+
+  // Testing for how default toJSON behaves.
+  [Default] object toJSON();
 };