Skip to content

Commit

Permalink
fix: post async regression with empty body (#5008)
Browse files Browse the repository at this point in the history
  • Loading branch information
mcollina committed Aug 31, 2023
1 parent 9b3ca58 commit 498b17c
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 27 deletions.
2 changes: 1 addition & 1 deletion lib/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ function multipleBindings (mainServer, httpHandler, serverOpts, listenOptions, o
// to the secondary servers. It is valid only when the user is
// listening on localhost
const originUnref = mainServer.unref
/* istanbul ignore next */
/* c8 ignore next 4 */
mainServer.unref = function () {
originUnref.call(mainServer)
mainServer.emit('unref')
Expand Down
5 changes: 1 addition & 4 deletions lib/wrapThenable.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ function wrapThenable (thenable, reply) {
// the request may be terminated during the reply. in this situation,
// it require an extra checking of request.aborted to see whether
// the request is killed by client.
// Most of the times aborted will be true when destroyed is true,
// however there is a race condition where the request is not
// aborted but only destroyed.
if (payload !== undefined || (reply.sent === false && reply.raw.headersSent === false && reply.request.raw.aborted === false && reply.request.raw.destroyed === false)) {
if (payload !== undefined || (reply.sent === false && reply.raw.headersSent === false && reply.request.raw.aborted === false)) {
// we use a try-catch internally to avoid adding a catch to another
// promise, increase promise perf by 10%
try {
Expand Down
31 changes: 31 additions & 0 deletions test/post-empty-body.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
'use strict'

const { test } = require('tap')
const fastify = require('../')
const { request, setGlobalDispatcher, Agent } = require('undici')

setGlobalDispatcher(new Agent({
keepAliveTimeout: 10,
keepAliveMaxTimeout: 10
}))

test('post empty body', async t => {
const app = fastify()
t.teardown(app.close.bind(app))

app.post('/bug', async (request, reply) => {
})

await app.listen({ port: 0 })

const res = await request(`http://127.0.0.1:${app.server.address().port}/bug`, {
method: 'POST',
headers: {
'Content-Type': 'application/json'
},
body: JSON.stringify({ foo: 'bar' })
})

t.equal(res.statusCode, 200)
t.equal(await res.body.text(), '')
})
22 changes: 0 additions & 22 deletions test/wrapThenable.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,3 @@ test('should reject immediately when reply[kReplyHijacked] is true', t => {
const thenable = Promise.reject(new Error('Reply sent already'))
wrapThenable(thenable, reply)
})

test('should not send the payload if the raw socket was destroyed but not aborted', async t => {
const reply = {
sent: false,
raw: {
headersSent: false
},
request: {
raw: {
aborted: false,
destroyed: true
}
},
send () {
t.fail('should not send')
}
}
const thenable = Promise.resolve()
wrapThenable(thenable, reply)

await thenable
})

0 comments on commit 498b17c

Please sign in to comment.