Skip to content

Commit

Permalink
fix: define conditions when content-length should be sent (#2305)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pxue
pxue committed Oct 26, 2023
1 parent a3a437e commit fc29aa0
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 51 deletions.
13 changes: 11 additions & 2 deletions lib/client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1499,6 +1499,11 @@ function _resume (client, sync) {
}
}

// https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2
function shouldSendContentLength (method) {
return method !== 'GET' && method !== 'HEAD' && method !== 'OPTIONS' && method !== 'TRACE' && method !== 'CONNECT'
}

function write (client, request) {
if (client[kHTTPConnVersion] === 'h2') {
writeH2(client, client[kHTTP2Session], request)
Expand Down Expand Up @@ -1542,7 +1547,9 @@ function write (client, request) {
contentLength = null
}

if (request.contentLength !== null && request.contentLength !== contentLength) {
// https://github.com/nodejs/undici/issues/2046
// A user agent may send a Content-Length header with 0 value, this should be allowed.
if (shouldSendContentLength(method) && contentLength > 0 && request.contentLength !== null && request.contentLength !== contentLength) {
if (client[kStrictContentLength]) {
errorRequest(client, request, new RequestContentLengthMismatchError())
return false
Expand Down Expand Up @@ -1763,7 +1770,9 @@ function writeH2 (client, session, request) {
contentLength = null
}

if (request.contentLength != null && request.contentLength !== contentLength) {
// https://github.com/nodejs/undici/issues/2046
// A user agent may send a Content-Length header with 0 value, this should be allowed.
if (shouldSendContentLength(method) && contentLength > 0 && request.contentLength != null && request.contentLength !== contentLength) {
if (client[kStrictContentLength]) {
errorRequest(client, request, new RequestContentLengthMismatchError())
return false
Expand Down
204 changes: 159 additions & 45 deletions test/content-length.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { Readable } = require('stream')
const { maybeWrapStream, consts } = require('./utils/async-iterators')

test('request invalid content-length', (t) => {
t.plan(10)
t.plan(7)

const server = createServer((req, res) => {
res.end()
Expand Down Expand Up @@ -61,54 +61,13 @@ test('request invalid content-length', (t) => {
t.type(err, errors.RequestContentLengthMismatchError)
})

client.request({
path: '/',
method: 'HEAD',
headers: {
'content-length': 10
}
}, (err, data) => {
t.type(err, errors.RequestContentLengthMismatchError)
})

client.request({
path: '/',
method: 'GET',
headers: {
'content-length': 0
}
}, (err, data) => {
t.type(err, errors.RequestContentLengthMismatchError)
})

client.request({
path: '/',
method: 'GET',
headers: {
'content-length': 4
},
body: new Readable({
read () {
this.push('asd')
this.push(null)
}
})
}, (err, data) => {
t.type(err, errors.RequestContentLengthMismatchError)
})

client.request({
path: '/',
method: 'GET',
headers: {
'content-length': 4
},
body: new Readable({
read () {
this.push('asasdasdasdd')
this.push(null)
}
})
body: ['asd']
}, (err, data) => {
t.type(err, errors.RequestContentLengthMismatchError)
})
Expand All @@ -119,14 +78,14 @@ test('request invalid content-length', (t) => {
headers: {
'content-length': 4
},
body: ['asd']
body: ['asasdasdasdd']
}, (err, data) => {
t.type(err, errors.RequestContentLengthMismatchError)
})

client.request({
path: '/',
method: 'GET',
method: 'DELETE',
headers: {
'content-length': 4
},
Expand Down Expand Up @@ -329,3 +288,158 @@ test('request streaming with Readable.from(buf)', (t) => {
})
})
})

test('request DELETE, content-length=0, with body', (t) => {
t.plan(5)
const server = createServer((req, res) => {
res.end()
})
server.on('request', (req, res) => {
t.equal(req.headers['content-length'], undefined)
})
t.teardown(server.close.bind(server))
server.listen(0, () => {
const client = new Client(`http://localhost:${server.address().port}`)
t.teardown(client.destroy.bind(client))

client.request({
path: '/',
method: 'DELETE',
headers: {
'content-length': 0
},
body: new Readable({
read () {
this.push('asd')
this.push(null)
}
})
}, (err) => {
t.type(err, errors.RequestContentLengthMismatchError)
})

client.request({
path: '/',
method: 'DELETE',
headers: {
'content-length': 0
}
}, (err, resp) => {
t.equal(resp.headers['content-length'], '0')
t.error(err)
})

client.on('disconnect', () => {
t.pass()
})
})
})

test('content-length shouldSendContentLength=false', (t) => {
t.plan(15)
const server = createServer((req, res) => {
res.end()
})
server.on('request', (req, res) => {
switch (req.url) {
case '/put0':
t.equal(req.headers['content-length'], '0')
break
case '/head':
t.equal(req.headers['content-length'], undefined)
break
case '/get':
t.equal(req.headers['content-length'], undefined)
break
}
})
t.teardown(server.close.bind(server))
server.listen(0, () => {
const client = new Client(`http://localhost:${server.address().port}`)
t.teardown(client.destroy.bind(client))

client.request({
path: '/put0',
method: 'PUT',
headers: {
'content-length': 0
}
}, (err, resp) => {
t.equal(resp.headers['content-length'], '0')
t.error(err)
})

client.request({
path: '/head',
method: 'HEAD',
headers: {
'content-length': 10
}
}, (err, resp) => {
t.equal(resp.headers['content-length'], undefined)
t.error(err)
})

client.request({
path: '/get',
method: 'GET',
headers: {
'content-length': 0
}
}, (err) => {
t.error(err)
})

client.request({
path: '/',
method: 'GET',
headers: {
'content-length': 4
},
body: new Readable({
read () {
this.push('asd')
this.push(null)
}
})
}, (err) => {
t.error(err)
})

client.request({
path: '/',
method: 'GET',
headers: {
'content-length': 4
},
body: new Readable({
read () {
this.push('asasdasdasdd')
this.push(null)
}
})
}, (err) => {
t.error(err)
})

client.request({
path: '/',
method: 'HEAD',
headers: {
'content-length': 4
},
body: new Readable({
read () {
this.push('asasdasdasdd')
this.push(null)
}
})
}, (err) => {
t.error(err)
})

client.on('disconnect', () => {
t.pass()
})
})
})
4 changes: 0 additions & 4 deletions test/no-strict-content-length.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ tap.test('strictContentLength: false', (t) => {
'content-length': 10
}
}, (err, data) => {
assertEmitWarningCalledAndReset()
t.error(err)
})

Expand All @@ -101,7 +100,6 @@ tap.test('strictContentLength: false', (t) => {
'content-length': 0
}
}, (err, data) => {
assertEmitWarningCalledAndReset()
t.error(err)
})

Expand All @@ -118,7 +116,6 @@ tap.test('strictContentLength: false', (t) => {
}
})
}, (err, data) => {
assertEmitWarningCalledAndReset()
t.error(err)
})

Expand All @@ -135,7 +132,6 @@ tap.test('strictContentLength: false', (t) => {
}
})
}, (err, data) => {
assertEmitWarningCalledAndReset()
t.error(err)
})
})
Expand Down

0 comments on commit fc29aa0

Please sign in to comment.