Skip to content
This repository has been archived by the owner on Jul 31, 2018. It is now read-only.

proposal: Implement HTTP/2 in Node.js core #38

Closed
wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Aug 11, 2016

@nodejs/collaborators @nodejs/http : This is the beginnings of a proposal to implement HTTP/2 in Node.js core. There are still many details that need to be worked out, but this outlines the basic direction.

@Fishrock123
Copy link

I'd be for this if the API could be worked out before it lands in core... Should probably investigate User modules heavily beforehand.

@jasnell
Copy link
Member Author

jasnell commented Aug 11, 2016

Yes, already in the process of doing so. This will be a long process with several iterations likely before anything lands in core... and even then the impl/API will need to land as experimental for a bit before becoming officially supported.

@jasnell
Copy link
Member Author

jasnell commented Aug 11, 2016

given the amount of work that's going to be involved, I'd really like to know if anyone is in the "oy, we really shouldn't do this" phase ... I'd rather get the Yes/No decision out of the way before tackling the How.

@yorkie
Copy link

yorkie commented Aug 11, 2016

@jasnell Not familiar with http/2 too much, but should we upgrade our http_parser to support the new syntax (if this exists) in version 2?

/cc @indutny

@jasnell
Copy link
Member Author

jasnell commented Aug 11, 2016

Upgrading the http_parser would be way too much work and we'd simply end up duplicating what nghttp2 has already done. HTTP/2 switches to a binary framing protocol that completely moves away from the text and line-delimited format assumed by the http-parser. It carries a whole different set of semantics.

5. The HTTP/2 specification allows for push streams (server initiated streams
with an *assumed* HTTP request). Both the low-level and high-level HTTP
APIs will support the use of push streams on both the client and server
side.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this could be reflected in the proposed API, that would be good.

@ronkorving
Copy link

I'm very curious about the performance implications of using nghttp2 over a pure JS implementation like node-spdy. I would argue it would be good if this were measured before a decision is taken.

@jasnell
Copy link
Member Author

jasnell commented Aug 12, 2016

@ronkorving ... yep, once the nghttp2 implementation is a bit further along I will be putting together a set of benchmark and feature comparisons with existing ecosystem implementations like node-spdy. If you have ideas / recommendations / suggestions along these lines please definitely let me know! :-)

@indutny
Copy link
Member

indutny commented Aug 12, 2016

Good job, @jasnell ! Can we base it off #32 ?

@jasnell
Copy link
Member Author

jasnell commented Aug 12, 2016

Probably. I need to dig into that more.

@YurySolovyov
Copy link

What would be expected setup if people would want to make upgrading to http2 transparent for their users? Listen on http1 and upgrade/redirect to http2 if some criteria is met?

@jasnell
Copy link
Member Author

jasnell commented Aug 12, 2016

The http/2 spec outlines a negotiation scheme for both HTTPS and HTTP
connections. Initially the impl that I am preposing here would not fully
implement that scheme for sake of simplicity but it's something I want to
work towards.

On Friday, August 12, 2016, Yury [email protected] wrote:

What would be expected setup if people would want to make upgrading to
http2 transparent for their users? Listen on http1 and upgrade/redirect to
http2 if some criteria is met?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#38 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAa2eeySsitK9U__P4QR0lq4jLcK7JfQks5qfKqegaJpZM4JiY2a
.

@Qard
Copy link
Member

Qard commented Aug 12, 2016

I feel like the usefulness of HTTP/2 drops quite a bit without the upgrade path there.

It might be wise to base it on uv_link_t, as @indutny suggests, then work upgrading into uv_http_t to a theoretical uv_http2_t. Then we could pull out all the old http_parser stuff and switch to the link pipeline.

@jasnell
Copy link
Member Author

jasnell commented Aug 12, 2016

I definitely don't disagree, and what I suspect is that we won't be able to move the http/2 implementation out of Experimental status to Supported status without having the upgrade mechanism included. That said, just getting the basic implementation working and making sure the API is solid will need to take initial priority. There's also the concern that this work needs to not interfere with the existing operation of the http module until it is fully baked. In other words, yes we can do the upgrade path, but it's going to take some time to get there.

@jasnell
Copy link
Member Author

jasnell commented Aug 12, 2016

oh, and btw, if anyone is interested in jumping in and collaborating on this, we could probably create a new repo that forks off master and move forward that way.

@yosuke-furukawa
Copy link
Member

I am so excited about this proposal.
I have 2 questions.

  1. according to the proposal, we will support push stream. but i don't understand how to get the pushed data from http2 client. will we get pushed data like the node-http2 way? https://github.com/molnarg/node-http2/blob/427e60c0255685df41090fbcf104e706b5ae08f2/example/client.js#L31
  2. we would be better to use ALPN if we will support negotiation either HTTP/2 or HTTP/1 on TLS mode. http2 module is not same to https module but, that will be close interface. so I think we can support negotiation layer, and fallback to http1.

@jasnell
Copy link
Member Author

jasnell commented Aug 12, 2016

@yosuke-furukawa,

  1. That's roughly what I had in mind yes. I still have to dig into the details a bit more tho.
  2. I agree, the upgrade path for TLS is significantly easier than for a plain text connection. Once the TLS connection is established, it would be straight forward for us to defer to either the http/1 or http/2 implementations.

@doug-wade
Copy link

doug-wade commented Aug 12, 2016

This is super awesome.

if anyone is interested in jumping in and collaborating on this

Count me in

@jokeyrhyme
Copy link

Nice work. I think there will definitely be use cases where we explicitly want either HTTP/1.x or HTTP/2.0: e.g. testing, modern services, etc. Even in a service that has to support both protocols, it will make sense to be able to attach different event handlers, in order to take advantage of new capabilities in HTTP/2.0.

I do think a very common case for services will involve auto-negotiation over the same port. This can probably be achieved in userland initially, but it would be terrific to follow up this initial support with an example library, at least.

@mikeal
Copy link

mikeal commented Aug 16, 2016

One question, why is the server completely separate from the regular http server given the protocol is "upgraded."

Basically, I'm wondering why we can't do something similar to what we do for WebSockets.

https://nodejs.org/api/http.html#http_event_upgrade

What is keeping us from having an "http2" event that hands over a new session object?

This proposal doesn't include a client, is this because we are thinking we'll just implement that later or are we expecting the ecosystem to handle that via the new bindings for decoding?

@mikeal
Copy link

mikeal commented Aug 16, 2016

wow... I just realized this isn't a breaking change and is purely a feature addition, we could land this in a minor release :)

@jasnell
Copy link
Member Author

jasnell commented Aug 16, 2016

@mikeal ... exactly, doing it this way we'd be able to land this as a semver-minor and iterate without risking any breakage to the existing http module. Once we're sure it's working and we have the API down, we can begin to modify things so that upgrade works between the two.

This would include a client but I'm still working through all the details on that.

@mikeal
Copy link

mikeal commented Aug 16, 2016

@jasnell hrm... I still think that we might want to go the upgrade right off.

  1. We already have a very similar feature we can stick this next to (WebSockets).
  2. We'd have to copy/paste most of the http.js code base in order to get the http2 server API up. Then we'll have to throw that away for the "real" implementation, which makes me wonder what the value of doing it separately was being that any inferred stability from the old implementation may not apply.

If we're this freaked out about touching http.js maybe we should just implement bindings('http2') as a starting point. This would make it easy for people to experiment with getting it working on top of http.js and as a standalone server, not to mention experiments with a client.

@mcollina
Copy link
Member

I'm on using "http2", rather than touching http.js. For two reasons:

  1. the two versions of http have only the semantics in common. And possibly the API. One is textual, and one is binary. And it support multiplexed content.
  2. Adding it to http.js means that we might introduce a lot of (unwanted) regressions and instability without a way to turn them off.
  3. node-spdy hijacks on top of http.js, and the code is really very hard to read and maintain.

I'm really 👍 on this @jasnell. Count me in for anything on this, we should get the ball rolling.

@mikeal
Copy link

mikeal commented Aug 16, 2016

@mcollina the exact same comments could be made about WebSockets. Completely different semantics and parsing. My understanding of why node-spdy is so hacky has to do with not being in the core codebase.

I don't see how people are going to be able to actually use this as a separate library being that you will still need a lot of fallbacks for http and you still need a full HTTP/1 implementation to get setup.

That said, I'd be happy with just process.bindings('http2') and an example of how to hook it up inside of an http handler :)

@jasnell
Copy link
Member Author

jasnell commented Aug 16, 2016

I'll have a repo set up to work on this by the end of the week; and I'll create a nodejs/http2 team. The repo will be initialized off of master. I do have a basically working prototype based on nghttp2 already but I'll hold off just pushing that in so we can start from a clean slate and make sure things are done right from the beginning ;-)

@silverwind
Copy link

A http2 module might create the issue on how to discern HTTP with and without TLS. Wouldn't it lead to a http2s module, if we want to keep the naming scheme?

I think a options object on listen would be a wiser choice. I'm pretty sure it could be done in a non-breaking way and it would also allow us to eventually deprecate https.

@mikeal
Copy link

mikeal commented Aug 16, 2016

Rethinking this a bit, we wouldn't want it to be an event but instead a feature of the HTTP server response object.

http.createServer((req, res) => {
  if (req.headers.upgrade === 'h2c') {
    var session = res.http2CreateSession()
    return
  }
})

@mikeal
Copy link

mikeal commented Aug 16, 2016

@silverwind actually, it's the client's responsibility to signal it supports HTTP2 and the server is free to respond with a regular HTTP/1 response even if it does support HTTP2.

If we were to implement it as I described above, on the response object, it would be the response objects job to figure out if it is over TLS or not setup the connection via the appropriate method.

@jasnell
Copy link
Member Author

jasnell commented Aug 16, 2016

Given that there's already an http2 module on npm, we'll definitely have to get creative with how we introduce this.

Keep in mind that there are two paths for negotiating an HTTP/2 session... one for TLS and one for plain text... and given the differences in the implementation internals, it's going to be significantly easier to do this if we focus on the basic impl and standalone impl first, then find ways of bringing them together for the upgrade path.

That said, here's what I'm currently thinking as far as bootstrap is concerned...

// http/1 only server
const httpServer = http.createServer((req, res) => {});

// http/2 only server, TLS or plaintext determined by options
const http2Server = http.createHttp2Server(options, (req, res) => {});

// http/1 to http/2 upgrade
httpServer.on('upgrade', (response, socket, head) => {
  // strawman API here .... from here, the http2Server instance takes over the socket.
  // make sure upgrade is to h2c ...
  http2Server.handoff(response, socket, head);
});

// http/1 over TLS to http/2 over TLS upgrade using ALPN
const options = {
  ALPN: {
    h2: http2Server
  }
};
const httpsServer = https.createServer(options, (req, res) => {});

This would allow us to maintain separation between the http/1 and http/2 implementations, preserve the existing API model for upgrades, and support both upgrade models specified by http/2.

@jasnell
Copy link
Member Author

jasnell commented Aug 16, 2016

Went ahead and created the repo and initialized it from master... https://github.com/nodejs/http2 ... will begin getting the first PRs open by end of week

@ronkorving
Copy link

FWIW, I've never seen plain text negotiation work. It seems no browser supports it, but would be thrilled if proven wrong.

@Jessidhia
Copy link

Jessidhia commented Aug 19, 2016

@ronkorving at least Mozilla and Google were strictly against supporting HTTP/2 in the browser without TLS.

https://daniel.haxx.se/blog/2015/03/06/tls-in-http2/

Now, there are actually use cases for plain text HTTP/2, as daniel mentions later in the post, but no consumer browser indicates any intention to support it over plain text.

EDIT: or do you mean upgrading plain text HTTP/1 to TLS HTTP/2? Would this not first require an upgrade from plain text to TLS, and then the TLS handshake could use ALPN to ask for h2?

@ronkorving
Copy link

@Kovensky No, you are right. You're confirming my statement. Given that, I'm just not sure what would be the point of having a plain text upgrade path implemented at all in Node.

@benjamingr
Copy link
Member

I can definitely see why I would use http 2 over plaintext - http is used for a lot more than browsers and services often communicate using it and sometimes it's fine for them to pass plaintext.

That said - the overhead of using tls is tiny anyway.

@ronkorving
Copy link

Fair enough. If there is an official way to do it, and browsers choose to ignore it, that should not mean by default that we should ignore it too.

@jokeyrhyme
Copy link

In a production deployment of a web service, it is common for HTTPS to be terminated at the load balancer, and for requests from the load balancer to the worker to be plain-text.

Ideally (?), you would want every worker to be able to provision an internally-signed certificate, so that load balancer requests can also be performed over HTTPS. But this is configuration is impractical for many, and might not even have the desired safety advantages. Virtual private networking likely has more of a security impact here than internal certificate verification.

So at least within the context of machine-to-machine communication, support for plain-text HTTP/2.0 will be necessary for the time being.

@bnoordhuis
Copy link
Member

Sounds reasonable. As data points, both httpd and nginx support plaintext HTTP/2.

@louiscryan
Copy link

Random comment from the peanut gallery but wouldn't using nghttp2 facilitate the implementation of

https://fetch.spec.whatwg.org/

and other high-quality HTTP client interfaces?

@bitinn
@tbetbetbe
@domenic

@mikeal
Copy link

mikeal commented Sep 8, 2016

@louiscryan you can already use fetch via npm, https://www.npmjs.com/package/whatwg-fetch

nghttp2 is just the underlying protocol, not a high level api.

@louiscryan
Copy link

cc @murgatroid99

@mikeal My interest is in the context of the full-duplex streaming capabilities & of the Fetch API which is now an accepted feature

whatwg/fetch#229

Full-duplex is not possible with HTTP/1.1 but is a standard feature of HTTP2 so nghttp2 ends up being a dependency here.

I also care about this

whatwg/fetch#34

@oprearocks
Copy link

Saw that you are planning to export HTTP2 on the http module. Would it make sense to use separate export files for each module and use require('http/2') instead of require('http').HTTP2.

@joshgav
Copy link

joshgav commented Mar 20, 2017

Since work is now in progress at https://github.com/nodejs/http2, should we now merge this PR? AFAIK that doesn't mean all is set in stone, but that we agree on the initial approach.

@jasnell - any idea on when this would first be included in a core release, presumably as experimental? Thanks!

@trevnorris
Copy link
Contributor

@jasnell everything here looks good. If you want go ahead and change the XXX to a number, add it to 000-index.md and merge.

@Trott
Copy link
Member

Trott commented Jun 12, 2018

http2 is a thing. 🎉 Closing.

@Trott Trott closed this Jun 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.