diff --git a/src/body.rs b/src/body.rs index 43fcfee..9bd9756 100644 --- a/src/body.rs +++ b/src/body.rs @@ -10,6 +10,17 @@ pub struct Body { impl Body { /// Instantiate a `Body` from a reader. + /// + /// # Note + /// + /// While allowing for many types to be used, these bodies do not have + /// a way to reset to the beginning and be reused. This means that when + /// encountering a 307 or 308 status code, instead of repeating the + /// request at the new location, the `Response` will be returned with + /// the redirect status code set. + /// + /// A `Body` constructed from a set of bytes, like `String` or `Vec`, + /// are stored differently and can be reused. pub fn new(reader: R) -> Body { Body { reader: Kind::Reader(Box::new(reader), None), @@ -115,3 +126,10 @@ pub fn as_hyper_body<'a>(body: &'a mut Body) -> ::hyper::client::Body<'a> { } } } + +pub fn can_reset(body: &Body) -> bool { + match body.reader { + Kind::Bytes(_) => true, + Kind::Reader(..) => false, + } +} diff --git a/src/client.rs b/src/client.rs index 5fd0319..a6a93f4 100644 --- a/src/client.rs +++ b/src/client.rs @@ -198,57 +198,68 @@ impl<'a> RequestBuilder<'a> { try!(req.send()) }; - body.take(); - match res.status { + let should_redirect = match res.status { StatusCode::MovedPermanently | StatusCode::Found | StatusCode::SeeOther => { - - //TODO: turn this into self.redirect_policy.check() - if redirect_count > 10 { - return Err(::Error::TooManyRedirects); + body = None; + match method { + Method::Get | Method::Head => {}, + _ => { + method = Method::Get; + } } - redirect_count += 1; - - method = match method { - Method::Post | Method::Put => Method::Get, - m => m - }; - - headers.set(Referer(url.to_string())); - - let loc = { - let loc = res.headers.get::().map(|loc| url.join(loc)); - if let Some(loc) = loc { - loc - } else { - return Ok(Response { - inner: res - }); - } - }; - - url = match loc { - Ok(u) => u, - Err(e) => { - debug!("Location header had invalid URI: {:?}", e); - return Ok(Response { - inner: res - }) - } - }; - - debug!("redirecting to '{}'", url); - - //TODO: removeSensitiveHeaders(&mut headers, &url); - + true }, - _ => { - return Ok(Response { - inner: res - }); + StatusCode::TemporaryRedirect | + StatusCode::PermanentRedirect => { + if let Some(ref body) = body { + body::can_reset(body) + } else { + true + } + }, + _ => false, + }; + + if should_redirect { + //TODO: turn this into self.redirect_policy.check() + if redirect_count > 10 { + return Err(::Error::TooManyRedirects); } + redirect_count += 1; + + headers.set(Referer(url.to_string())); + + let loc = { + let loc = res.headers.get::().map(|loc| url.join(loc)); + if let Some(loc) = loc { + loc + } else { + return Ok(Response { + inner: res + }); + } + }; + + url = match loc { + Ok(u) => u, + Err(e) => { + debug!("Location header had invalid URI: {:?}", e); + return Ok(Response { + inner: res + }) + } + }; + + debug!("redirecting to {:?} '{}'", method, url); + + //TODO: removeSensitiveHeaders(&mut headers, &url); + } else { + return Ok(Response { + inner: res + }); } } } diff --git a/tests/client.rs b/tests/client.rs index c6e8863..39b8080 100644 --- a/tests/client.rs +++ b/tests/client.rs @@ -33,45 +33,130 @@ fn test_get() { } #[test] -fn test_redirect_302_changes_post_to_get() { - - let redirect = server! { - request: b"\ - POST /302 HTTP/1.1\r\n\ - Host: $HOST\r\n\ - User-Agent: $USERAGENT\r\n\ - Content-Length: 0\r\n\ - \r\n\ - ", - response: b"\ - HTTP/1.1 302 Found\r\n\ - Server: test-redirect\r\n\ - Content-Length: 0\r\n\ - Location: /dst\r\n\ - Connection: close\r\n\ - \r\n\ - ", - - request: b"\ - GET /dst HTTP/1.1\r\n\ - Host: $HOST\r\n\ - User-Agent: $USERAGENT\r\n\ - Referer: http://$HOST/302\r\n\ - \r\n\ - ", - response: b"\ - HTTP/1.1 200 OK\r\n\ - Server: test-dst\r\n\ - Content-Length: 0\r\n\ - \r\n\ - " - }; - +fn test_redirect_301_and_302_and_303_changes_post_to_get() { let client = reqwest::Client::new().unwrap(); - let res = client.post(&format!("http://{}/302", redirect.addr())) - .send() - .unwrap(); - assert_eq!(res.status(), &reqwest::StatusCode::Ok); - assert_eq!(res.headers().get(), Some(&reqwest::header::Server("test-dst".to_string()))); + let codes = [301, 302, 303]; + for code in codes.iter() { + let redirect = server! { + request: format!("\ + POST /{} HTTP/1.1\r\n\ + Host: $HOST\r\n\ + User-Agent: $USERAGENT\r\n\ + Content-Length: 0\r\n\ + \r\n\ + ", code), + response: format!("\ + HTTP/1.1 {} reason\r\n\ + Server: test-redirect\r\n\ + Content-Length: 0\r\n\ + Location: /dst\r\n\ + Connection: close\r\n\ + \r\n\ + ", code), + + request: format!("\ + GET /dst HTTP/1.1\r\n\ + Host: $HOST\r\n\ + User-Agent: $USERAGENT\r\n\ + Referer: http://$HOST/{}\r\n\ + \r\n\ + ", code), + response: b"\ + HTTP/1.1 200 OK\r\n\ + Server: test-dst\r\n\ + Content-Length: 0\r\n\ + \r\n\ + " + }; + + let res = client.post(&format!("http://{}/{}", redirect.addr(), code)) + .send() + .unwrap(); + assert_eq!(res.status(), &reqwest::StatusCode::Ok); + assert_eq!(res.headers().get(), Some(&reqwest::header::Server("test-dst".to_string()))); + } +} + +#[test] +fn test_redirect_307_and_308_tries_to_post_again() { + let client = reqwest::Client::new().unwrap(); + let codes = [307, 308]; + for code in codes.iter() { + let redirect = server! { + request: format!("\ + POST /{} HTTP/1.1\r\n\ + Host: $HOST\r\n\ + User-Agent: $USERAGENT\r\n\ + Content-Length: 5\r\n\ + \r\n\ + Hello\ + ", code), + response: format!("\ + HTTP/1.1 {} reason\r\n\ + Server: test-redirect\r\n\ + Content-Length: 0\r\n\ + Location: /dst\r\n\ + Connection: close\r\n\ + \r\n\ + ", code), + + request: format!("\ + POST /dst HTTP/1.1\r\n\ + Host: $HOST\r\n\ + User-Agent: $USERAGENT\r\n\ + Referer: http://$HOST/{}\r\n\ + Content-Length: 5\r\n\ + \r\n\ + Hello\ + ", code), + response: b"\ + HTTP/1.1 200 OK\r\n\ + Server: test-dst\r\n\ + Content-Length: 0\r\n\ + \r\n\ + " + }; + + let res = client.post(&format!("http://{}/{}", redirect.addr(), code)) + .body("Hello") + .send() + .unwrap(); + assert_eq!(res.status(), &reqwest::StatusCode::Ok); + assert_eq!(res.headers().get(), Some(&reqwest::header::Server("test-dst".to_string()))); + } +} + +#[test] +fn test_redirect_307_does_not_try_if_reader_cannot_reset() { + let client = reqwest::Client::new().unwrap(); + let codes = [307, 308]; + for &code in codes.iter() { + let redirect = server! { + request: format!("\ + POST /{} HTTP/1.1\r\n\ + Host: $HOST\r\n\ + User-Agent: $USERAGENT\r\n\ + Transfer-Encoding: chunked\r\n\ + \r\n\ + 5\r\n\ + Hello\r\n\ + 0\r\n\r\n\ + ", code), + response: format!("\ + HTTP/1.1 {} reason\r\n\ + Server: test-redirect\r\n\ + Content-Length: 0\r\n\ + Location: /dst\r\n\ + Connection: close\r\n\ + \r\n\ + ", code) + }; + + let res = client.post(&format!("http://{}/{}", redirect.addr(), code)) + .body(reqwest::Body::new(&b"Hello"[..])) + .send() + .unwrap(); + assert_eq!(res.status(), &reqwest::StatusCode::from_u16(code)); + } }