Repository navigation
UnhandledPromiseRejection: Improper headers on http2 response #3830
Description
Are you sure this is an issue with the hapi core module or are you just looking for some help?
Issue with hapi core.
Is this a security related issue?
No.
What are you trying to achieve or the steps to reproduce?
Make a http2 request that triggers the following:
!(isInjection || request._core.started) ||
(request._isPayloadPending && !request.raw.req._readableState.ended)What was the result you received?
No response to request.
(node:8018) UnhandledPromiseRejectionWarning: TypeError [ERR_HTTP2_INVALID_CONNECTION_HEADERS]: HTTP/1 Connection specific headers are forbidden: "connection"
at mapToHeaders (internal/http2/util.js:453:16)
at ServerHttp2Stream.respond (internal/http2/core.js:2273:25)
at Http2ServerResponse.[begin-send] (internal/http2/compat.js:691:19)
at Http2ServerResponse.writeHead (internal/http2/compat.js:589:21)
at Object.internals.writeHead ([redacted]/server/node_modules/hapi/lib/transmit.js:340:13)
at Object.internals.transmit ([redacted]/server/node_modules/hapi/lib/transmit.js:105:15)
at Object.internals.fail ([redacted]/server/node_modules/hapi/lib/transmit.js:79:22)
at process._tickCallback (internal/process/next_tick.js:68:7)
What did you expect?
A response of some kind, if this error was handled, an error response.
Context
- node version: v10.11.0
- hapi version: 17.6.0
- os: Linux archlinux 4.18.11-1-ck-skylake package.json points to old github profile (hueverse -> walmartlabs) #1 SMP PREEMPT Sun Sep 30 06:40:33 EDT 2018 x86_64 GNU/Linux
- any other relevant information: ~
Fix
From MDN:
Also, Connection and Keep-Alive are ignored in HTTP/2; connection management is handled by other mechanisms there.
Replace:
Lines 96 to 101 in 7ffd301
With:
const isH2 = request.httpVersion === '2.0';
const isInjection = Shot.isInjection(request.raw.req);
if (!isH2 && (!(isInjection || request._core.started) ||
(request._isPayloadPending && !request.raw.req._readableState.ended))) {
response._header('connection', 'close');
}Afterthoughts
Should this even be allowed to happen? This would suggest that somewhere along the response chain a possible error is not accounted for. What should have really happened is an internal error response to the client and an error message to the console stating: TypeError [ERR_HTTP2_INVALID_CONNECTION_HEADERS]: HTTP/1 Connection specific headers are forbidden: "connection".
Activity
kanongil commentedon Oct 16, 2018
There are some bits of hapi internals that are not guarded against promise rejections. Mostly, this is not a problem due to the extensive test suite, but occasionally something like this can slip through.
Normally, hapi works similar to what you expect, however you have run into a corner case, where an already errorred transmission, causes another error in the attempt to send the error response.
kanongil commentedon Oct 16, 2018
The fact that adding a
connection: closeheader to the response, causes it to throw an error, seems like a bug in the nodejs http2 Compatibility API.FallingSnow commentedon Oct 16, 2018
Is this an issue or is this just accepted behaviour?
I agree, I think the fact that adding a connection header causes the response (I assume) to fail breaks the compatibility API. I assume there is a reason they fail rather than just ignoring it though.
Either way I suppose this is something that should be brought upstream but for the time being I feel the changes I purposed should be implemented, hopefully as a short term patch. If the node team does decide that this is intended behaviour then it just becomes a permanent patch.
However before I open an issue upstream, the compatibility API states:
Do you know if the connection header falls under internal methods or state?
Fixes hapijs#3830
kanongil commentedon Oct 17, 2018
Definitely not. It's just an argument for the very public
response.setHeader()API method.kanongil commentedon Nov 12, 2018
It seems that a node core fix is on its way for your actual issue: nodejs/node#23908.
Even when fixed, this still leaves hapi with potential
UnhandledPromiseRejectionerrors if something throws while transmitting a "transmission error" error response.hueniverse commentedon Sep 13, 2019
Replaced by #3974