Support HTTP/2 (including Cleartext)

[alexeyr-ci]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

Add options to inject() to send HTTP/2 requests instead of HTTP/1.1. My proposal would be app.inject({ http2: true })... or app.inject({ httpVersion: 2 })... but automatic detection could work too.

Motivation

When I add http2: true to Fastify options, all requests made with inject() receive 505 status code. For secure servers, adding https: { allowHttp1: true } works, but

1. the user may not want to allow HTTP/1.1;
2. the server may behave differently depending on the protocol version and it's desired to test HTTP/2 in particular.

As a workaround for 1, you could have https: { allowHttp1: process.env.JEST_WORKER_ID !== undefined }, but I don't like server code knowing about tests like that.

And of course, if the server wants to use H2C, there's no allowHttp1 option.

Example

No response

[mcollina]

Very interesting. I don't there should be any different between HTTP/2 and HTTP/1.1 for this module, as there is no actual "data" flowing on the wire. I think there is just some internal API compatibility somewhere that we should handle.

[alexeyr-ci]

There is

light-my-request/lib/request.js

Lines 103 to 105 in b820106

	this.httpVersionMajor = 1
	this.httpVersionMinor = 1
	this.httpVersion = '1.1'

They don't seem to be used in light-my-request directly, but maybe something a Request is passed to uses them?

[alexeyr-ci]

I suspect https://github.com/fastify/fastify/blob/c86750e02bcad8651bac63b717c26a2d46c5c440/lib/route.js#L435. Should I post the issue in https://github.com/fastify/fastify then instead of here?

[alexeyr-ci]

On the other hand, if this is the issue, it's easy to fix by adding options as suggested.

[climba03003]

I suspect https://github.com/fastify/fastify/blob/c86750e02bcad8651bac63b717c26a2d46c5c440/lib/route.js#L435. Should I post the issue in https://github.com/fastify/fastify then instead of here?

It should be changed in here, the detection in fastify is correct.
We should provide Connection: close only if the request is not HTTP/2.

We might consider to provide option override the http version.

[alexeyr-ci]

Would it be OK to also release a 5.15.0 with this fix, so it can be used with Fastify 4 without additional resolutions? I see 5.14.0 was released after 6.0.0, for example.

[climba03003]

Would it be OK to also release a 5.15.0 with this fix, so it can be used with Fastify 4 without additional resolutions?

Yes, we can definitely back-port the change and release a new version on old release line.