Skip to content

Commit

Permalink
perf: allow keep alive for HEAD requests (nodejs#1858)
Browse files Browse the repository at this point in the history
  • Loading branch information
ronag authored and crysmags committed Feb 27, 2024
1 parent 40c7698 commit 64f86fa
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 6 deletions.
17 changes: 12 additions & 5 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ class Parser {

this.keepAlive = ''
this.contentLength = ''
this.connection = ''
this.maxResponseSize = client[kMaxResponseSize]
}

Expand Down Expand Up @@ -616,6 +617,8 @@ class Parser {
const key = this.headers[len - 2]
if (key.length === 10 && key.toString().toLowerCase() === 'keep-alive') {
this.keepAlive += buf.toString()
} else if (key.length === 10 && key.toString().toLowerCase() === 'connection') {
this.connection += buf.toString()
} else if (key.length === 14 && key.toString().toLowerCase() === 'content-length') {
this.contentLength += buf.toString()
}
Expand Down Expand Up @@ -709,7 +712,11 @@ class Parser {
assert.strictEqual(this.timeoutType, TIMEOUT_HEADERS)

this.statusCode = statusCode
this.shouldKeepAlive = shouldKeepAlive
this.shouldKeepAlive = (
shouldKeepAlive ||
// Override llhttp value which does not allow keepAlive for HEAD.
(request.method === 'HEAD' && !socket[kReset] && this.connection.toLowerCase() === 'keep-alive')
)

if (this.statusCode >= 200) {
const bodyTimeout = request.bodyTimeout != null
Expand Down Expand Up @@ -739,7 +746,7 @@ class Parser {
this.headers = []
this.headersSize = 0

if (shouldKeepAlive && client[kPipelining]) {
if (this.shouldKeepAlive && client[kPipelining]) {
const keepAliveTimeout = this.keepAlive ? util.parseKeepAliveTimeout(this.keepAlive) : null

if (keepAliveTimeout != null) {
Expand Down Expand Up @@ -769,7 +776,6 @@ class Parser {
}

if (request.method === 'HEAD') {
assert(socket[kReset])
return 1
}

Expand Down Expand Up @@ -843,6 +849,7 @@ class Parser {
this.bytesRead = 0
this.contentLength = ''
this.keepAlive = ''
this.connection = ''

assert(this.headers.length % 2 === 0)
this.headers = []
Expand Down Expand Up @@ -1376,8 +1383,8 @@ function write (client, request) {
socket[kReset] = true
}

if (reset) {
socket[kReset] = true
if (reset != null) {
socket[kReset] = reset
}

if (client[kMaxRequests] && socket[kCounter]++ >= client[kMaxRequests]) {
Expand Down
2 changes: 1 addition & 1 deletion lib/core/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class Request {

this.blocking = blocking == null ? false : blocking

this.reset = reset == null ? false : reset
this.reset = reset == null ? null : reset

this.host = null

Expand Down
62 changes: 62 additions & 0 deletions test/client-head-reset-override.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
'use strict'

const { createServer } = require('http')
const { test } = require('tap')
const { Client } = require('..')

test('override HEAD reset', (t) => {
const expected = 'testing123'
const server = createServer((req, res) => {
if (req.method === 'GET') {
res.write(expected)
}
res.end()
})
t.teardown(server.close.bind(server))

server.listen(0, () => {
const client = new Client(`http://localhost:${server.address().port}`)
t.teardown(client.close.bind(client))

let done
client.on('disconnect', () => {
if (!done) {
t.fail()
}
})

client.request({
path: '/',
method: 'HEAD',
reset: false
}, (err, res) => {
t.error(err)
res.body.resume()
})

client.request({
path: '/',
method: 'HEAD',
reset: false
}, (err, res) => {
t.error(err)
res.body.resume()
})

client.request({
path: '/',
method: 'GET',
reset: false
}, (err, res) => {
t.error(err)
let str = ''
res.body.on('data', (data) => {
str += data
}).on('end', () => {
t.same(str, expected)
done = true
t.end()
})
})
})
})

0 comments on commit 64f86fa

Please sign in to comment.