Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP upgrade mechanism #4

Open
molnarg opened this issue Jul 10, 2013 · 11 comments
Open

HTTP upgrade mechanism #4

molnarg opened this issue Jul 10, 2013 · 11 comments

Comments

@molnarg
Copy link
Owner

molnarg commented Jul 10, 2013

With fallback to HTTP/1.1 if HTTP/2 is not supported.

@molnarg
Copy link
Owner Author

molnarg commented Aug 7, 2013

This is one of the last items on my TODO list so feel free to pick this up if you are interested :) It shouldn't be hard to implement given that most of the infrastructure is in place in http.js and node supports upgrades natively. (see the docs: client side, server side)

@stanier
Copy link

stanier commented Jun 3, 2015

Out of curiosity, is there any particular reason this is pushed back so far on the TODO list? I'd think this was essential, considering many developers (myself included) don't like to use HTTPS when developing/debugging an application locally.

@nwgh
Copy link
Collaborator

nwgh commented Jun 3, 2015

I can't speak for @molnarg , but from my perspective, unencrypted h2 shouldn't exist at all - HTTPS is the future of the web (even for HTTP/1.1). Given that, I am not going to personally spend any time trying to support it.

My own perspective aside, though, the reality of the situation is that only 1 of the 3 major browsers that have announced h2 support so far even supports (or plans to support) Upgrade - Internet Explorer. Therefore, if you want your site to be able to speak h2 to Chrome and/or Firefox for any purpose (testing OR live site), you're going to have to use HTTPS anyway. The tools exist to debug HTTPS connections, and are quite usable in Wireshark (which you'll want to use if you're debugging h2 traffic anyway, given h2's binary nature). Setting up an HTTPS-enabled development environment isn't significantly harder than setting up a plaintext HTTP environment, it really just takes a few more minutes.

@molnarg
Copy link
Owner Author

molnarg commented Jun 4, 2015

Agreed. If someone implements this in a clean way, I may merge it, but don't plan to put any effort into it.

@stanier
Copy link

stanier commented Jun 11, 2015

RFC 7540 Section 3.2 describes support for discovery of HTTP/2 and use of the Upgrade Mechanism from Section 6.7 of RFC 7230. While I understand that opinions may be that TLS is superior because use of encryption is best practice, which I agree with whole-heartedly, I feel that node.js' only reliable implementation of HTTP/2 should meet RFC 7450's specifications in full. Section 3.2 exists for a reason, as does the Upgrade Mechanism.

And as @todesschaf stated, while tools exist to debug the connection itself, many development/debugging tools such as LiveReload implementations do not support TLS.

I'll try my hand at implementing this as while my views digress, I still feel like this should be implemented.

@nwgh
Copy link
Collaborator

nwgh commented Jun 11, 2015

Not to get too pushy on this (patches are good, and I welcome them!), but the dev/debug tools that don't support TLS are, frankly, going to have to for h2, given that the 2 major developer browsers (Chrome & Firefox) will not be supporting plaintext h2. That sounds more like an argument for working on adding TLS support to those tools, instead of plaintext h2 support here :)

@nwgh nwgh added the p5 label Jan 11, 2016
@felixsanz
Copy link

@nwgh HAProxy + SSL termination needs this ^

@feychenie
Copy link

@nwgh, it's all about what @felixsanz said. Most entreprise-grade projects handle SSL at the proxy/LB level.

@devinivy
Copy link

For my purposes this is essential– for me this also has to do with SSL termination at the proxy/LB level. Chrome/FF aren't the only clients of HTTP/2 in practice (even when the end-client is a browser), and ultimately RFC 7540 doesn't call for mandatory TLS, though I know it was an area of contention.

Anybody familiar enough with the code base to point to the right areas to start chipping away at this?

@patoi
Copy link

patoi commented Sep 18, 2016

Terminating HTTP/2 TLS on nodejs server: 1400 req/s
Without TLS and HTTP/1.1 on nodejs server: 6900 req/s

Repository owner locked and limited conversation to collaborators Sep 19, 2016
@nwgh
Copy link
Collaborator

nwgh commented Sep 19, 2016

I've locked this because I'm not quite ready to wontfix it. As noted (though as a non-sequitur to the actual purpose of this bug) this isn't the most high-performance server. Given that node will soon(ish) have a native http2 implementation based on nghttp2, and that will almost certainly perform better, I'm not inclined to care so much about perf - especially given the design constraints when we first made this (it must be usable for testing firefox, which means it must be easy to modify this to behave incorrectly). I'll probably wontfix this once node has its native implementation.

In the meantime, there's no point in people continually asking me for this - I've noted above that I flat-out won't write the support myself. If someone wants to give me a patch, I'm still open to at least thinking about merging it, but I'm not going to support that effort with my time in any way other than code review (and possibly not even that).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants