-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Enable HTTP/2 & Push Promise #5856
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
Comments
@DamianEdwards Do we want HTTP/2 in WebListener? (just for comparison - we won't be doing it in Kestrel any time soon) cc @shirhatti |
Not a high priority for now. Backlog it until we get more customers asking for it. |
The HTTP_REQUEST_FLAG_HTTP2 flag was added to HTTP_REQUEST.Flags some time after Win10 RTM. |
Enabling HTTP/2 for WebListener would allow API Management to support HTTP/2 and we have internal and external customers asking for it. |
@darrelmiller HTTP/2 is already available (Win10), no changes were required, only PushPromise is missing. |
I just tried making a HTTP/2 request using HttpClient with WinHttpHandler and WebListener hosted using Damian's OwinWebListenerHost and it wasn't working. Maybe I'm just doing it wrong. |
A) SSL is required. B) it's hard to tell if you succeeded, nothing in the server data structures tells you. Your best bet is to use a browser and its debugger tools. |
Thanks. I was just being dense. It is making HTTP/2 requests now. Do I actually need to migrate to WebListener, or would HttpListener work automatically too? |
WebListener, HttpListener and IIS all inherit the same HTTP/2 functionality from Http.Sys. |
@darrelmiller OwinWebListenerHost is fork of the katana HttpListener impl. It may need some changes to support HTTP/2... |
@damianh It did work, but I actually didn't need to change to WebListener at all. HttpListener worked just fine with HTTP/2. |
Hi, I did an implementation of HTTP/2 with Server Push for HttpSysServer here and described it here. Please let me know if this could be a valuable pull request (if the design is in line with team approach and this is a right time). Of course I would first make a pull request with |
@tpeczek Looks like a good start. I think you duplicated existing code in several places (e.g. IndexOfKnownHeader). I don't think we'd merge it until we look at the whole end-to-end, especially for the Feature interface. aspnet/HttpAbstractions#371. We want to make sure to expose everything we need to and that it works well for both HttpSysServer and Kestrel. |
@Tratcher Yes the HTTP_REQUEST_HEADER_ID is a lazy copy of HTTP_RESPONSE_HEADER_ID while they should share a base class to avoid duplication. There are probably other places which can be improved as well (for example a private method reused by both I'll be happy to iterate over this to get this into acceptable state if/when team decides this is something to go with. |
We are definitely doing HTTP/2, just not any time soon. Too many of Kestrel's dependencies are blocked. You've got the basic outline we'll need, feel free to keep iterating and we'll pick it up when we're ready. |
@Tratcher I understand. I'll try to tweak some things in my spare time to make sure it is in decent shape if you ever decide to use it. |
Request.Protocol has been set to "HTTP/2" for 2.2. Push promise is not in scope for this release. |
Where is HTTP/2 Push in the roadmap? Should we use what tpeczek has done to date until official support released? |
Maybe for 3.0. |
Closing as we've decided not to support push. See #4249 (comment) for details. |
Edit: Proof of concept provided below https://github.com/aspnet/HttpSysServer/issues/106#issuecomment-291283012
How do we detect if the request is HTTP/2 before calling the Push method?
Also, should/can we expose things like request priority?
Fowler has asked us to set the HttpContext.Request.Protocol to "http/2" rather than it's current "http/1.1". HttpClient is doing something similar.
Response headers and body can be streamlined. HTTP/2 does not require us to manually chunk the response. All responses are treated like the old 1.0 connection:close mode.
The text was updated successfully, but these errors were encountered: