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

fix #3337 #3338

Merged
merged 2 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 10 additions & 12 deletions lib/web/fetch/body.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ function bodyMixinMethods (instance) {
// Return a Blob whose contents are bytes and type attribute
// is mimeType.
return new Blob([bytes], { type: mimeType })
}, instance, false)
}, instance)
},

arrayBuffer () {
Expand All @@ -318,21 +318,20 @@ function bodyMixinMethods (instance) {
// given a byte sequence bytes: return a new ArrayBuffer
// whose contents are bytes.
return consumeBody(this, (bytes) => {
// Note: arrayBuffer already cloned.
return bytes.buffer
}, instance, true)
return new Uint8Array(bytes).buffer
tsctx marked this conversation as resolved.
Show resolved Hide resolved
}, instance)
},

text () {
// The text() method steps are to return the result of running
// consume body with this and UTF-8 decode.
return consumeBody(this, utf8DecodeBytes, instance, false)
return consumeBody(this, utf8DecodeBytes, instance)
},

json () {
// The json() method steps are to return the result of running
// consume body with this and parse JSON from bytes.
return consumeBody(this, parseJSONFromBytes, instance, false)
return consumeBody(this, parseJSONFromBytes, instance)
},

formData () {
Expand Down Expand Up @@ -384,16 +383,16 @@ function bodyMixinMethods (instance) {
throw new TypeError(
'Content-Type was not one of "multipart/form-data" or "application/x-www-form-urlencoded".'
)
}, instance, false)
}, instance)
},

bytes () {
// The bytes() method steps are to return the result of running consume body
// 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.buffer, 0, bytes.byteLength)
}, instance, true)
return new Uint8Array(bytes)
}, instance)
}
}

Expand All @@ -409,9 +408,8 @@ function mixinBody (prototype) {
* @param {Response|Request} object
* @param {(value: unknown) => unknown} convertBytesToJSValue
* @param {Response|Request} instance
* @param {boolean} [shouldClone]
*/
async function consumeBody (object, convertBytesToJSValue, instance, shouldClone) {
async function consumeBody (object, convertBytesToJSValue, instance) {
webidl.brandCheck(object, instance)

// 1. If object is unusable, then return a promise rejected
Expand Down Expand Up @@ -449,7 +447,7 @@ async function consumeBody (object, convertBytesToJSValue, instance, shouldClone

// 6. Otherwise, fully read object’s body given successSteps,
// errorSteps, and object’s relevant global object.
await fullyReadBody(object[kState].body, successSteps, errorSteps, shouldClone)
await fullyReadBody(object[kState].body, successSteps, errorSteps)

// 7. Return promise.
return promise.promise
Expand Down
14 changes: 3 additions & 11 deletions lib/web/fetch/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -1026,7 +1026,7 @@ function iteratorMixin (name, object, kInternalIterator, keyIndex = 0, valueInde
/**
* @see https://fetch.spec.whatwg.org/#body-fully-read
*/
async function fullyReadBody (body, processBody, processBodyError, shouldClone) {
async function fullyReadBody (body, processBody, processBodyError) {
// 1. If taskDestination is null, then set taskDestination to
// the result of starting a new parallel queue.

Expand All @@ -1052,7 +1052,7 @@ async function fullyReadBody (body, processBody, processBodyError, shouldClone)

// 5. Read all bytes from reader, given successSteps and errorSteps.
try {
successSteps(await readAllBytes(reader, shouldClone))
successSteps(await readAllBytes(reader))
} catch (e) {
errorSteps(e)
}
Expand Down Expand Up @@ -1100,9 +1100,8 @@ function isomorphicEncode (input) {
* @see https://streams.spec.whatwg.org/#readablestreamdefaultreader-read-all-bytes
* @see https://streams.spec.whatwg.org/#read-loop
* @param {ReadableStreamDefaultReader} reader
* @param {boolean} [shouldClone]
*/
async function readAllBytes (reader, shouldClone) {
async function readAllBytes (reader) {
const bytes = []
let byteLength = 0

Expand All @@ -1111,13 +1110,6 @@ async function readAllBytes (reader, shouldClone) {

if (done) {
// 1. Call successSteps with bytes.
if (bytes.length === 1) {
const { buffer, byteOffset, byteLength } = bytes[0]
if (shouldClone === false) {
return Buffer.from(buffer, byteOffset, byteLength)
}
return Buffer.from(buffer.slice(byteOffset, byteOffset + byteLength), 0, byteLength)
}
return Buffer.concat(bytes, byteLength)
Copy link
Member

@ChALkeR ChALkeR Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case wasn't cloned in shouldClone = true case in the same sense that the call in arrayBuffer() expected, as Buffer.concat returns a pooled Buffer, and arrayBuffer() returned just a .buffer of it

}

Expand Down
Loading