Skip to content

Commit

Permalink
fix(response): prevent keep-alive if the Request had a body
Browse files Browse the repository at this point in the history
This puts a bandaid on nickel-org#326. A more complete fix will require
some shuffling of the api exposed by Request so that we can
monitor for when the body has been fully read (or read up to
a defined limit).
  • Loading branch information
Ryman committed Apr 10, 2016
1 parent 121e1e0 commit 5a8de5d
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 3 deletions.
17 changes: 14 additions & 3 deletions src/server.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@

use std::net::ToSocketAddrs;
use std::sync::{Arc, RwLock};
use std::collections::HashMap;
use std::time::Duration;
use hyper::Result as HttpResult;
use hyper::header::{ContentLength, Connection};
use hyper::server::{Request, Response, Handler, Listening};
use hyper::server::Server as HyperServer;

Expand All @@ -21,12 +23,21 @@ struct ArcServer<D>(Arc<Server<D>>);

impl<D: Sync + Send + 'static> Handler for ArcServer<D> {
fn handle<'a, 'k>(&'a self, req: Request<'a, 'k>, res: Response<'a>) {
let request_has_body = match req.headers.get::<ContentLength>() {
Some(len) if **len > 0 => true,
_ => false
};

let nickel_req = request::Request::from_internal(req,
&self.0.shared_data);

let nickel_res = response::Response::from_internal(res,
&self.0.templates,
&self.0.shared_data);
let mut nickel_res = response::Response::from_internal(res,
&self.0.templates,
&self.0.shared_data);

if request_has_body {
nickel_res.on_send(move |res| { res.set(Connection::close()); });
};

self.0.middleware_stack.invoke(nickel_req, nickel_res);
}
Expand Down
45 changes: 45 additions & 0 deletions tests/examples/chaining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,51 @@ where F: Fn(&mut Response),
}
}

#[test]
fn issue_326() {
use hyper::Client;
use hyper::header::Connection;

// The issue is that if we have a body for the request, and it isn't read
// by the server, it bleeds into the next request if keep-alive is active
run_example("chaining", |port| {
// The client is backed by a pool of keep-alive connections
let client = Client::new();
let url = format!("http://localhost:{}{}", port, "/post");

// scope the requests so the connection gets released back to the pool
{
// Assert that keep alive is still true after a zero length request body
// so that we know we're not dropping *all* requests with a body
let ref mut res = client.post(&url)
.send()
.unwrap();
assert_eq!(false, res.headers.has::<Connection>());
assert_eq!("post", read_body_to_string(res));
}

{
// Now send a request with a body, which won't be read by the server
let ref mut res = client.post(&url)
.body("0123456789")
.send()
.unwrap();
// we could assert that `Connection: close` was given, but we may
// want to be more flexible in future and allow buffers up to a
// certain length to be drained.
assert_eq!("post", read_body_to_string(res));
}

// Ensure the next request works as intended with no failure due to
// a corrupted stream.
let ref mut res = client.post(&url)
.body("0123456789")
.send()
.unwrap();
assert_eq!("post", read_body_to_string(res));
})
}

mod expect_200 {
use super::with_paths_and_method;
use util::*;
Expand Down

0 comments on commit 5a8de5d

Please sign in to comment.