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

fix(response): prevent keep-alive if the Request had a body #327

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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