From 28809e104f58324596a134a0b1a53a3649d9bc17 Mon Sep 17 00:00:00 2001 From: Tim Perry <1526883+pimterry@users.noreply.github.com> Date: Fri, 6 May 2022 16:06:37 +0200 Subject: [PATCH] Remove unnecessary mismatched trailers check (#1419) --- docs/api/Errors.md | 1 - lib/client.js | 24 +----------------------- lib/core/errors.js | 11 ----------- test/errors.js | 1 - test/trailers.js | 34 ++++++++++++++-------------------- 5 files changed, 15 insertions(+), 56 deletions(-) diff --git a/docs/api/Errors.md b/docs/api/Errors.md index 406fd5c40fc..6287ddcfd0b 100644 --- a/docs/api/Errors.md +++ b/docs/api/Errors.md @@ -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` diff --git a/lib/client.js b/lib/client.js index d3d4cfc705d..1e23e7aad34 100644 --- a/lib/client.js +++ b/lib/client.js @@ -11,7 +11,6 @@ const RedirectHandler = require('./handler/redirect') const { RequestContentLengthMismatchError, ResponseContentLengthMismatchError, - TrailerMismatchError, InvalidArgumentError, RequestAbortedError, HeadersTimeoutError, @@ -425,7 +424,6 @@ class Parser { this.bytesRead = 0 - this.trailer = '' this.keepAlive = '' this.contentLength = '' } @@ -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() } @@ -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 @@ -838,7 +834,6 @@ class Parser { this.statusText = '' this.bytesRead = 0 this.contentLength = '' - this.trailer = '' this.keepAlive = '' assert(this.headers.length % 2 === 0) @@ -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()) diff --git a/lib/core/errors.js b/lib/core/errors.js index a36107c5a98..f480f31a176 100644 --- a/lib/core/errors.js +++ b/lib/core/errors.js @@ -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) @@ -196,7 +186,6 @@ module.exports = { BodyTimeoutError, RequestContentLengthMismatchError, ConnectTimeoutError, - TrailerMismatchError, InvalidArgumentError, InvalidReturnValueError, RequestAbortedError, diff --git a/test/errors.js b/test/errors.js index bbcef80ab6b..b79d0d639d0 100644 --- a/test/errors.js +++ b/test/errors.js @@ -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'), diff --git a/test/trailers.js b/test/trailers.js index 357701f533e..ca56de23cc5 100644 --- a/test/trailers.js +++ b/test/trailers.js @@ -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) => { @@ -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' }) }) })