Skip to content

Commit

Permalink
fix: verify trailers (#433)
Browse files Browse the repository at this point in the history
Fixes: #432
  • Loading branch information
ronag committed Sep 24, 2020
1 parent f87fa32 commit 6a22361
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 11 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,7 @@ const { errors } = require('undici')
| `NotSupportedError` | `UND_ERR_NOT_SUPPORTED` | encountered unsupported functionality. |
| `ContentLengthMismatchError` | `UND_ERR_CONTENT_LENGTH_MISMATCH`| 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 |
## Specification Compliance
Expand Down
41 changes: 39 additions & 2 deletions lib/core/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const Request = require('./request')
const {
ContentLengthMismatchError,
SocketTimeoutError,
TrailerMismatchError,
InvalidArgumentError,
RequestAbortedError,
ClientDestroyedError,
Expand Down Expand Up @@ -494,11 +495,26 @@ class Parser extends HTTPParser {
return 2
}

let keepAlive
let trailers

for (let n = 0; n < this.headers.length; n += 2) {
const key = this.headers[n + 0]
const val = this.headers[n + 1]

if (!keepAlive && key.length === 10 && key.toLowerCase() === 'keep-alive') {
keepAlive = val
} else if (!trailers && key.length === 7 && key.toLowerCase() === 'trailer') {
trailers = val
}
}

const { headers } = this
this.headers = null
this.trailers = trailers ? trailers.toLowerCase().split(/,\s*/) : null

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

if (keepAliveTimeout != null) {
const timeout = Math.min(
Expand Down Expand Up @@ -547,7 +563,7 @@ class Parser extends HTTPParser {
}

[HTTPParser.kOnMessageComplete] () {
const { client, socket, statusCode, headers, upgrade, request } = this
const { client, socket, statusCode, headers, upgrade, request, trailers } = this

if (socket.destroyed) {
return
Expand All @@ -564,13 +580,34 @@ class Parser extends HTTPParser {
this.statusCode = null
this.headers = null
this.request = null
this.trailers = null

if (statusCode < 200) {
assert(!socket.isPaused())
return
}

try {
if (trailers) {
if (!headers) {
throw new TrailerMismatchError()
}

for (const trailer of trailers) {
let found = false
for (let n = 0; n < headers.length; n += 2) {
const key = headers[n + 0]
if (key.length === trailer.length && key.toLowerCase() === trailer.toLowerCase()) {
found = true
break
}
}
if (!found) {
throw new TrailerMismatchError()
}
}
}

request.onComplete(headers)
} catch (err) {
util.destroy(socket, err)
Expand Down
11 changes: 11 additions & 0 deletions lib/core/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,16 @@ class ContentLengthMismatchError extends UndiciError {
}
}

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

class ClientDestroyedError extends UndiciError {
constructor (message) {
super(message)
Expand Down Expand Up @@ -134,6 +144,7 @@ module.exports = {
HeadersTimeoutError,
RequestTimeoutError,
ContentLengthMismatchError,
TrailerMismatchError,
InvalidArgumentError,
InvalidReturnValueError,
RequestAbortedError,
Expand Down
11 changes: 3 additions & 8 deletions lib/core/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,9 @@ function destroy (stream, err) {
}

const KEEPALIVE_TIMEOUT_EXPR = /timeout=(\d+)s/
function parseKeepAliveTimeout (headers) {
for (let n = 0; n < headers.length; n += 2) {
const key = headers[n + 0]
if (key.length === 10 && key.toLowerCase() === 'keep-alive') {
const m = headers[n + 1].match(KEEPALIVE_TIMEOUT_EXPR)
return m ? parseInt(m[1]) * 1000 : null
}
}
function parseKeepAliveTimeout (val) {
const m = val.match(KEEPALIVE_TIMEOUT_EXPR)
return m ? parseInt(m[1]) * 1000 : null
}

function parseHeaders (headers) {
Expand Down
2 changes: 1 addition & 1 deletion test/client-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -565,8 +565,8 @@ test('trailers', (t) => {
path: '/',
method: 'GET'
}, () => new PassThrough(), (err, data) => {
t.strictDeepEqual({ 'content-md5': 'test' }, data.trailers)
t.error(err)
t.strictDeepEqual(data.trailers, { 'content-md5': 'test' })
})
})
})
Expand Down
63 changes: 63 additions & 0 deletions test/trailers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
'use strict'

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)

const server = createServer((req, res) => {
res.writeHead(200, {
Trailer: 'content-length'
})
res.end()
})
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: 'GET',
body: 'asd'
}, (err, data) => {
t.error(err)
data.body.on('error', (err) => {
t.ok(err instanceof errors.TrailerMismatchError)
})
})
})
})

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

const server = createServer((req, res) => {
res.writeHead(200, {
Trailer: 'content-length'
})
res.addTrailers({
asd: 'foo'
})
res.end()
})
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: 'GET',
body: 'asd'
}, (err, data) => {
t.error(err)
data.body.on('error', (err) => {
t.ok(err instanceof errors.TrailerMismatchError)
})
})
})
})

0 comments on commit 6a22361

Please sign in to comment.