http2: ALPN fallback to HTTP/1.1

[sebdeckers]

Fixes #122

Well then, this was a lot easier than I thought it would be. 😌

• Deprecate NPN since this is not part of the HTTP/2 standard (and poorly documented AFAICT).
• Announce ALPN as h2 or http/1.1, as per IANA-registered protocol IDs.
• For backwards compatibility pass the non-HTTP/2 socket to the HTTP/1.1 parser. This then emits request events on the Http2SecureServer instance. Analogous to the https module.

Note 1: Some other tests are failing. Not sure if related to this patch. TIL --expose-internals

Note 2: Only doing fallback to https clients. Not for plaintext http since ALPN is inherently TLS-only. Should use the magic number to sniff plaintext http.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

http2

[sebdeckers]

@mcollina The only problem I am seeing in this is "how do a user know if the current request is HTTP1.1 or HTTP2"?

message.httpVersion

request.httpVersion; // "2.0" or "1.1" or "1.0" etc.

[mcollina]

@sebdeckers can you add it to the test? also with an http2 request, so that we show the difference.

[mcollina]

@mikeal you had some good opinions on the design of this step of the API. What do you think?

[sebdeckers]

@mcollina I've created an option called allowHTTP1. It can be passed to Http2SecureServer. The default value is true. Setting to false prevents the ALPN HTTP/1.1 fallback. This should be documented, perhaps when #102 lands.

Tests cover every combination of https/http2 clients against servers with/without ALPN fallback. Also explicitly reporting the httpVersion from server to the client.

[jasnell]

fyi... needs a rebase :-)

[sebdeckers]

@jasnell Nice, only took me a few minutes this time. Starting to get the hang of git cherry-pick. 😎

[sebdeckers]

Nits and code issues aside, this implementation hinges on http._connectionListener.

Con

• It has an underscore prefix and is not documented. This means it is not a public interface?
• node-spdy and node-http2 extend https.Server. I don't know if we want to make Http2SecureServer extend https.Server?

Pro

• require('https') imports http._connectionListener which imports it from require('_http_server'). This may imply that http._connectionListener is a public interface? At least for core modules?
• Core modules could import _connectionListener directly from require('_http_server'). Then it stays private, but somewhat couples http2 and http modules.

See also: nodejs/node#2017 (comment)

[mcollina]

It has an underscore prefix and is not documented. This means it is not a public interface?

I think overall this uses less of the internals compared to node-spdy and node-http2, so ok for me.

node-spdy and node-http2 extend https.Server. I don't know if we want to make Http2SecureServer extend https.Server?

IMHO that causes most of the issues in those modules. This is a clean implementation so no.

require('https') imports http._connectionListener which imports it from require('_http_server'). This may imply that http._connectionListener is a public interface? At least for core modules?

I think we can just import from _http_server.js, the function can still be underscored. How about we add a comment there?

Core modules could import _connectionListener directly from require('_http_server'). Then it stays private, but somewhat couples http2 and http modules.

Coupling is needed if we want to support both http1 and http2.

[sebdeckers]

(Rebased again)

[mcollina]

Landed as 6232681

[mcollina]

My mistake, I force-pushed 5cc2b2a.