Skip to content

Commit

Permalink
fix(fetch): implement fully read body algorithm (#1597)
Browse files Browse the repository at this point in the history
  • Loading branch information
KhafraDev authored Aug 12, 2022
1 parent 0d1419c commit deed628
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 12 deletions.
15 changes: 4 additions & 11 deletions lib/fetch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ const {
sameOrigin,
isCancelled,
isAborted,
isErrorLike
isErrorLike,
fullyReadBody
} = require('./util')
const { kState, kHeaders, kGuard, kRealm } = require('./symbols')
const assert = require('assert')
Expand Down Expand Up @@ -738,11 +739,7 @@ async function mainFetch (fetchParams, recursive = false) {
}

// 4. Fully read response’s body given processBody and processBodyError.
try {
processBody(await response.arrayBuffer())
} catch (err) {
processBodyError(err)
}
await fullyReadBody(response.body, processBody, processBodyError)
} else {
// 21. Otherwise, run fetch finale given fetchParams and response.
fetchFinale(fetchParams, response)
Expand Down Expand Up @@ -974,11 +971,7 @@ async function fetchFinale (fetchParams, response) {
} else {
// 4. Otherwise, fully read response’s body given processBody, processBodyError,
// and fetchParams’s task destination.
try {
processBody(await response.body.stream.arrayBuffer())
} catch (err) {
processBodyError(err)
}
await fullyReadBody(response.body, processBody, processBodyError)
}
}
}
Expand Down
51 changes: 50 additions & 1 deletion lib/fetch/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { redirectStatus } = require('./constants')
const { performance } = require('perf_hooks')
const { isBlobLike, toUSVString, ReadableStreamFrom } = require('../core/util')
const assert = require('assert')
const { isUint8Array } = require('util/types')

let File

Expand Down Expand Up @@ -438,6 +439,53 @@ function makeIterator (iterator, name) {
return Object.setPrototypeOf({}, i)
}

/**
* @see https://fetch.spec.whatwg.org/#body-fully-read
*/
async function fullyReadBody (body, processBody, processBodyError) {
// 1. If taskDestination is null, then set taskDestination to
// the result of starting a new parallel queue.

// 2. Let promise be the result of fully reading body as promise
// given body.
try {
/** @type {Uint8Array[]} */
const chunks = []
let length = 0

const reader = body.stream.getReader()

while (true) {
const { done, value } = await reader.read()

if (done === true) {
break
}

// read-loop chunk steps
assert(isUint8Array(value))

chunks.push(value)
length += value.byteLength
}

// 3. Let fulfilledSteps given a byte sequence bytes be to queue
// a fetch task to run processBody given bytes, with
// taskDestination.
const fulfilledSteps = (bytes) => queueMicrotask(() => {
processBody(bytes)
})

fulfilledSteps(Buffer.concat(chunks, length))
} catch (err) {
// 4. Let rejectedSteps be to queue a fetch task to run
// processBodyError, with taskDestination.
queueMicrotask(() => processBodyError(err))
}

// 5. React to promise with fulfilledSteps and rejectedSteps.
}

/**
* Fetch supports node >= 16.8.0, but Object.hasOwn was added in v16.9.0.
*/
Expand Down Expand Up @@ -477,5 +525,6 @@ module.exports = {
isValidHeaderName,
isValidHeaderValue,
hasOwn,
isErrorLike
isErrorLike,
fullyReadBody
}
25 changes: 25 additions & 0 deletions test/fetch/client-fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -536,3 +536,28 @@ test('Receiving non-Latin1 headers', async (t) => {
t.same(lengths, [30, 34, 94, 104, 90])
t.end()
})

// https://github.com/nodejs/undici/issues/1594
// TODO(@KhafraDev): this test fails because of an integrity-mismatch check
// that hasn't been implemented when this comment was written. Enable this
// check once a resource's integrity is checked.
// test('with RequestInit.integrity set', async (t) => {
// const body = 'Hello world'
// const hash = require('crypto').createHash('sha256').update(body).digest('hex')
//
// const server = createServer((req, res) => {
// res.write(body)
// res.end()
// }).listen(0)
//
// t.teardown(server.close.bind(server))
// await once(server, 'listening')
//
// const response = await fetch(`http://localhost:${server.address().port}`, {
// integrity: `sha256-${hash}`
// })
//
// const ab = await response.arrayBuffer()
//
// t.same(new Uint8Array(ab), new TextEncoder().encode(body))
// })

0 comments on commit deed628

Please sign in to comment.