Bug 1268845. Make sure to set up an XPCWrappedNativeScope for SimpleGlobalObject globals on the main thread. r=bholley,ttaubert,ejpbruel a=lizzard
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 10 May 2016 20:57:29 -0400
changeset 334974 6dc895b3dea4e53ca1aaeaa2acb778512152588e
parent 334973 a2c46d6880e2ae4488c657c58c4bf71ece9d4433
child 334975 a0f48f447f41503a403053dc6f6b88a045a9d3a8
push id1146
push userCallek@gmail.com
push dateMon, 25 Jul 2016 16:35:44 +0000
treeherdermozilla-release@a55778f9cd5a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley, ttaubert, ejpbruel, lizzard
bugs1268845
milestone48.0a2
Bug 1268845. Make sure to set up an XPCWrappedNativeScope for SimpleGlobalObject globals on the main thread. r=bholley,ttaubert,ejpbruel a=lizzard
dom/bindings/SimpleGlobalObject.cpp
dom/crypto/test/test_WebCrypto_Wrap_Unwrap.html
dom/workers/WorkerPrivate.cpp
js/xpconnect/src/nsXPConnect.cpp
--- a/dom/bindings/SimpleGlobalObject.cpp
+++ b/dom/bindings/SimpleGlobalObject.cpp
@@ -91,29 +91,33 @@ JSObject*
 SimpleGlobalObject::Create(GlobalType globalType, JS::Handle<JS::Value> proto)
 {
   JSContext* cx = nsContentUtils::GetDefaultJSContextForThread();
   JSAutoRequest ar(cx);
 
   JS::CompartmentOptions options;
   options.creationOptions().setInvisibleToDebugger(true);
 
-  nsCOMPtr<nsIPrincipal> principal;
+  JS::Rooted<JSObject*> global(cx);
+
   if (NS_IsMainThread()) {
-    principal = nsNullPrincipal::Create();
+    nsCOMPtr<nsIPrincipal> principal = nsNullPrincipal::Create();
     if (!principal) {
       return nullptr;
     }
+    options.creationOptions().setTrace(xpc::TraceXPCGlobal);
+    global = xpc::CreateGlobalObject(cx, js::Jsvalify(&SimpleGlobalClass),
+                                     nsJSPrincipals::get(principal),
+                                     options);
+  } else {
+    global = JS_NewGlobalObject(cx, js::Jsvalify(&SimpleGlobalClass),
+                                nullptr,
+                                JS::DontFireOnNewGlobalHook, options);
   }
 
-  JS::Rooted<JSObject*> global(cx,
-    JS_NewGlobalObject(cx, js::Jsvalify(&SimpleGlobalClass),
-                       nsJSPrincipals::get(principal),
-                       JS::DontFireOnNewGlobalHook, options));
-
   if (!global) {
     JS_ClearPendingException(cx);
     return nullptr;
   }
 
   JSAutoCompartment ac(cx, global);
 
   // It's important to create the nsIGlobalObject for our new global before we
@@ -128,17 +132,17 @@ SimpleGlobalObject::Create(GlobalType gl
 
   if (proto.isObjectOrNull()) {
     JS::Rooted<JSObject*> protoObj(cx, proto.toObjectOrNull());
     if (!JS_WrapObject(cx, &protoObj)) {
       JS_ClearPendingException(cx);
       return nullptr;
     }
 
-    if (!JS_SetPrototype(cx, global, protoObj)) {
+    if (!JS_SplicePrototype(cx, global, protoObj)) {
       JS_ClearPendingException(cx);
       return nullptr;
     }
   } else if (!proto.isUndefined()) {
     // Bogus proto.
     return nullptr;
   }
 
--- a/dom/crypto/test/test_WebCrypto_Wrap_Unwrap.html
+++ b/dom/crypto/test/test_WebCrypto_Wrap_Unwrap.html
@@ -302,16 +302,48 @@ TestArray.addTest(
       .then(
         complete(that, function(x) {
           return exists(x.k) && x.k == originalKeyJwk.k;
         }),
         error(that)
       );
   }
 );
+
+// -----------------------------------------------------------------------------
+TestArray.addTest(
+  "JWK unwrap attempt on bogus data should error out",
+  function () {
+    // Largely cribbed from the "JWK wrap/unwrap round-trip, with AES-GCM" test
+    var that = this;
+    var wrapAlg = { name: "AES-GCM", iv: tv.aes_gcm_enc.iv };
+    var wrapKey;
+
+    function doBogusWrap() {
+      var abv = new TextEncoder("utf-8").encode("I am so not JSON");
+      return crypto.subtle.encrypt(wrapAlg, wrapKey, abv);
+    }
+    function doUnwrap(wrappedKey) {
+      return crypto.subtle.unwrapKey("jwk", wrappedKey, wrapKey, wrapAlg,
+                                     {name: "HMAC", hash: "SHA-384"},
+                                     true, ['sign', 'verify']);
+    }
+
+    crypto.subtle.importKey("jwk", tv.aes_gcm_enc.key_jwk,
+                            "AES-GCM", false, ['encrypt','unwrapKey'])
+      .then(function(x) { wrapKey = x; })
+      .then(doBogusWrap, error(that))
+      .then(doUnwrap, error(that))
+      .then(
+        error(that),
+        complete(that)
+      );
+  }
+);
+
 /*]]>*/</script>
 </head>
 
 <body>
 
 <div id="content">
 	<div id="head">
 		<b>Web</b>Crypto<br>
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -54,16 +54,17 @@
 #include "mozilla/dom/MessageEvent.h"
 #include "mozilla/dom/MessageEventBinding.h"
 #include "mozilla/dom/MessagePort.h"
 #include "mozilla/dom/MessagePortBinding.h"
 #include "mozilla/dom/MessagePortList.h"
 #include "mozilla/dom/PMessagePort.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/PromiseDebugging.h"
+#include "mozilla/dom/SimpleGlobalObject.h"
 #include "mozilla/dom/ScriptSettings.h"
 #include "mozilla/dom/StructuredCloneHolder.h"
 #include "mozilla/dom/TabChild.h"
 #include "mozilla/dom/WorkerBinding.h"
 #include "mozilla/dom/WorkerDebuggerGlobalScopeBinding.h"
 #include "mozilla/dom/WorkerGlobalScopeBinding.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/TimelineConsumers.h"
@@ -1061,36 +1062,50 @@ public:
 
         if (status == nsEventStatus_eConsumeNoDefault) {
           return;
         }
       }
 
       // Now fire an event at the global object, but don't do that if the error
       // code is too much recursion and this is the same script threw the error.
+      // XXXbz the interaction of this with worker errors seems kinda broken.
+      // An overrecursion in the debugger or debugger sandbox will get turned
+      // into an error event on our parent worker!
+      // https://bugzilla.mozilla.org/show_bug.cgi?id=1271441 tracks making this
+      // better.
       if (aFireAtScope && (aTarget || aErrorNumber != JSMSG_OVER_RECURSED)) {
         JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
         NS_ASSERTION(global, "This should never be null!");
 
         nsEventStatus status = nsEventStatus_eIgnore;
         nsIScriptGlobalObject* sgo;
 
         if (aWorkerPrivate) {
           WorkerGlobalScope* globalScope = nullptr;
           UNWRAP_WORKER_OBJECT(WorkerGlobalScope, global, globalScope);
 
           if (!globalScope) {
             WorkerDebuggerGlobalScope* globalScope = nullptr;
             UNWRAP_OBJECT(WorkerDebuggerGlobalScope, global, globalScope);
 
             MOZ_ASSERT_IF(globalScope, globalScope->GetWrapperPreserveColor() == global);
-            MOZ_ASSERT_IF(!globalScope, IsDebuggerSandbox(global));
-
-            aWorkerPrivate->ReportErrorToDebugger(aFilename, aLineNumber,
-                                                  aMessage);
+            if (globalScope || IsDebuggerSandbox(global)) {
+              aWorkerPrivate->ReportErrorToDebugger(aFilename, aLineNumber,
+                                                    aMessage);
+              return;
+            }
+
+            MOZ_ASSERT(SimpleGlobalObject::SimpleGlobalType(global) ==
+                         SimpleGlobalObject::GlobalType::BindingDetail);
+            // XXXbz We should really log this to console, but unwinding out of
+            // this stuff without ending up firing any events is ... hard.  Just
+            // return for now.
+            // https://bugzilla.mozilla.org/show_bug.cgi?id=1271441 tracks
+            // making this better.
             return;
           }
 
           MOZ_ASSERT(globalScope->GetWrapperPreserveColor() == global);
           nsIDOMEventTarget* target = static_cast<nsIDOMEventTarget*>(globalScope);
 
           RefPtr<ErrorEvent> event =
             ErrorEvent::Constructor(aTarget, NS_LITERAL_STRING("error"), init);
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -397,29 +397,32 @@ CreateGlobalObject(JSContext* cx, const 
     if (!global)
         return nullptr;
     JSAutoCompartment ac(cx, global);
 
     // The constructor automatically attaches the scope to the compartment private
     // of |global|.
     (void) new XPCWrappedNativeScope(cx, global);
 
+    if (clasp->flags & JSCLASS_DOM_GLOBAL) {
 #ifdef DEBUG
-    // Verify that the right trace hook is called. Note that this doesn't
-    // work right for wrapped globals, since the tracing situation there is
-    // more complicated. Manual inspection shows that they do the right thing.
-    if (!((const js::Class*)clasp)->isWrappedNative())
-    {
-        VerifyTraceProtoAndIfaceCacheCalledTracer trc(JS_GetRuntime(cx));
-        TraceChildren(&trc, GCCellPtr(global.get()));
-        MOZ_ASSERT(trc.ok, "Trace hook on global needs to call TraceXPCGlobal for XPConnect compartments.");
-    }
+        // Verify that the right trace hook is called. Note that this doesn't
+        // work right for wrapped globals, since the tracing situation there is
+        // more complicated. Manual inspection shows that they do the right
+        // thing.  Also note that we only check this for JSCLASS_DOM_GLOBAL
+        // classes because xpc::TraceXPCGlobal won't call
+        // TraceProtoAndIfaceCache unless that flag is set.
+        if (!((const js::Class*)clasp)->isWrappedNative())
+        {
+            VerifyTraceProtoAndIfaceCacheCalledTracer trc(JS_GetRuntime(cx));
+            TraceChildren(&trc, GCCellPtr(global.get()));
+            MOZ_ASSERT(trc.ok, "Trace hook on global needs to call TraceXPCGlobal for XPConnect compartments.");
+        }
 #endif
 
-    if (clasp->flags & JSCLASS_DOM_GLOBAL) {
         const char* className = clasp->name;
         AllocateProtoAndIfaceCache(global,
                                    (strcmp(className, "Window") == 0 ||
                                     strcmp(className, "ChromeWindow") == 0)
                                    ? ProtoAndIfaceCache::WindowLike
                                    : ProtoAndIfaceCache::NonWindowLike);
     }