From 50ab5572f3512e35c6568087bd51be16fa388efa Mon Sep 17 00:00:00 2001 From: Debadree Chatterjee Date: Tue, 19 Sep 2023 13:52:35 +0530 Subject: [PATCH 1/3] lib: allow byob reader for 'blob.stream()' Fixes: https://github.com/nodejs/node/issues/47993 --- lib/internal/blob.js | 4 ++- test/parallel/test-blob.js | 46 +++++++++++++++++++++++++++++-- test/wpt/status/FileAPI/blob.json | 7 ----- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/lib/internal/blob.js b/lib/internal/blob.js index c5b417ddc291b6..dacd27972dcb74 100644 --- a/lib/internal/blob.js +++ b/lib/internal/blob.js @@ -320,6 +320,7 @@ class Blob { const reader = this[kHandle].getReader(); return new lazyReadableStream({ + type: 'bytes', start(c) { // There really should only be one read at a time so using an // array here is purely defensive. @@ -339,6 +340,7 @@ class Blob { if (status === 0) { // EOS c.close(); + c.byobRequest?.respond(0); const pending = this.pendingPulls.shift(); pending.resolve(); return; @@ -358,7 +360,7 @@ class Blob { // We keep reading until we either reach EOS, some error, or we // hit the flow rate of the stream (c.desiredSize). queueMicrotask(() => { - if (c.desiredSize <= 0) { + if (c.desiredSize < 0) { // A manual backpressure check. if (this.pendingPulls.length !== 0) { // A case of waiting pull finished (= not yet canceled) diff --git a/test/parallel/test-blob.js b/test/parallel/test-blob.js index a517bad1ccb42d..370db2025580a4 100644 --- a/test/parallel/test-blob.js +++ b/test/parallel/test-blob.js @@ -331,12 +331,54 @@ assert.throws(() => new Blob({}), { const b = new Blob(Array(10).fill('hello')); const stream = b.stream(); const reader = stream.getReader(); - assert.strictEqual(stream[kState].controller.desiredSize, 1); + assert.strictEqual(stream[kState].controller.desiredSize, 0); const { value, done } = await reader.read(); assert.strictEqual(value.byteLength, 5); assert(!done); setTimeout(() => { - assert.strictEqual(stream[kState].controller.desiredSize, 0); + // the blob stream is now a byte stream hence after the first read, + // it should pull in the next 'hello' which is 5 bytes hence -5. + assert.strictEqual(stream[kState].controller.desiredSize, -5); + }, 0); +})().then(common.mustCall()); + +(async () => { + const blob = new Blob(['hello', 'world']); + const stream = blob.stream(); + const reader = stream.getReader({ mode: 'byob' }); + const decoder = new TextDecoder(); + const chunks = []; + while (true) { + const { value, done } = await reader.read(new Uint8Array(100)); + if (done) break; + chunks.push(decoder.decode(value, { stream: true })); + } + assert.strictEqual(chunks.join(''), 'helloworld'); +})().then(common.mustCall()); + +(async () => { + const b = new Blob(Array(10).fill('hello')); + const stream = b.stream(); + const reader = stream.getReader({ mode: 'byob' }); + assert.strictEqual(stream[kState].controller.desiredSize, 0); + const { value, done } = await reader.read(new Uint8Array(100)); + assert.strictEqual(value.byteLength, 5); + assert(!done); + setTimeout(() => { + assert.strictEqual(stream[kState].controller.desiredSize, -5); + }, 0); +})().then(common.mustCall()); + +(async () => { + const b = new Blob(Array(10).fill('hello')); + const stream = b.stream(); + const reader = stream.getReader({ mode: 'byob' }); + assert.strictEqual(stream[kState].controller.desiredSize, 0); + const { value, done } = await reader.read(new Uint8Array(2)); + assert.strictEqual(value.byteLength, 2); + assert(!done); + setTimeout(() => { + assert.strictEqual(stream[kState].controller.desiredSize, -3); }, 0); })().then(common.mustCall()); diff --git a/test/wpt/status/FileAPI/blob.json b/test/wpt/status/FileAPI/blob.json index 017d931d7abdc4..8ea03bbc019992 100644 --- a/test/wpt/status/FileAPI/blob.json +++ b/test/wpt/status/FileAPI/blob.json @@ -44,12 +44,5 @@ }, "Blob-slice.any.js": { "skip": "Depends on File API" - }, - "Blob-stream.any.js": { - "fail": { - "expected": [ - "Reading Blob.stream() with BYOB reader" - ] - } } } From 23cf23ff6f010af64edbeb6bb5aa7949fa256b48 Mon Sep 17 00:00:00 2001 From: Debadree Chatterjee Date: Tue, 19 Sep 2023 16:03:23 +0530 Subject: [PATCH 2/3] fixup! fix structured cloning wpt and add comments --- lib/internal/blob.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/internal/blob.js b/lib/internal/blob.js index dacd27972dcb74..b6587f1d9937f1 100644 --- a/lib/internal/blob.js +++ b/lib/internal/blob.js @@ -340,6 +340,8 @@ class Blob { if (status === 0) { // EOS c.close(); + // this is to signal the end for byob readers + // see https://streams.spec.whatwg.org/#example-rbs-pull c.byobRequest?.respond(0); const pending = this.pendingPulls.shift(); pending.resolve(); @@ -354,7 +356,9 @@ class Blob { pending.reject(error); return; } - if (buffer !== undefined) { + // ReadableByteStreamController.enqueue errors if we submit a 0-length + // buffer. We need to check for that here. + if (buffer !== undefined && buffer.byteLength !== 0) { c.enqueue(new Uint8Array(buffer)); } // We keep reading until we either reach EOS, some error, or we From 7e446f1ffffc19aed08ae0cc126691ac698bbf71 Mon Sep 17 00:00:00 2001 From: Debadree Chatterjee Date: Tue, 19 Sep 2023 16:12:59 +0530 Subject: [PATCH 3/3] fixup! lint --- lib/internal/blob.js | 2 +- test/parallel/test-blob.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/blob.js b/lib/internal/blob.js index b6587f1d9937f1..a54adb615fbc17 100644 --- a/lib/internal/blob.js +++ b/lib/internal/blob.js @@ -340,7 +340,7 @@ class Blob { if (status === 0) { // EOS c.close(); - // this is to signal the end for byob readers + // This is to signal the end for byob readers // see https://streams.spec.whatwg.org/#example-rbs-pull c.byobRequest?.respond(0); const pending = this.pendingPulls.shift(); diff --git a/test/parallel/test-blob.js b/test/parallel/test-blob.js index 370db2025580a4..e17824d2833682 100644 --- a/test/parallel/test-blob.js +++ b/test/parallel/test-blob.js @@ -336,7 +336,7 @@ assert.throws(() => new Blob({}), { assert.strictEqual(value.byteLength, 5); assert(!done); setTimeout(() => { - // the blob stream is now a byte stream hence after the first read, + // The blob stream is now a byte stream hence after the first read, // it should pull in the next 'hello' which is 5 bytes hence -5. assert.strictEqual(stream[kState].controller.desiredSize, -5); }, 0);