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

Use ALPN by default to negotiate for HTTP2 #95

Merged
merged 2 commits into from
Dec 15, 2019

Conversation

CryZe
Copy link
Contributor

@CryZe CryZe commented Dec 8, 2019

Hyper supports HTTP2, so hyper-rustls should use ALPN to negotiate for HTTP2 by default.

Resolves #94

Hyper supports HTTP2, so hyper-rustls should use ALPN to negotiate for
HTTP2 by default.
@CryZe
Copy link
Contributor Author

CryZe commented Dec 8, 2019

The commit builds, Azure Pipelines just timed out for no reason.

@alex
Copy link
Contributor

alex commented Dec 11, 2019

Ten seconds of grepping https://github.com/hyperium/hyper-tls and I don't see ALPN being set there either. Do we think that's a bug? Or am I bad at grep? I'd assume hyper-rustls would basically match hyper-tls on behavior.

@CryZe
Copy link
Contributor Author

CryZe commented Dec 11, 2019

hyper-tls uses native-tls and the PR for ALPN there has been rejected as not all bindings for the native APIs it uses have ALPN (it's basically blocked by Windows and macOS afaik).

sfackler/rust-native-tls#113

@alex
Copy link
Contributor

alex commented Dec 11, 2019

Oh noes :-( Do I understand correctly that, at the moment, hyper's HTTP/2 support is basically never used then?

@CryZe
Copy link
Contributor Author

CryZe commented Dec 11, 2019

Yeah for hyper-tls you have to use the force_http2 (or whatever it's called) option that basically just assumes the server can handle HTTP2. For hyper-rustls you have to manually use the From impl for the HttpsConnector to pass the ALPN settings (at least until this PR). However hyper-openssl does proper ALPN by default on all platforms (at the "price" of having to use OpenSSL on Windows and macOS as well).

@alex
Copy link
Contributor

alex commented Dec 11, 2019

Thanks, the ecosystem summary was incredibly valuable, I appreciate it

@ctz ctz merged commit 524f615 into rustls:master Dec 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Why is HTTP2 not the default?
3 participants