From 6a22361e1d75de6262c8014a37d6910c4530b0ca Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Thu, 24 Sep 2020 08:56:34 +0200 Subject: [PATCH] fix: verify trailers (#433) Fixes: https://github.com/nodejs/undici/issues/432 --- README.md | 1 + lib/core/client.js | 41 ++++++++++++++++++++++++++-- lib/core/errors.js | 11 ++++++++ lib/core/util.js | 11 +++----- test/client-stream.js | 2 +- test/trailers.js | 63 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 118 insertions(+), 11 deletions(-) create mode 100644 test/trailers.js diff --git a/README.md b/README.md index 0399a88b426..f9a768b988e 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/lib/core/client.js b/lib/core/client.js index 8c7f7eb912f..22aac49feb5 100644 --- a/lib/core/client.js +++ b/lib/core/client.js @@ -11,6 +11,7 @@ const Request = require('./request') const { ContentLengthMismatchError, SocketTimeoutError, + TrailerMismatchError, InvalidArgumentError, RequestAbortedError, ClientDestroyedError, @@ -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( @@ -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 @@ -564,6 +580,7 @@ class Parser extends HTTPParser { this.statusCode = null this.headers = null this.request = null + this.trailers = null if (statusCode < 200) { assert(!socket.isPaused()) @@ -571,6 +588,26 @@ class Parser extends HTTPParser { } 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) diff --git a/lib/core/errors.js b/lib/core/errors.js index 721c5742954..c66a49a1d1d 100644 --- a/lib/core/errors.js +++ b/lib/core/errors.js @@ -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) @@ -134,6 +144,7 @@ module.exports = { HeadersTimeoutError, RequestTimeoutError, ContentLengthMismatchError, + TrailerMismatchError, InvalidArgumentError, InvalidReturnValueError, RequestAbortedError, diff --git a/lib/core/util.js b/lib/core/util.js index 1a4ed45ef79..bf3a00ab607 100644 --- a/lib/core/util.js +++ b/lib/core/util.js @@ -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) { diff --git a/test/client-stream.js b/test/client-stream.js index fcf66c651c6..d415405c75a 100644 --- a/test/client-stream.js +++ b/test/client-stream.js @@ -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' }) }) }) }) diff --git a/test/trailers.js b/test/trailers.js new file mode 100644 index 00000000000..e1a4714fbda --- /dev/null +++ b/test/trailers.js @@ -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) + }) + }) + }) +})