Skip to content

Commit

Permalink
Adjust BufferSource for sandbox.
Browse files Browse the repository at this point in the history
  • Loading branch information
erikcorry committed Nov 20, 2024
1 parent e5cb622 commit 7a1716d
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 32 deletions.
2 changes: 1 addition & 1 deletion src/workerd/api/blob.c++
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ jsg::BufferSource wrap(jsg::Lock& js, kj::Array<byte> data) {
// the underlying v8::BackingStore is supposed to free the buffer when
// it is done with it. Unfortunately ASAN complains about a leak that
// will require more investigation.
// return jsg::BufferSource(js, jsg::BackingStore::from(kj::mv(data)));
// return jsg::BufferSource(js, jsg::BackingStore::from(js, kj::mv(data)));
}

kj::ArrayPtr<const kj::byte> getPtr(jsg::BufferSource& source) {
Expand Down
19 changes: 6 additions & 13 deletions src/workerd/api/crypto/crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,8 @@ class CryptoKey: public jsg::Object {
KJ_SWITCH_ONEOF(publicExponent) {
KJ_CASE_ONEOF(array, BigInteger) {
if (fixPublicExp) {
// alloc will, by default create a Uint8Array
auto expBack = jsg::BackingStore::alloc(js, array.size());
expBack.asArrayPtr().copyFrom(array);
auto expCopy = kj::heapArray<kj::byte>(array.asPtr());
jsg::BackingStore expBack = jsg::BackingStore::from(js, kj::mv(expCopy));
return {name, modulusLength, jsg::BufferSource(js, kj::mv(expBack)), hash};
} else {
auto expBack = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, array.size());
Expand All @@ -219,16 +218,10 @@ class CryptoKey: public jsg::Object {
}
KJ_CASE_ONEOF(source, jsg::BufferSource) {
// Should only happen if the flag is enabled and an algorithm field is cloned twice.
if (fixPublicExp) {
// alloc will, by default create a Uint8Array
auto expBack = jsg::BackingStore::alloc(js, source.size());
expBack.asArrayPtr().copyFrom(source);
return {name, modulusLength, jsg::BufferSource(js, kj::mv(expBack)), hash};
} else {
auto expBack = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, source.size());
expBack.asArrayPtr().copyFrom(source);
return {name, modulusLength, jsg::BufferSource(js, kj::mv(expBack)), hash};
}
KJ_ASSERT(fixPublicExp == true);
auto expCopy = kj::heapArray<kj::byte>(source.asArrayPtr());
jsg::BackingStore expBack = jsg::BackingStore::from(js, kj::mv(expCopy));
return {name, modulusLength, jsg::BufferSource(js, kj::mv(expBack)), hash};
}
}
KJ_UNREACHABLE;
Expand Down
6 changes: 3 additions & 3 deletions src/workerd/api/streams/standard-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ void preamble(auto callback) {
}

v8::Local<v8::Value> toBytes(jsg::Lock& js, kj::String str) {
return jsg::BackingStore::from(str.asBytes().attach(kj::mv(str))).createHandle(js);
return jsg::BackingStore::from(js, str.asBytes().attach(kj::mv(str))).createHandle(js);
}

jsg::BufferSource toBufferSource(jsg::Lock& js, kj::String str) {
auto backing = jsg::BackingStore::from(str.asBytes().attach(kj::mv(str))).createHandle(js);
auto backing = jsg::BackingStore::from(js, str.asBytes().attach(kj::mv(str))).createHandle(js);
return jsg::BufferSource(js, kj::mv(backing));
}

jsg::BufferSource toBufferSource(jsg::Lock& js, kj::Array<kj::byte> bytes) {
auto backing = jsg::BackingStore::from(kj::mv(bytes)).createHandle(js);
auto backing = jsg::BackingStore::from(js, kj::mv(bytes)).createHandle(js);
return jsg::BufferSource(js, kj::mv(backing));
}

Expand Down
2 changes: 1 addition & 1 deletion src/workerd/jsg/buffersource-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ struct BufferSourceContext: public jsg::Object, public jsg::ContextGlobal {
}

BufferSource makeBufferSource(jsg::Lock& js) {
return BufferSource(js, BackingStore::from(kj::arr<kj::byte>(1, 2, 3)));
return BufferSource(js, BackingStore::from(js, kj::arr<kj::byte>(1, 2, 3)));
}

BufferSource makeArrayBuffer(jsg::Lock& js) {
Expand Down
28 changes: 19 additions & 9 deletions src/workerd/jsg/buffersource.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,29 @@ using BufferSourceViewConstructor = v8::Local<v8::Value> (*)(Lock&, BackingStore
// the byte length, offset, element size, and constructor type allowing the view to be
// recreated.
//
// The BackingStore can be safely used outside of the isolate lock and can even be passed
// into another isolate if necessary.
// The BackingStore must be created in a particular isolate, but it can be
// safely used outside of the isolate lock. It cannot be passed to a different
// isolate, unless they are in the same isolate group, which means they are in
// the same sandbox.
class BackingStore {
public:
template <BufferSourceType T = v8::Uint8Array>
static BackingStore from(kj::Array<kj::byte> data) {
static BackingStore from(Lock& js, kj::Array<kj::byte> data) {
// Creates a new BackingStore that takes over ownership of the given kj::Array.
// The bytes may be moved if they are not inside the sandbox already.
size_t size = data.size();
auto ptr = new kj::Array<byte>(kj::mv(data));
return BackingStore(
v8::ArrayBuffer::NewBackingStore(ptr->begin(), size,
[](void*, size_t, void* ptr) { delete reinterpret_cast<kj::Array<byte>*>(ptr); }, ptr),
size, 0, getBufferSourceElementSize<T>(), construct<T>, checkIsIntegerType<T>());
if (js.v8Isolate->GetGroup().SandboxContains(data.begin())) {
auto ptr = new kj::Array<byte>(kj::mv(data));
return BackingStore(
v8::ArrayBuffer::NewBackingStore(ptr->begin(), size,
[](void*, size_t, void* ptr) { delete reinterpret_cast<kj::Array<byte>*>(ptr); }, ptr),
size, 0, getBufferSourceElementSize<T>(), construct<T>, checkIsIntegerType<T>());
} else {
auto result = BackingStore(v8::ArrayBuffer::NewBackingStore(js.v8Isolate, size), size, 0,
getBufferSourceElementSize<T>(), construct<T>, checkIsIntegerType<T>());
memcpy(result.asArrayPtr().begin(), data.begin(), size);
return result;
}
}

// Creates a new BackingStore of the given size.
Expand Down Expand Up @@ -449,7 +459,7 @@ class BufferSourceWrapper {
};

inline BufferSource Lock::arrayBuffer(kj::Array<kj::byte> data) {
return BufferSource(*this, BackingStore::from<v8::ArrayBuffer>(kj::mv(data)));
return BufferSource(*this, BackingStore::from<v8::ArrayBuffer>(*this, kj::mv(data)));
}

} // namespace workerd::jsg
3 changes: 2 additions & 1 deletion src/workerd/jsg/jsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -2603,7 +2603,8 @@ class Lock {
BufferSource bytes(kj::Array<kj::byte> data) KJ_WARN_UNUSED_RESULT;

// Returns a jsg::BufferSource whose underlying JavaScript handle is an ArrayBuffer
// as opposed to the default Uint8Array.
// as opposed to the default Uint8Array. May copy and move the bytes if they are
// not in the right sandbox.
BufferSource arrayBuffer(kj::Array<kj::byte> data) KJ_WARN_UNUSED_RESULT;

enum RegExpFlags {
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/jsg/jsvalue.c++
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ JsValue Lock::rangeError(kj::StringPtr message) {
}

BufferSource Lock::bytes(kj::Array<kj::byte> data) {
return BufferSource(*this, BackingStore::from(kj::mv(data)));
return BufferSource(*this, BackingStore::from(*this, kj::mv(data)));
}

JsSymbol Lock::symbol(kj::StringPtr str) {
Expand Down
3 changes: 0 additions & 3 deletions src/workerd/jsg/value.h
Original file line number Diff line number Diff line change
Expand Up @@ -931,9 +931,6 @@ class ArrayBufferWrapper {
// impossible to support unifying Array<T> and Vector<T> in the future (i.e. making all
// Array<T>s growable). So it may be best to stick with allocating an Array<byte> on the
// heap after all...

fprintf(stderr, "begin is %p\n", begin);

auto ownerPtr = new kj::Array<byte>(kj::mv(value));

std::unique_ptr<v8::BackingStore> backing =
Expand Down

0 comments on commit 7a1716d

Please sign in to comment.