Bug 1385890 - Fix Streams implementation in multiple-global uses. r=baku,tcampbell,jorendorff

Streams have multiple parts that can be JS objects from different compartments.
For example, the [[reader]] internal slot of a stream can point to a reader
object in another compartment.

This patch makes the ReadableStream implementation robust against mixing and
matching stream-related objects and methods from different globals.

This also removes ReadableStreamBYOBReader and ReadableStreamBYOBRequest for
now, with a view toward enabling basic ReadableStream features by default in
bug 1389628.

Differential Revision: https://phabricator.services.mozilla.com/D8450

dom/fetch/Fetch.cpp

@@ -56,9 +56,14 @@ namespace dom {
 namespace {
 
 void
-AbortStream(JSContext* aCx, JS::Handle<JSObject*> aStream)
+AbortStream(JSContext* aCx, JS::Handle<JSObject*> aStream, ErrorResult& aRv)
 {
-  if (!JS::ReadableStreamIsReadable(aStream)) {
+  bool isReadable;
+  if (!JS::ReadableStreamIsReadable(aCx, aStream, &isReadable)) {
+    aRv.StealExceptionFromJSContext(aCx);
+    return;
+  }
+  if (!isReadable) {
     return;
   }
 
@@ -69,7 +74,9 @@ AbortStream(JSContext* aCx, JS::Handle<JSObject*> aStream)
     return;
   }
 
-  JS::ReadableStreamError(aCx, aStream, value);
+  if (!JS::ReadableStreamError(aCx, aStream, value)) {
+    aRv.StealExceptionFromJSContext(aCx);
+  }
 }
 
 } // anonymous
@@ -1101,27 +1108,33 @@ FetchBody<Response>::~FetchBody();
 
 template <class Derived>
 bool
-FetchBody<Derived>::BodyUsed() const
+FetchBody<Derived>::GetBodyUsed(ErrorResult& aRv) const
 {
   if (mBodyUsed) {
     return true;
   }
 
-  // If this object is disturbed or locked, return false.
+  // If this stream is disturbed or locked, return true.
   if (mReadableStreamBody) {
     AutoJSAPI jsapi;
     if (!jsapi.Init(mOwner)) {
+      aRv.Throw(NS_ERROR_FAILURE);
       return true;
     }
 
     JSContext* cx = jsapi.cx();
-
     JS::Rooted<JSObject*> body(cx, mReadableStreamBody);
-    if (JS::ReadableStreamIsDisturbed(body) ||
-        JS::ReadableStreamIsLocked(body) ||
-        !JS::ReadableStreamIsReadable(body)) {
-      return true;
+    bool disturbed;
+    bool locked;
+    bool readable;
+    if (!JS::ReadableStreamIsDisturbed(cx, body, &disturbed) ||
+        !JS::ReadableStreamIsLocked(cx, body, &locked) ||
+        !JS::ReadableStreamIsReadable(cx, body, &readable)) {
+      aRv.StealExceptionFromJSContext(cx);
+      return false;
     }
+
+    return disturbed || locked || !readable;
   }
 
   return false;
@@ -1129,11 +1142,24 @@ FetchBody<Derived>::BodyUsed() const
 
 template
 bool
-FetchBody<Request>::BodyUsed() const;
+FetchBody<Request>::GetBodyUsed(ErrorResult&) const;
 
 template
 bool
-FetchBody<Response>::BodyUsed() const;
+FetchBody<Response>::GetBodyUsed(ErrorResult&) const;
+
+template <class Derived>
+bool
+FetchBody<Derived>::CheckBodyUsed() const
+{
+  IgnoredErrorResult result;
+  bool bodyUsed = GetBodyUsed(result);
+  if (result.Failed()) {
+    // Ignore the error.
+    return true;
+  }
+  return bodyUsed;
+}
 
 template <class Derived>
 void
@@ -1152,8 +1178,14 @@ FetchBody<Derived>::SetBodyUsed(JSContext* aCx, ErrorResult& aRv)
   // have to lock it now because it can have been shared with other objects.
   if (mReadableStreamBody) {
     JS::Rooted<JSObject*> readableStreamObj(aCx, mReadableStreamBody);
-    if (JS::ReadableStreamGetMode(readableStreamObj) ==
-          JS::ReadableStreamMode::ExternalSource) {
+
+    JS::ReadableStreamMode mode;
+    if (!JS::ReadableStreamGetMode(aCx, readableStreamObj, &mode)) {
+      aRv.StealExceptionFromJSContext(aCx);
+      return;
+    }
+
+    if (mode == JS::ReadableStreamMode::ExternalSource) {
       LockStream(aCx, readableStreamObj, aRv);
       if (NS_WARN_IF(aRv.Failed())) {
         return;
@@ -1192,7 +1224,11 @@ FetchBody<Derived>::ConsumeBody(JSContext* aCx, FetchConsumeType aType,
     return nullptr;
   }
 
-  if (BodyUsed()) {
+  bool bodyUsed = GetBodyUsed(aRv);
+  if (NS_WARN_IF(aRv.Failed())) {
+    return nullptr;
+  }
+  if (bodyUsed) {
     aRv.ThrowTypeError<MSG_FETCH_BODY_CONSUMED_ERROR>();
     return nullptr;
   }
@@ -1298,7 +1334,11 @@ FetchBody<Derived>::SetReadableStreamBody(JSContext* aCx, JSObject* aBody)
   bool aborted = signalImpl->Aborted();
   if (aborted) {
     JS::Rooted<JSObject*> body(aCx, mReadableStreamBody);
-    AbortStream(aCx, body);
+    IgnoredErrorResult result;
+    AbortStream(aCx, body, result);
+    if (NS_WARN_IF(result.Failed())) {
+      return;
+    }
   } else if (!IsFollowing()) {
     Follow(signalImpl);
   }
@@ -1341,7 +1381,11 @@ FetchBody<Derived>::GetBody(JSContext* aCx,
   MOZ_ASSERT(body);
 
   // If the body has been already consumed, we lock the stream.
-  if (BodyUsed()) {
+  bool bodyUsed = GetBodyUsed(aRv);
+  if (NS_WARN_IF(aRv.Failed())) {
+    return;
+  }
+  if (bodyUsed) {
     LockStream(aCx, body, aRv);
     if (NS_WARN_IF(aRv.Failed())) {
       return;
@@ -1351,7 +1395,10 @@ FetchBody<Derived>::GetBody(JSContext* aCx,
   RefPtr<AbortSignalImpl> signalImpl = DerivedClass()->GetSignalImpl();
   if (signalImpl) {
     if (signalImpl->Aborted()) {
-      AbortStream(aCx, body);
+      AbortStream(aCx, body, aRv);
+      if (NS_WARN_IF(aRv.Failed())) {
+        return;
+      }
     } else if (!IsFollowing()) {
       Follow(signalImpl);
     }
@@ -1379,8 +1426,14 @@ FetchBody<Derived>::LockStream(JSContext* aCx,
                                JS::HandleObject aStream,
                                ErrorResult& aRv)
 {
-  MOZ_ASSERT(JS::ReadableStreamGetMode(aStream) ==
-               JS::ReadableStreamMode::ExternalSource);
+#if DEBUG
+  JS::ReadableStreamMode streamMode;
+  if (!JS::ReadableStreamGetMode(aCx, aStream, &streamMode)) {
+    aRv.StealExceptionFromJSContext(aCx);
+    return;
+  }
+  MOZ_ASSERT(streamMode == JS::ReadableStreamMode::ExternalSource);
+#endif // DEBUG
 
   // This is native stream, creating a reader will not execute any JS code.
   JS::Rooted<JSObject*> reader(aCx,
@@ -1416,7 +1469,7 @@ FetchBody<Derived>::MaybeTeeReadableStreamBody(JSContext* aCx,
 {
   MOZ_DIAGNOSTIC_ASSERT(aStreamReader);
   MOZ_DIAGNOSTIC_ASSERT(aInputStream);
-  MOZ_DIAGNOSTIC_ASSERT(!BodyUsed());
+  MOZ_DIAGNOSTIC_ASSERT(!CheckBodyUsed());
 
   aBodyOut.set(nullptr);
   *aStreamReader = nullptr;
@@ -1431,7 +1484,12 @@ FetchBody<Derived>::MaybeTeeReadableStreamBody(JSContext* aCx,
   // If this is a ReadableStream with an external source, this has been
   // generated by a Fetch. In this case, Fetch will be able to recreate it
   // again when GetBody() is called.
-  if (JS::ReadableStreamGetMode(stream) == JS::ReadableStreamMode::ExternalSource) {
+  JS::ReadableStreamMode streamMode;
+  if (!JS::ReadableStreamGetMode(aCx, stream, &streamMode)) {
+    aRv.StealExceptionFromJSContext(aCx);
+    return;
+  }
+  if (streamMode == JS::ReadableStreamMode::ExternalSource) {
     aBodyOut.set(nullptr);
     return;
   }
@@ -1485,7 +1543,8 @@ FetchBody<Derived>::Abort()
   JSContext* cx = jsapi.cx();
 
   JS::Rooted<JSObject*> body(cx, mReadableStreamBody);
-  AbortStream(cx, body);
+  IgnoredErrorResult result;
+  AbortStream(cx, body, result);
 }
 
 template