From eb84ebf9746ac1dfca32ced145c2346e86f67244 Mon Sep 17 00:00:00 2001 From: hborchardt <66408901+hborchardt@users.noreply.github.com> Date: Sat, 6 Jan 2024 07:25:01 +0100 Subject: [PATCH] Fix multiple partial consume from BufferList (#8007) * Add test for multiple partial consume from BufferList This shows the problem indicated in #7385 * Fix multiple partial consume from BufferList The JSUint8Array::possiblySharedBuffer() returns the backing array, not taking into account the byteOffset that indicates the start of the data in the backing array. This means that when creating an array with the same backing array, the current byteOffset needs to be added to the start of the new slice. This led to consume() returning the same data when repeatedly consuming small numbers of bytes from the BufferList. --- src/bun.js/bindings/JSBufferList.cpp | 8 +++++--- test/js/node/stream/bufferlist.test.ts | 11 +++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/bun.js/bindings/JSBufferList.cpp b/src/bun.js/bindings/JSBufferList.cpp index e592fb96d2..f378cd954c 100644 --- a/src/bun.js/bindings/JSBufferList.cpp +++ b/src/bun.js/bindings/JSBufferList.cpp @@ -182,8 +182,9 @@ JSC::JSValue JSBufferList::_getBuffer(JSC::VM& vm, JSC::JSGlobalObject* lexicalG } if (n < len) { auto buffer = array->possiblySharedBuffer(); - JSC::JSUint8Array* retArray = JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, buffer, 0, n); - JSC::JSUint8Array* newArray = JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, buffer, n, len - n); + auto off = array->byteOffset(); + JSC::JSUint8Array* retArray = JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, buffer, off, n); + JSC::JSUint8Array* newArray = JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, buffer, off + n, len - n); m_deque.first().set(vm, this, newArray); RELEASE_AND_RETURN(throwScope, retArray); } @@ -207,7 +208,8 @@ JSC::JSValue JSBufferList::_getBuffer(JSC::VM& vm, JSC::JSGlobalObject* lexicalG return throwOutOfMemoryError(lexicalGlobalObject, throwScope); } auto buffer = array->possiblySharedBuffer(); - JSC::JSUint8Array* newArray = JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, buffer, n, len - n); + auto off = array->byteOffset(); + JSC::JSUint8Array* newArray = JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, buffer, off + n, len - n); element.set(vm, this, newArray); offset += n; break; diff --git a/test/js/node/stream/bufferlist.test.ts b/test/js/node/stream/bufferlist.test.ts index 8ab147d7e4..625ba03c9c 100644 --- a/test/js/node/stream/bufferlist.test.ts +++ b/test/js/node/stream/bufferlist.test.ts @@ -194,6 +194,17 @@ it("should work with .unshift()", () => { expect(list.shift()).toBeUndefined(); }); +it("should work with multiple partial .consume() from buffers", () => { + // @ts-ignore + const list = new Readable().readableBuffer; + expect(list.length).toBe(0); + expect(list.push(Buffer.from("f000baaa", "hex"))).toBeUndefined(); + expect(list.length).toBe(1); + expect(list.consume(2, undefined)).toEqual(Buffer.from("f000", "hex")); + expect(list.consume(1, undefined)).toEqual(Buffer.from("ba", "hex")); + expect(list.length).toBe(1); +}); + it("should work with partial .consume() followed by .first()", () => { // @ts-ignore const list = new Readable().readableBuffer;