Skip to content

Commit

Permalink
fix: backport reused connection fix (#4217)
Browse files Browse the repository at this point in the history
  • Loading branch information
salzhrani committed Aug 30, 2022
1 parent 5a79181 commit 76c38d2
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 3 deletions.
4 changes: 2 additions & 2 deletions fastify.js
@@ -1,6 +1,6 @@
'use strict'

const VERSION = '3.29.0'
const VERSION = '3.29.1'

const Avvio = require('avvio')
const http = require('http')
Expand Down Expand Up @@ -584,7 +584,7 @@ function fastify (options) {
// https://github.com/nodejs/node/blob/6ca23d7846cb47e84fd344543e394e50938540be/lib/_http_server.js#L666

// If the socket is not writable, there is no reason to try to send data.
if (socket.writable && socket.bytesWritten === 0) {
if (socket.writable) {
socket.write(`HTTP/1.1 400 Bad Request\r\nContent-Length: ${body.length}\r\nContent-Type: application/json\r\n\r\n${body}`)
}
socket.destroy(err)
Expand Down
48 changes: 47 additions & 1 deletion test/request-error.test.js
Expand Up @@ -2,6 +2,7 @@

const { connect } = require('net')
const t = require('tap')
const semver = require('semver')
const test = t.test
const Fastify = require('..')
const { kRequest } = require('../lib/symbols.js')
Expand Down Expand Up @@ -153,7 +154,7 @@ test('default clientError handler ignores sockets in destroyed state', t => {
})

test('default clientError handler destroys sockets in writable state', t => {
t.plan(1)
t.plan(2)

const fastify = Fastify({
bodyLimit: 1,
Expand All @@ -169,6 +170,9 @@ test('default clientError handler destroys sockets in writable state', t => {
},
destroy () {
t.pass('destroy should be called')
},
write (response) {
t.match(response, /^HTTP\/1.1 400 Bad Request/)
}
})
})
Expand All @@ -189,6 +193,9 @@ test('default clientError handler destroys http sockets in non-writable state',
},
destroy () {
t.pass('destroy should be called')
},
write (response) {
t.fail('write should not be called')
}
})
})
Expand Down Expand Up @@ -273,3 +280,42 @@ test('encapsulated error handler binding', t => {
t.equal(fastify.hello, undefined)
})
})

const skip = semver.lt(process.versions.node, '11.0.0')

test('default clientError replies with bad request on reused keep-alive connection', { skip }, t => {
t.plan(2)

let response = ''

const fastify = Fastify({
bodyLimit: 1,
keepAliveTimeout: 100
})

fastify.get('/', (request, reply) => {
reply.send('OK\n')
})

fastify.listen({ port: 0 }, function (err) {
t.error(err)
fastify.server.unref()

const client = connect(fastify.server.address().port)

client.on('data', chunk => {
response += chunk.toString('utf-8')
})

client.on('end', () => {
t.match(response, /^HTTP\/1.1 200 OK.*HTTP\/1.1 400 Bad Request/s)
})

client.resume()
client.write('GET / HTTP/1.1\r\n')
client.write('\r\n\r\n')
client.write('GET /?a b HTTP/1.1\r\n')
client.write('Connection: close\r\n')
client.write('\r\n\r\n')
})
})

0 comments on commit 76c38d2

Please sign in to comment.