From e22845913df3fea5ce7760b1a35832711ac74555 Mon Sep 17 00:00:00 2001
From: Brian Smith <brian@dbsoft.org>
Date: Thu, 28 Sep 2023 22:06:25 -0500
Subject: [PATCH] Issue #1442 - Part 17 - Creating FetchStream as a out param
 in order to avoid JS hazards
 https://bugzilla.mozilla.org/show_bug.cgi?id=1128959

---
 dom/fetch/Fetch.cpp       |  9 +++------
 dom/fetch/FetchStream.cpp | 26 +++++++++++++++++---------
 dom/fetch/FetchStream.h   |  4 ++--
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/dom/fetch/Fetch.cpp b/dom/fetch/Fetch.cpp
index dfc65c840e..d53886e96c 100644
--- a/dom/fetch/Fetch.cpp
+++ b/dom/fetch/Fetch.cpp
@@ -1097,12 +1097,9 @@ FetchBody<Derived>::GetBody(JSContext* aCx,
     return;
   }
 
-  JS::Rooted<JSObject*> body(aCx,
-                             FetchStream::Create(aCx,
-                                                 this,
-                                                 DerivedClass()->GetParentObject(),
-                                                 inputStream,
-                                                 aRv));
+  JS::Rooted<JSObject*> body(aCx);
+  FetchStream::Create(aCx, this, DerivedClass()->GetParentObject(),
+                      inputStream, &body, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }
diff --git a/dom/fetch/FetchStream.cpp b/dom/fetch/FetchStream.cpp
index 94837504f1..fa4802a990 100644
--- a/dom/fetch/FetchStream.cpp
+++ b/dom/fetch/FetchStream.cpp
@@ -104,10 +104,10 @@ private:
 NS_IMPL_ISUPPORTS(FetchStream, nsIInputStreamCallback, nsIObserver,
                   nsISupportsWeakReference)
 
-/* static */ JSObject*
+/* static */ void
 FetchStream::Create(JSContext* aCx, FetchStreamHolder* aStreamHolder,
                     nsIGlobalObject* aGlobal, nsIInputStream* aInputStream,
-                    ErrorResult& aRv)
+                    JS::MutableHandle<JSObject*> aStream, ErrorResult& aRv)
 {
   MOZ_DIAGNOSTIC_ASSERT(aCx);
   MOZ_DIAGNOSTIC_ASSERT(aInputStream);
@@ -119,12 +119,12 @@ FetchStream::Create(JSContext* aCx, FetchStreamHolder* aStreamHolder,
     nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
     if (NS_WARN_IF(!os)) {
       aRv.Throw(NS_ERROR_FAILURE);
-      return nullptr;
+      return;
     }
 
     aRv = os->AddObserver(stream, DOM_WINDOW_DESTROYED_TOPIC, true);
     if (NS_WARN_IF(aRv.Failed())) {
-      return nullptr;
+      return;
     }
 
   } else {
@@ -135,7 +135,7 @@ FetchStream::Create(JSContext* aCx, FetchStreamHolder* aStreamHolder,
       new FetchStreamWorkerHolder(stream));
     if (NS_WARN_IF(!holder->HoldWorker(workerPrivate, Closing))) {
       aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
-      return nullptr;
+      return;
     }
 
     // Note, this will create a ref-cycle between the holder and the stream.
@@ -158,14 +158,17 @@ FetchStream::Create(JSContext* aCx, FetchStreamHolder* aStreamHolder,
     JS::NewReadableExternalSourceStreamObject(aCx, stream, FETCH_STREAM_FLAG));
   if (!body) {
     aRv.StealExceptionFromJSContext(aCx);
-    return nullptr;
+    return;
   }
 
   stream->mReadableStream = body;
 
-  // JS engine will call the finalize callback.
+  // This will be released in FetchStream::FinalizeCallback().  We are
+  // guaranteed the jsapi will call FinalizeCallback when ReadableStream
+  // js object is finalized.
   NS_ADDREF(stream.get());
-  return body;
+
+  aStream.set(body);
 }
 
 /* static */ void
@@ -307,7 +310,11 @@ FetchStream::CancelCallback(JSContext* aCx, JS::HandleObject aStream,
   MOZ_DIAGNOSTIC_ASSERT(aUnderlyingSource);
   MOZ_DIAGNOSTIC_ASSERT(aFlags == FETCH_STREAM_FLAG);
 
-  RefPtr<FetchStream> stream = static_cast<FetchStream*>(aUnderlyingSource);
+  // This is safe because we created an extra reference in FetchStream::Create()
+  // that won't be released until FetchStream::FinalizeCallback() is called.
+  // We are guaranteed that won't happen until the js ReadableStream object
+  // is finalized.
+  FetchStream* stream = static_cast<FetchStream*>(aUnderlyingSource);
 
   if (stream->mInputStream) {
     stream->mInputStream->CloseWithStatus(NS_BASE_STREAM_CLOSED);
@@ -343,6 +350,7 @@ FetchStream::FinalizeCallback(void* aUnderlyingSource, uint8_t aFlags)
 
   // This can be called in any thread.
 
+  // This takes ownership of the ref created in FetchStream::Create().
   RefPtr<FetchStream> stream =
     dont_AddRef(static_cast<FetchStream*>(aUnderlyingSource));
 
diff --git a/dom/fetch/FetchStream.h b/dom/fetch/FetchStream.h
index b27588eda0..4562b3f351 100644
--- a/dom/fetch/FetchStream.h
+++ b/dom/fetch/FetchStream.h
@@ -35,10 +35,10 @@ public:
   NS_DECL_NSIINPUTSTREAMCALLBACK
   NS_DECL_NSIOBSERVER
 
-  static JSObject*
+  static void
   Create(JSContext* aCx, FetchStreamHolder* aStreamHolder,
          nsIGlobalObject* aGlobal, nsIInputStream* aInputStream,
-         ErrorResult& aRv);
+         JS::MutableHandle<JSObject*> aStream, ErrorResult& aRv);
 
   void
   Close();