Skip to content

Commit

Permalink
stream: improve respondWithNewView()
Browse files Browse the repository at this point in the history
This fixes validating an ArrayBufferView given to
ReadableStreamBYOBRequest.respondWithNewView() to improve the web
streams compatibility.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com
  • Loading branch information
daeyeon committed Jul 17, 2022
1 parent dabda03 commit 3dc3627
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 3 deletions.
12 changes: 12 additions & 0 deletions lib/internal/webstreams/readablestream.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ const {
Uint8Array,
} = primordials;

const { arrayBufferIsDetached } = internalBinding('util');

const {
codes: {
ERR_ILLEGAL_CONSTRUCTOR,
Expand Down Expand Up @@ -681,6 +683,16 @@ class ReadableStreamBYOBRequest {
'This BYOB request has been invalidated');
}

const viewedBuffer = ArrayBufferViewGetBuffer(view);
const viewedBufferByteLength = ArrayBufferGetByteLength(viewedBuffer);
const isDetached = arrayBufferIsDetached(viewedBuffer);

if (viewedBufferByteLength === 0 && isDetached) {
throw new ERR_INVALID_STATE.TypeError(
'Viewed ArrayBuffer is detached',
);
}

readableByteStreamControllerRespondWithNewView(controller, view);
}

Expand Down
8 changes: 8 additions & 0 deletions src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,12 @@ void ArrayBufferViewHasBuffer(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(args[0].As<ArrayBufferView>()->HasBuffer());
}

void ArrayBufferIsDetached(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsArrayBuffer());
v8::Local<v8::ArrayBuffer> ab = args[0].As<v8::ArrayBuffer>();
args.GetReturnValue().Set(ab->GetBackingStore()->Data() == nullptr);
}

class WeakReference : public BaseObject {
public:
WeakReference(Environment* env, Local<Object> object, Local<Object> target)
Expand Down Expand Up @@ -334,6 +340,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(GetExternalValue);
registry->Register(Sleep);
registry->Register(ArrayBufferViewHasBuffer);
registry->Register(ArrayBufferIsDetached);
registry->Register(WeakReference::New);
registry->Register(WeakReference::Get);
registry->Register(WeakReference::IncRef);
Expand Down Expand Up @@ -380,6 +387,7 @@ void Initialize(Local<Object> target,
env->SetMethod(target, "sleep", Sleep);

env->SetMethod(target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer);
env->SetMethod(target, "arrayBufferIsDetached", ArrayBufferIsDetached);
Local<Object> constants = Object::New(env->isolate());
NODE_DEFINE_CONSTANT(constants, ALL_PROPERTIES);
NODE_DEFINE_CONSTANT(constants, ONLY_WRITABLE);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';

const common = require('../common');
const assert = require('node:assert');

{
// ReadableStream with byte source: respondWithNewView() throws if the
// supplied view's buffer has a different length (in the closed state)
const stream = new ReadableStream({
pull: common.mustCall(async (c) => {
const view = new Uint8Array(new ArrayBuffer(10), 0, 0);

c.close();

assert.throws(() => {
c.byobRequest.respondWithNewView(view);
}, RangeError);
}),
type: 'bytes',
});

const reader = stream.getReader({ mode: 'byob' });
reader.read(new Uint8Array([4, 5, 6]));
}

{
// ReadableStream with byte source: respondWithNewView() throws if the
// supplied view's buffer has been detached (in the closed state)
const stream = new ReadableStream({
pull: common.mustCall((c) => {
c.close();

// Detach it by reading into it
const view = new Uint8Array([1, 2, 3]);
reader.read(view);

assert.throws(() => {
c.byobRequest.respondWithNewView(view);
}, TypeError);
}),
type: 'bytes',
});

const reader = stream.getReader({ mode: 'byob' });
reader.read(new Uint8Array([4, 5, 6]));
}
4 changes: 1 addition & 3 deletions test/parallel/test-whatwg-readablebytestream.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,7 @@ class Source {
this.controller.close();
}

assert.throws(() => byobRequest.respondWithNewView({}), {
code: 'ERR_INVALID_ARG_TYPE',
});
assert.throws(() => byobRequest.respondWithNewView({}), TypeError);

byobRequest.respond(bytesRead);

Expand Down

0 comments on commit 3dc3627

Please sign in to comment.