Skip to content

Commit

Permalink
Remove unnecessary mismatched trailers check (nodejs#1419)
Browse files Browse the repository at this point in the history
  • Loading branch information
pimterry authored and crysmags committed Feb 27, 2024
1 parent b5b97dc commit 28809e1
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 56 deletions.
1 change: 0 additions & 1 deletion docs/api/Errors.md
Expand Up @@ -19,7 +19,6 @@ import { errors } from 'undici'
| `RequestContentLengthMismatchError` | `UND_ERR_REQ_CONTENT_LENGTH_MISMATCH` | request body does not match content-length header |
| `ResponseContentLengthMismatchError` | `UND_ERR_RES_CONTENT_LENGTH_MISMATCH` | response body does not match content-length header |
| `InformationalError` | `UND_ERR_INFO` | expected error with reason |
| `TrailerMismatchError` | `UND_ERR_TRAILER_MISMATCH` | trailers did not match specification |

### `SocketError`

Expand Down
24 changes: 1 addition & 23 deletions lib/client.js
Expand Up @@ -11,7 +11,6 @@ const RedirectHandler = require('./handler/redirect')
const {
RequestContentLengthMismatchError,
ResponseContentLengthMismatchError,
TrailerMismatchError,
InvalidArgumentError,
RequestAbortedError,
HeadersTimeoutError,
Expand Down Expand Up @@ -425,7 +424,6 @@ class Parser {

this.bytesRead = 0

this.trailer = ''
this.keepAlive = ''
this.contentLength = ''
}
Expand Down Expand Up @@ -615,8 +613,6 @@ 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 === 7 && key.toString().toLowerCase() === 'trailer') {
this.trailer += buf.toString()
} else if (key.length === 14 && key.toString().toLowerCase() === 'content-length') {
this.contentLength += buf.toString()
}
Expand Down Expand Up @@ -819,7 +815,7 @@ class Parser {
}

onMessageComplete () {
const { client, socket, statusCode, upgrade, trailer, headers, contentLength, bytesRead, shouldKeepAlive } = this
const { client, socket, statusCode, upgrade, headers, contentLength, bytesRead, shouldKeepAlive } = this

if (socket.destroyed && (!statusCode || shouldKeepAlive)) {
return -1
Expand All @@ -838,7 +834,6 @@ class Parser {
this.statusText = ''
this.bytesRead = 0
this.contentLength = ''
this.trailer = ''
this.keepAlive = ''

assert(this.headers.length % 2 === 0)
Expand All @@ -849,23 +844,6 @@ class Parser {
return
}

const trailers = trailer ? trailer.split(/,\s*/) : []
for (let i = 0; i < trailers.length; i++) {
const trailer = trailers[i]
let found = false
for (let n = 0; n < headers.length; n += 2) {
const key = headers[n]
if (key.length === trailer.length && key.toString().toLowerCase() === trailer.toLowerCase()) {
found = true
break
}
}
if (!found) {
util.destroy(socket, new TrailerMismatchError())
return -1
}
}

/* istanbul ignore next: should be handled by llhttp? */
if (request.method !== 'HEAD' && contentLength && bytesRead !== parseInt(contentLength, 10)) {
util.destroy(socket, new ResponseContentLengthMismatchError())
Expand Down
11 changes: 0 additions & 11 deletions lib/core/errors.js
Expand Up @@ -116,16 +116,6 @@ class ResponseContentLengthMismatchError extends UndiciError {
}
}

class TrailerMismatchError extends UndiciError {
constructor (message) {
super(message)
Error.captureStackTrace(this, TrailerMismatchError)
this.name = 'TrailerMismatchError'
this.message = message || 'Trailers does not match trailer header'
this.code = 'UND_ERR_TRAILER_MISMATCH'
}
}

class ClientDestroyedError extends UndiciError {
constructor (message) {
super(message)
Expand Down Expand Up @@ -196,7 +186,6 @@ module.exports = {
BodyTimeoutError,
RequestContentLengthMismatchError,
ConnectTimeoutError,
TrailerMismatchError,
InvalidArgumentError,
InvalidReturnValueError,
RequestAbortedError,
Expand Down
1 change: 0 additions & 1 deletion test/errors.js
Expand Up @@ -22,7 +22,6 @@ const scenarios = [
createScenario(errors.RequestAbortedError, 'Request aborted', 'AbortError', 'UND_ERR_ABORTED'),
createScenario(errors.InformationalError, 'Request information', 'InformationalError', 'UND_ERR_INFO'),
createScenario(errors.RequestContentLengthMismatchError, 'Request body length does not match content-length header', 'RequestContentLengthMismatchError', 'UND_ERR_REQ_CONTENT_LENGTH_MISMATCH'),
createScenario(errors.TrailerMismatchError, 'Trailers does not match trailer header', 'TrailerMismatchError', 'UND_ERR_TRAILER_MISMATCH'),
createScenario(errors.ClientDestroyedError, 'The client is destroyed', 'ClientDestroyedError', 'UND_ERR_DESTROYED'),
createScenario(errors.ClientClosedError, 'The client is closed', 'ClientClosedError', 'UND_ERR_CLOSED'),
createScenario(errors.SocketError, 'Socket error', 'SocketError', 'UND_ERR_SOCKET'),
Expand Down
34 changes: 14 additions & 20 deletions test/trailers.js
Expand Up @@ -3,36 +3,32 @@
const { test } = require('tap')
const { Client } = require('..')
const { createServer } = require('http')
const errors = require('../lib/core/errors')

test('response trailers missing', (t) => {
t.plan(2)
test('response trailers missing is OK', (t) => {
t.plan(1)

const server = createServer((req, res) => {
res.writeHead(200, {
Trailer: 'content-length'
})
res.end()
res.end('response')
})
t.teardown(server.close.bind(server))
server.listen(0, () => {
server.listen(0, async () => {
const client = new Client(`http://localhost:${server.address().port}`)
t.teardown(client.destroy.bind(client))

client.request({
const { body } = await client.request({
path: '/',
method: 'GET',
body: 'asd'
}, (err, data) => {
t.error(err)
data.body.on('error', (err) => {
t.type(err, errors.TrailerMismatchError)
})
})

t.equal(await body.text(), 'response')
})
})

test('response trailers missing w trailers', (t) => {
test('response trailers missing w trailers is OK', (t) => {
t.plan(2)

const server = createServer((req, res) => {
Expand All @@ -42,22 +38,20 @@ test('response trailers missing w trailers', (t) => {
res.addTrailers({
asd: 'foo'
})
res.end()
res.end('response')
})
t.teardown(server.close.bind(server))
server.listen(0, () => {
server.listen(0, async () => {
const client = new Client(`http://localhost:${server.address().port}`)
t.teardown(client.destroy.bind(client))

client.request({
const { body, trailers } = await client.request({
path: '/',
method: 'GET',
body: 'asd'
}, (err, data) => {
t.error(err)
data.body.on('error', (err) => {
t.type(err, errors.TrailerMismatchError)
})
})

t.equal(await body.text(), 'response')
t.same(trailers, { asd: 'foo' })
})
})

0 comments on commit 28809e1

Please sign in to comment.