From 32cc207ac3afbd48c2c52f79805905fba116f70f Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Wed, 21 Oct 2020 11:28:35 +0200 Subject: [PATCH 1/6] Propagate "peek" and "finish" errors as error response (fixes hapijs/hapi#4172) --- lib/response.js | 24 +++++++++++++++++++----- test/payload.js | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/lib/response.js b/lib/response.js index 47408867e..bd2679956 100755 --- a/lib/response.js +++ b/lib/response.js @@ -766,13 +766,27 @@ internals.Response.Peek = class extends Stream.Transform { super(); this._podium = podium; - this.on('finish', () => podium.emit('finish')); } - _transform(chunk, encoding, callback) { + async _transform(chunk, encoding, callback) { - this._podium.emit('peek', [chunk, encoding]); - this.push(chunk, encoding); - callback(); + try { + await this._podium.emit('peek', [chunk, encoding]); + + callback(null, chunk); + } catch (err) { + callback(err); + } + } + + async _flush(callback) { + + try { + await this._podium.emit('finish'); + + callback(null); + } catch (err) { + callback(err); + } } }; diff --git a/test/payload.js b/test/payload.js index b776a44fa..ddf61a378 100755 --- a/test/payload.js +++ b/test/payload.js @@ -5,6 +5,7 @@ const Http = require('http'); const Path = require('path'); const Zlib = require('zlib'); +const Boom = require('@hapi/boom'); const Code = require('@hapi/code'); const Hapi = require('..'); const Hoek = require('@hapi/hoek'); @@ -290,6 +291,48 @@ describe('Payload', () => { expect(peeked).to.be.true(); }); + it('propagates peek errors', async () => { + + const ext = (request, h) => { + + request.events.once('peek', () => { + + throw Boom.badRequest(); + }); + + return h.continue; + }; + + const server = Hapi.server(); + server.ext('onRequest', ext); + server.route({ method: 'POST', path: '/', options: { handler: () => null, payload: { parse: false } } }); + + const payload = '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'; + const res = await server.inject({ method: 'POST', url: '/', payload }); + expect(res.statusCode).to.equal(400); + }); + + it('propagates finish errors', async () => { + + const ext = (request, h) => { + + request.events.once('finish', () => { + + throw Boom.badRequest(); + }); + + return h.continue; + }; + + const server = Hapi.server(); + server.ext('onRequest', ext); + server.route({ method: 'POST', path: '/', options: { handler: () => null, payload: { parse: false } } }); + + const payload = '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'; + const res = await server.inject({ method: 'POST', url: '/', payload }); + expect(res.statusCode).to.equal(400); + }); + it('handles gzipped payload', async () => { const message = { 'msg': 'This message is going to be gzipped.' }; From 7c121a89fba374b91e337a00c0fd97f5cc045e6b Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Wed, 21 Oct 2020 12:04:30 +0200 Subject: [PATCH 2/6] Linting --- lib/response.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/response.js b/lib/response.js index bd2679956..15f7bd8b1 100755 --- a/lib/response.js +++ b/lib/response.js @@ -774,7 +774,8 @@ internals.Response.Peek = class extends Stream.Transform { await this._podium.emit('peek', [chunk, encoding]); callback(null, chunk); - } catch (err) { + } + catch (err) { callback(err); } } @@ -785,7 +786,8 @@ internals.Response.Peek = class extends Stream.Transform { await this._podium.emit('finish'); callback(null); - } catch (err) { + } + catch (err) { callback(err); } } From 393bcc4d4ffd16f8a9366ca655937edfad16062b Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Wed, 21 Oct 2020 13:21:49 +0200 Subject: [PATCH 3/6] Using _final instead of _flush to emit finish --- lib/response.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/response.js b/lib/response.js index 15f7bd8b1..6c1ca92db 100755 --- a/lib/response.js +++ b/lib/response.js @@ -780,7 +780,7 @@ internals.Response.Peek = class extends Stream.Transform { } } - async _flush(callback) { + async _final(callback) { try { await this._podium.emit('finish'); From 790236ae923f26b9c2e407887f54897a4142aabe Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Wed, 21 Oct 2020 13:22:35 +0200 Subject: [PATCH 4/6] Ensure that Transform's callback are called only once --- lib/response.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/response.js b/lib/response.js index 6c1ca92db..7a2b2d3c5 100755 --- a/lib/response.js +++ b/lib/response.js @@ -772,23 +772,25 @@ internals.Response.Peek = class extends Stream.Transform { try { await this._podium.emit('peek', [chunk, encoding]); - - callback(null, chunk); } catch (err) { callback(err); + return; } + + callback(null, chunk); } async _final(callback) { try { await this._podium.emit('finish'); - - callback(null); } catch (err) { callback(err); + return; } + + callback(); } }; From 0e95258f377a846321a84e9df0d089c39162658f Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Wed, 21 Oct 2020 14:10:55 +0200 Subject: [PATCH 5/6] Push with encoding --- lib/response.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/response.js b/lib/response.js index 7a2b2d3c5..faf111719 100755 --- a/lib/response.js +++ b/lib/response.js @@ -778,7 +778,8 @@ internals.Response.Peek = class extends Stream.Transform { return; } - callback(null, chunk); + this.push(chunk, encoding); + callback(); } async _final(callback) { From 931395c718d76dd34fa50dfaa8b4a5f3f6392e30 Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Mon, 9 Nov 2020 12:04:46 +0100 Subject: [PATCH 6/6] optimization: avoid useless microtasks --- lib/response.js | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/response.js b/lib/response.js index faf111719..48715114b 100755 --- a/lib/response.js +++ b/lib/response.js @@ -770,12 +770,14 @@ internals.Response.Peek = class extends Stream.Transform { async _transform(chunk, encoding, callback) { - try { - await this._podium.emit('peek', [chunk, encoding]); - } - catch (err) { - callback(err); - return; + if (this._podium.hasListeners('peek')) { + try { + await this._podium.emit('peek', [chunk, encoding]); + } + catch (err) { + callback(err); + return; + } } this.push(chunk, encoding); @@ -784,12 +786,14 @@ internals.Response.Peek = class extends Stream.Transform { async _final(callback) { - try { - await this._podium.emit('finish'); - } - catch (err) { - callback(err); - return; + if (this._podium.hasListeners('finish')) { + try { + await this._podium.emit('finish'); + } + catch (err) { + callback(err); + return; + } } callback();