Skip to content

Commit

Permalink
fix: handle request body as late as possible
Browse files Browse the repository at this point in the history
  • Loading branch information
ronag committed Feb 12, 2024
1 parent 0a069ab commit 55ac735
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 23 deletions.
45 changes: 43 additions & 2 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ const {
// Experimental
let h2ExperimentalWarned = false

let extractBody

const FastBuffer = Buffer[Symbol.species]

const kClosedResolve = Symbol('kClosedResolve')
Expand Down Expand Up @@ -1477,7 +1479,9 @@ function write (client, request) {
return
}

const { body, method, path, host, upgrade, headers, blocking, reset } = request
const { method, path, host, upgrade, blocking, reset } = request

let { body, headers, contentLength } = request

// https://tools.ietf.org/html/rfc7231#section-4.3.1
// https://tools.ietf.org/html/rfc7231#section-4.3.2
Expand All @@ -1494,14 +1498,42 @@ function write (client, request) {
method === 'PATCH'
)

if (util.isFormDataLike(body)) {
if (!extractBody) {
extractBody = require('./fetch/body.js').extractBody
}

const [bodyStream, contentType] = extractBody(body)
if (request.contentType == null) {
headers += `content-type: ${contentType}\r\n`
}
body = bodyStream.stream
contentLength = bodyStream.length
} else if (util.isBlobLike(body) && request.contentType == null && body.type) {
headers += `content-type: ${body.type}\r\n`
}

// TODO (fix): Error if stream is disturbed?

if (util.isStream(body) && body.errored) {
let err = body.errored
if (!err) {
err = new RequestAbortedError()
util.destroy(body, err)
}
body.on('error', () => {})
errorRequest(client, request, err)
return false
}

if (body && typeof body.read === 'function') {
// Try to read EOF in order to get length.
body.read(0)
}

const bodyLength = util.bodyLength(body)

let contentLength = bodyLength
contentLength = bodyLength ?? contentLength

if (contentLength === null) {
contentLength = request.contentLength
Expand Down Expand Up @@ -1544,6 +1576,7 @@ function write (client, request) {
}

if (request.aborted) {
util.destroy(body)
return false
}

Expand Down Expand Up @@ -2050,6 +2083,14 @@ function writeStream ({ h2stream, body, client, request, socket, contentLength,
socket
.on('drain', onDrain)
.on('error', onFinished)

if (body.readableEnded) {
queueMicrotask(onFinished)
}

if (body.closeEmitted ?? body.closed) {
queueMicrotask(onClose)
}
}

async function writeBlob ({ h2stream, body, client, request, socket, contentLength, header, expectsPayload }) {
Expand Down
19 changes: 0 additions & 19 deletions lib/core/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ const invalidPathRegex = /[^\u0021-\u00ff]/

const kHandler = Symbol('handler')

let extractBody

class Request {
constructor (origin, {
path,
Expand Down Expand Up @@ -173,23 +171,6 @@ class Request {
throw new InvalidArgumentError('headers must be an object or an array')
}

if (util.isFormDataLike(this.body)) {
if (!extractBody) {
extractBody = require('../fetch/body.js').extractBody
}

const [bodyStream, contentType] = extractBody(body)
if (this.contentType == null) {
this.contentType = contentType
this.headers += `content-type: ${contentType}\r\n`
}
this.body = bodyStream.stream
this.contentLength = bodyStream.length
} else if (util.isBlobLike(body) && this.contentType == null && body.type) {
this.contentType = body.type
this.headers += `content-type: ${body.type}\r\n`
}

util.validateHandler(handler, method, upgrade)

this.servername = util.getServerName(this.host)
Expand Down
4 changes: 2 additions & 2 deletions lib/core/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ function bodyLength (body) {
return null
}

function isDestroyed (stream) {
return !stream || !!(stream.destroyed || stream[kDestroyed])
function isDestroyed (body) {
return body && !!(body.destroyed || body[kDestroyed] || (stream.isDestroyed?.(body)))
}

function isReadableAborted (stream) {
Expand Down
1 change: 1 addition & 0 deletions test/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -1055,6 +1055,7 @@ test('basic POST with empty stream', (t) => {
method: 'POST',
body
}, (err, { statusCode, headers, body }) => {
console.error(err)
t.error(err)
body
.on('data', () => {
Expand Down

0 comments on commit 55ac735

Please sign in to comment.