{"action":"created","issue":{"url":"https://api.github.com/repos/hyperium/hyper/issues/448","labels_url":"https://api.github.com/repos/hyperium/hyper/issues/448/labels{/name}","comments_url":"https://api.github.com/repos/hyperium/hyper/issues/448/comments","events_url":"https://api.github.com/repos/hyperium/hyper/issues/448/events","html_url":"https://github.com/hyperium/hyper/pull/448","id":67808953,"number":448,"title":"WIP: Initial support for HTTP/2 based on solicit","user":{"login":"mlalic","id":229614,"avatar_url":"https://avatars.githubusercontent.com/u/229614?v=3","gravatar_id":"","url":"https://api.github.com/users/mlalic","html_url":"https://github.com/mlalic","followers_url":"https://api.github.com/users/mlalic/followers","following_url":"https://api.github.com/users/mlalic/following{/other_user}","gists_url":"https://api.github.com/users/mlalic/gists{/gist_id}","starred_url":"https://api.github.com/users/mlalic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mlalic/subscriptions","organizations_url":"https://api.github.com/users/mlalic/orgs","repos_url":"https://api.github.com/users/mlalic/repos","events_url":"https://api.github.com/users/mlalic/events{/privacy}","received_events_url":"https://api.github.com/users/mlalic/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2015-04-11T17:17:24Z","updated_at":"2015-04-15T09:29:48Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/hyperium/hyper/pulls/448","html_url":"https://github.com/hyperium/hyper/pull/448","diff_url":"https://github.com/hyperium/hyper/pull/448.diff","patch_url":"https://github.com/hyperium/hyper/pull/448.patch"},"body":"**PR NOT YET READY TO BE MERGED - WIP**\r\n\r\n# Summary (TL;DR)\r\n\r\nThis PR adds initial support for performing HTTP/2 requests. The changes to the public API of `hyper` are minimal: the only actual change is that now clients need to explicitly annotate a variable with the `Client` type when creating it, i.e.:\r\n```rust\r\nlet client: Client = Client::new()\r\n```\r\nThis is because Rust doesn't seem to be able to infer the default type parameter without the type annotation, although it's not explicitly given there either.\r\n\r\nThere is an [example](https://github.com/mlalic/hyper/blob/4ad58bc9753e640d81d78ffc757a4193cee98ee7/examples/client_http2.rs) that you can use to try out the HTTP/2 requests (analogous to the existing client example):\r\n```\r\nmlalic@skaro:~/src/hyper$ cargo run --example client_http2 -- http://http2bin.org/get?hi=hi\r\n     Running `target/debug/examples/client_http2 http://http2bin.org/get?hi=hi`\r\nResponse: 200 OK\r\nHeaders:\r\naccess-control-allow-origin: *\r\nx-clacks-overhead: GNU Terry Pratchett\r\ndate: Sat, 11 Apr 2015 16:16:21 GMT\r\nserver: h2o/1.1.1\r\ncontent-type: application/json\r\naccess-control-allow-credentials: true\r\n\r\n{\r\n  \"args\": {\r\n    \"hi\": \"hi\"\r\n  }, \r\n  \"headers\": {\r\n    \"Connection\": \"keep-alive\", \r\n    \"Host\": \"http2bin.org\", \r\n    \"Via\": \"2 http2bin.org\"\r\n  }, \r\n  \"origin\": \"88.217.180.94\", \r\n  \"url\": \"http://http2bin.org/get?hi=hi\"\r\n}\r\n```\r\n\r\nNote the `Via` header indicating that HTTP/2 was used (try it with the normal client and note it will be `1.1 http2bin.org`.\r\n\r\nAlso note the minimal difference in the implementation of the two example clients.\r\n\r\n# Details\r\n\r\nThe PR contains two distinct commit series: a refactoring of some of `hyper`'s components to make plugging in the HTTP/2 adapter possible and the actual implementation of the HTTP/2 support, based on wrapping [`solicit`'s](https://github.com/mlalic/solicit/) API.\r\n\r\n## Refactoring\r\n\r\nIn the refactoring I tried to minimize the number of changes that are made to both the public API of `hyper`, as well as to the internal components. I believe that those goals are definitely achieved.\r\n\r\nThe changes are based on extracting the API of the `Request<Fresh>`, `Request<Streaming>`, and `Response` structs into corresponding traits: `FreshHttpRequest`, `StreamingHttpRequest`, and `HttpResponse`, respectively. The existing types then, of course, were made to implement the corresponding trait.\r\n\r\nNext, an `HttpRequestFactory` trait is added that requires its implementors to provide `FreshHttpRequest` instances when the `get_fresh_request` method is called. The parameter of the method is a new struct that wraps up all the components that are necessary to create a new request (what is already being used in the `RequestBuilder::start` method).\r\n\r\nAn `Http11RequestFactory` struct is added that implements this trait and creates requests based on the existing `Request` type -- exactly as the `RequestBuilder` itself used to do.\r\n\r\nThe `Client` and `RequestBuilder` were then refactored to depend on the factory, rather than create their own requests, as well as to use only methods that the request and response *traits* expose, instead of accessing the fields directly.\r\n\r\nThe result of this was that the `Client` and `RequestBuilder` became decoupled from the particular implementation of requests and responses.\r\n\r\nAt that point, it was a simple matter of making the `Client` be generic over all types that implement the `HttpRequestFactory` trait. I made the `Http11RequestFactory` the default type, though, in order to preserve the old API as much as possible -- clients need to explicitly opt-into using HTTP/2.\r\n\r\nI believe the refactoring steps should be easy enough to follow by stepping through the individual commits one by one, rather than looking at the whole PR at once.\r\n\r\n## HTTP/2 Adapter\r\n\r\nThe adapter that makes it possible to use the HTTP/2 implementation of `solicit` in `hyper` is implemented in the `hyper::http2` module. It is a fairly straight forward implementation of the traits outlined above, based on wrapping the `solicit::client::SimpleClient` struct.\r\n\r\nImportantly, for now I took some shortcuts in order to push this out, so HTTPS is not yet implemented and pretty much all errors are ignored...\r\n\r\nObviously, the implementation is limited by what is currently supported by `solicit`, so most notably request bodies for all requests are simply ignored. Adding this wouldn't be difficult at all, though.\r\n\r\n# Drawbacks\r\n\r\nThe design does have some drawbacks. The most important one, in my opinion, is that it doesn't really get to leverage all the features that should make HTTP/2 more performant. All requests (even to the same host) still necessitate establishing a new TCP connection (which is thrown away once the response is received). It also doesn't lend itself to any sort of parallelism (i.e. issuing a number of concurrent requests to the same host over the same connection) nor handling server push events, both of which are big selling points of HTTP/2.\r\n\r\nAny of this would require more drastic changes to `hyper`'s APIs, which is something I wanted to avoid here. Especially since, from what I understand, `hyper` is exploring changes to its IO model, which would definitely introduce its own changes to the API.\r\n\r\n# Conclusion\r\n\r\nSo, as outlined in the beginning, the PR shows how it's possible to seamlessly leverage the lower-level HTTP/2 implementation of `solicit` and have `hyper`'s `Client` use HTTP/2.\r\n\r\nIf this design is something you'd like to pursue and further hash out, let me know and I can polish up the PR (proper docs, handle all errors properly, tests to guard against possible regressions, guidelines-compatible commit messages, etc. :))\r\n\r\nAny other comments are also greatly appreciated."},"comment":{"url":"https://api.github.com/repos/hyperium/hyper/issues/comments/93281424","html_url":"https://github.com/hyperium/hyper/pull/448#issuecomment-93281424","issue_url":"https://api.github.com/repos/hyperium/hyper/issues/448","id":93281424,"user":{"login":"GitCop","id":7943652,"avatar_url":"https://avatars.githubusercontent.com/u/7943652?v=3","gravatar_id":"","url":"https://api.github.com/users/GitCop","html_url":"https://github.com/GitCop","followers_url":"https://api.github.com/users/GitCop/followers","following_url":"https://api.github.com/users/GitCop/following{/other_user}","gists_url":"https://api.github.com/users/GitCop/gists{/gist_id}","starred_url":"https://api.github.com/users/GitCop/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/GitCop/subscriptions","organizations_url":"https://api.github.com/users/GitCop/orgs","repos_url":"https://api.github.com/users/GitCop/repos","events_url":"https://api.github.com/users/GitCop/events{/privacy}","received_events_url":"https://api.github.com/users/GitCop/received_events","type":"User","site_admin":false},"created_at":"2015-04-15T09:29:48Z","updated_at":"2015-04-15T09:29:48Z","body":"Thanks for contributing! Unfortunately, I'm here to tell you there were the following style issues with your Pull Request:\n\n * Commit: 3ce492a13535b223636212eb59c7f08995b0a791\n  * Commits must be in the following format: **%{type}(%{scope}): %{description}**\n\nGuidelines are available at https://github.com/hyperium/hyper/blob/master/CONTRIBUTING.md\n\n---\nThis message was auto-generated by https://gitcop.com\n"}}