Bug 1823429 - Don't try another read from ChunkSteps r=smaug

Having another read request on an empty chunk sounds nice, but I guess it's not worth allowing recursion for this edge case. Now the next read request will happen asynchronously by the next OnOutputStreamReady callback, which is similar to what Blink does and the spec recommends. * Blink: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/fetch/readable_stream_bytes_consumer.cc;l=179-186;drc=059796845b1738dbf28ea76f0e9ff4b6f8787d3a (queues a microtask to prevent recursion) * Spec: https://streams.spec.whatwg.org/#readablestreamdefaultreader-read-all-bytes (See the note below "chunk steps") Differential Revision: https://phabricator.services.mozilla.com/D173813
iaxat · Apr 4, 2023 · e6777de · e6777de
1 parent 530972c
commit e6777de
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 3 deletions.
diff --git a/dom/fetch/FetchStreamReader.cpp b/dom/fetch/FetchStreamReader.cpp
@@ -336,7 +336,10 @@ void FetchStreamReader::ChunkSteps(JSContext* aCx, JS::Handle<JS::Value> aChunk,
   mBufferOffset = 0;
   mBufferRemaining = chunk.Length();
 
-  Process(aCx);
+  nsresult rv = WriteBuffer();
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    CloseAndRelease(aCx, NS_ERROR_DOM_ABORT_ERR);
+  }
 }
 
 void FetchStreamReader::CloseSteps(JSContext* aCx, ErrorResult& aRv) {
@@ -352,11 +355,11 @@ void FetchStreamReader::ErrorSteps(JSContext* aCx, JS::Handle<JS::Value> aError,
 }
 
 nsresult FetchStreamReader::WriteBuffer() {
-  MOZ_ASSERT(!mBuffer.IsEmpty());
+  MOZ_ASSERT(mBuffer.Length() == (mBufferOffset + mBufferRemaining));
 
   char* data = reinterpret_cast<char*>(mBuffer.Elements());
 
-  while (1) {
+  while (mBufferRemaining > 0) {
     uint32_t written = 0;
     nsresult rv =
         mPipeOut->Write(data + mBufferOffset, mBufferRemaining, &written);

diff --git a/testing/web-platform/tests/fetch/api/response/many-empty-chunks-crash.html b/testing/web-platform/tests/fetch/api/response/many-empty-chunks-crash.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<script>
+  new Response(new ReadableStream({
+    start(c) {
+
+      for (const i of new Array(40000).fill()) {
+        c.enqueue(new Uint8Array(0));
+      }
+      c.close();
+
+    }
+  })).text();
+</script>