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

For 0.10.x: fix(buffer): fix maybe_reserve() not working after take_buf() #2288

Closed
wants to merge 1 commit into from

Conversation

strohel
Copy link

@strohel strohel commented Sep 24, 2020

This is a fix for the ancient pre-async 0.10.x branch (the bug is not present in 0.11.0+).
I was hesitant whether to submit a patch for such an old branch at all, but I'll let you decide whether you still support it.

The bug affects Rocket 0.4.*, whose ecosystem seems to be the last major user of hyper 0.10.x. Rocket 0.5 with up-to-date hyper dependency is in the works, but there are no indications it will be finished any time soon.

The problem demonstrates as Rocket's inability to read second request on
a keep-alive connection. I.e. this fixes a bug where keep-alive connections
don't work at all
in current Rocket 0.4.5. More details in the commit message.

...through the means of backporting and adapting a fix from 0.11 from
hyperium@d35992d#diff-078f367374debbc894f7cc1c4084e467R64-R66

Includes a test that fails when the code change is not applied.

The problem demostrates as Rocket's inability to read second request on
a keep-alive connection. I.e. this fixes a bug where keep-alive connections
don't work at all in current Rocket 0.4.5.

See a Rocket log excerpt, where the last 6 messages are the culprit:
```
 2020-09-21T16:06:43.885Z DEBUG hyper::server                                > Incoming stream
 2020-09-21T16:06:43.885Z TRACE hyper::buffer                                > get_buf []
 2020-09-21T16:06:43.885Z TRACE hyper::buffer                                > read_into_buf buf[0..4096]
 2020-09-21T16:06:43.885Z TRACE hyper::buffer                                > get_buf [u8; 4096][0..129]
 2020-09-21T16:06:43.885Z TRACE hyper::http::h1                              > try_parse([71, 69, 84, 32, 47, 100, 111, 99, 115, 47, 115, 119, 97, 103, 103, 101, 114, 45, 117, 105, 45, 99, 111, 110, 102, 105, 103, 46, 106, 115, 111, 110, 32, 72, 84, 84, 80, 47, 49, 46, 49, 13, 10, 72, 111, 115, 116, 58, 32, 49, 50, 55, 46, 48, 46, 48, 46, 49, 58, 56, 48, 56, 48, 13, 10, 85, 115, 101, 114, 45, 65, 103, 101, 110, 116, 58, 32, 99, 117, 114, 108, 47, 55, 46, 55, 50, 46, 48, 13, 10, 65, 99, 99, 101, 112, 116, 58, 32, 42, 47, 42, 13, 10, 67, 111, 110, 110, 101, 99, 116, 105, 111, 110, 58, 32, 107, 101, 101, 112, 45, 97, 108, 105, 118, 101, 13, 10, 13, 10])
 2020-09-21T16:06:43.885Z TRACE hyper::http::h1                              > Request.try_parse([Header; 100], [u8; 129])
 2020-09-21T16:06:43.885Z TRACE hyper::http::h1                              > Request.try_parse Complete(129)
 2020-09-21T16:06:43.885Z TRACE hyper::header                                > raw header: "Host"=[49, 50, 55, 46, 48, 46, 48, 46, 49, 58, 56, 48, 56, 48]
 2020-09-21T16:06:43.885Z TRACE hyper::header                                > raw header: "User-Agent"=[99, 117, 114, 108, 47, 55, 46, 55, 50, 46, 48]
 2020-09-21T16:06:43.885Z TRACE hyper::header                                > raw header: "Accept"=[42, 47, 42]
 2020-09-21T16:06:43.885Z TRACE hyper::header                                > raw header: "Connection"=[107, 101, 101, 112, 45, 97, 108, 105, 118, 101]
 2020-09-21T16:06:43.886Z DEBUG hyper::server::request                       > Request Line: Get AbsolutePath("/docs/swagger-ui-config.json") Http11
 2020-09-21T16:06:43.886Z DEBUG hyper::server::request                       > Headers { Host: 127.0.0.1:8080
, User-Agent: curl/7.72.0
, Accept: */*
, Connection: keep-alive
, }
 2020-09-21T16:06:43.886Z TRACE hyper::http                                  > should_keep_alive( Http11, Some(Connection([KeepAlive])) )
 2020-09-21T16:06:43.886Z TRACE _                                            > Data::new(EmptyReader)
 2020-09-21T16:06:43.886Z TRACE _                                            > Filled peek buf with 0 bytes.
 2020-09-21T16:06:43.886Z TRACE _                                            > Peek bytes: 0/512 bytes.
 2020-09-21T16:06:43.886Z INFO  rocket::rocket                               > GET /docs/swagger-ui-config.json:
 2020-09-21T16:06:43.886Z TRACE _                                            > Routing the request: GET /docs/swagger-ui-config.json
 2020-09-21T16:06:43.886Z TRACE _                                            > All matches: [Route { name: None, method: Get, base: Origin { source: None, path: "/docs", query: None, segment_count: [uninitialized storage] }, uri: Origin { source: None, path: "/docs/swagger-ui-config.json", query: None, segment_count: [uninitialized storage] }, rank: -4, format: None, metadata: Metadata { path_segments: [RouteSegment { string: "docs", kind: Static, name: "docs", index: Some(0), _part: PhantomData }, RouteSegment { string: "swagger-ui-config.json", kind: Static, name: "swagger-ui-config.json", index: Some(1), _part: PhantomData }], query_segments: None, fully_dynamic_query: true } }]
 2020-09-21T16:06:43.886Z INFO  _                                            > Matched: GET /docs/swagger-ui-config.json
 2020-09-21T16:06:43.886Z INFO  _                                            > Outcome: Success
 2020-09-21T16:06:43.886Z TRACE hyper::header                                > Headers.append_raw( "Content-Type", [97, 112, 112, 108, 105, 99, 97, 116, 105, 111, 110, 47, 106, 115, 111, 110] )
 2020-09-21T16:06:43.886Z TRACE hyper::header                                > Headers.append_raw( "Server", [82, 111, 99, 107, 101, 116] )
 2020-09-21T16:06:43.886Z TRACE hyper::header                                > Headers.set( "Content-Length", ContentLength(326) )
 2020-09-21T16:06:43.886Z DEBUG hyper::server::response                      > writing head: Http11 Ok
 2020-09-21T16:06:43.886Z TRACE hyper::header                                > Headers.set( "Date", Date(HttpDate(Tm { tm_sec: 43, tm_min: 6, tm_hour: 16, tm_mday: 21, tm_mon: 8, tm_year: 120, tm_wday: 1, tm_yday: 264, tm_isdst: 0, tm_utcoff: 0, tm_nsec: 886849073 })) )
 2020-09-21T16:06:43.886Z DEBUG hyper::server::response                      > headers [
Headers { Content-Type: application/json
, Server: Rocket
, Content-Length: 326
, Date: Mon, 21 Sep 2020 16:06:43 GMT
, }]
 2020-09-21T16:06:43.886Z DEBUG hyper::server::response                      > write 326 bytes
 2020-09-21T16:06:43.887Z TRACE hyper::server::response                      > ending
 2020-09-21T16:06:43.887Z INFO  _                                            > Response succeeded.
 2020-09-21T16:06:43.887Z TRACE hyper::http                                  > should_keep_alive( Http11, None )
 2020-09-21T16:06:43.887Z DEBUG hyper::server                                > keep_alive = true for 127.0.0.1:33952
 2020-09-21T16:06:43.887Z TRACE hyper::buffer                                > get_buf []
 2020-09-21T16:06:43.887Z TRACE hyper::buffer                                > reserved 0
 2020-09-21T16:06:43.887Z TRACE hyper::buffer                                > read_into_buf buf[0..0]
 2020-09-21T16:06:43.887Z TRACE hyper::buffer                                > read_into_buf at full capacity
 2020-09-21T16:06:43.887Z TRACE hyper::buffer                                > get_buf []
 2020-09-21T16:06:43.887Z DEBUG hyper::server                                > ioerror in keepalive loop = Custom { kind: UnexpectedEof, error: "end of stream before headers finished" }
 2020-09-21T16:06:43.887Z DEBUG hyper::server                                > keep_alive loop ending for 127.0.0.1:33952
```
@seanmonstar
Copy link
Member

seanmonstar commented Sep 24, 2020

Thanks for filing! There's been a few recent PRs targeting 0.10.x. In those, I felt it was best to close, since 0.10.x stopped seeing active development 3 years ago. This change does look small, but I'm inclined to follow the same precedent. With no development, and very little testing, I'd hate to publish a new release that might require more support, or breaks something else unexpected...

I think all I can do is suggest upgrading to a newer version of hyper. Sorry if that's disappointing.

@strohel
Copy link
Author

strohel commented Sep 24, 2020

Cool, that's totally understandable.

To communicate this, you may want to remove the 0.10.x branch from the repo - it would not possible to open a PR for it and the v0.10.16 tag still references all its commits.

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.

None yet

2 participants