Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetch: don't use pooled buffer in body mixin #3377

Closed
6 changes: 3 additions & 3 deletions lib/web/fetch/body.js
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ function bodyMixinMethods (instance) {
// given a byte sequence bytes: return a new ArrayBuffer
// whose contents are bytes.
return consumeBody(this, (bytes) => {
return new Uint8Array(bytes).buffer
return bytes.buffer
}, instance)
},

Expand Down Expand Up @@ -391,7 +391,7 @@ function bodyMixinMethods (instance) {
// with this and the following step given a byte sequence bytes: return the
// result of creating a Uint8Array from bytes in this’s relevant realm.
return consumeBody(this, (bytes) => {
return new Uint8Array(bytes)
return new Uint8Array(bytes.buffer)
}, instance)
}
}
Expand Down Expand Up @@ -441,7 +441,7 @@ async function consumeBody (object, convertBytesToJSValue, instance) {
// 5. If object’s body is null, then run successSteps with an
// empty byte sequence.
if (object[kState].body == null) {
successSteps(Buffer.allocUnsafe(0))
successSteps(Buffer.allocUnsafeSlow(0))
return promise.promise
}

Expand Down
37 changes: 36 additions & 1 deletion lib/web/fetch/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,41 @@ function isomorphicEncode (input) {
return input
}

/** @type {Uint8ArrayConstructor} */
const FastBuffer = Buffer[Symbol.species]

/**
* Buffer is always cloned and does not use buffer pools.
* @param {Uint8Array[]} buffers
* @param {number} [length]
* @returns {Buffer}
*/
function safeBufferConcat (buffers, length) {
KhafraDev marked this conversation as resolved.
Show resolved Hide resolved
if (buffers.length === 0 || length === 0) {
return new FastBuffer(0)
}
if (buffers.length === 1) {
return new FastBuffer(buffers[0])
}
let offset = 0
if (length === undefined) {
length = 0
for (let i = 0; i < buffers.length; ++i) {
length += buffers[i].length
}
}
const output = Buffer.allocUnsafeSlow(length)
for (let i = 0; i < buffers.length; ++i) {
const buffer = buffers[i]
output.set(buffer, offset)
offset += buffer.length
}
if (offset < length) {
output.fill(0, offset, length)
}
return output
}

/**
* @see https://streams.spec.whatwg.org/#readablestreamdefaultreader-read-all-bytes
* @see https://streams.spec.whatwg.org/#read-loop
Expand All @@ -1113,7 +1148,7 @@ async function readAllBytes (reader) {

if (done) {
// 1. Call successSteps with bytes.
return Buffer.concat(bytes, byteLength)
return safeBufferConcat(bytes, byteLength)
}

// 1. If chunk is not a Uint8Array object, call failureSteps
Expand Down
24 changes: 24 additions & 0 deletions test/fetch/body-mixin-no-pooled-buffer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict'

const { Response } = require('../../')
const assert = require('node:assert')
const { test } = require('node:test')

test('Do not use pooled buffer in body mixin', async () => {
const allocUnsafe = Buffer.allocUnsafe

try {
let counter = 0
Buffer.allocUnsafe = function (...args) {
counter++
return allocUnsafe(...args)
}
// Do not use Buffer.allocUnsafe as it exposes the body to the pooled buffer.
await new Response('...any body').text()
// Body will be printed included.
// console.log(new TextDecoder().decode(Buffer.allocUnsafe(1).buffer))
assert.strictEqual(counter, 0)
} finally {
Buffer.allocUnsafe = allocUnsafe
}
})
Loading