diff --git a/lib/client.js b/lib/client.js index 00f4467cc8b..0ac23f3ee99 100644 --- a/lib/client.js +++ b/lib/client.js @@ -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) @@ -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 @@ -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 diff --git a/test/content-length.js b/test/content-length.js index 66c010a7c9e..9ce74051d8b 100644 --- a/test/content-length.js +++ b/test/content-length.js @@ -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() @@ -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) }) @@ -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 }, @@ -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() + }) + }) +}) diff --git a/test/no-strict-content-length.js b/test/no-strict-content-length.js index 1d0eae6c789..993b0fdcf7a 100644 --- a/test/no-strict-content-length.js +++ b/test/no-strict-content-length.js @@ -90,7 +90,6 @@ tap.test('strictContentLength: false', (t) => { 'content-length': 10 } }, (err, data) => { - assertEmitWarningCalledAndReset() t.error(err) }) @@ -101,7 +100,6 @@ tap.test('strictContentLength: false', (t) => { 'content-length': 0 } }, (err, data) => { - assertEmitWarningCalledAndReset() t.error(err) }) @@ -118,7 +116,6 @@ tap.test('strictContentLength: false', (t) => { } }) }, (err, data) => { - assertEmitWarningCalledAndReset() t.error(err) }) @@ -135,7 +132,6 @@ tap.test('strictContentLength: false', (t) => { } }) }, (err, data) => { - assertEmitWarningCalledAndReset() t.error(err) }) })