From 2d11a4bd7167e1bf3a35b62f5aeb36d5d294e56e Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Mon, 24 Jul 2017 16:11:54 -0700 Subject: [PATCH] fix 307/308 redirects with GET requests --- src/async_impl/client.rs | 15 +++++----- tests/redirect.rs | 59 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/src/async_impl/client.rs b/src/async_impl/client.rs index b437b7f7b..d0095098f 100644 --- a/src/async_impl/client.rs +++ b/src/async_impl/client.rs @@ -328,10 +328,10 @@ impl Client { let uri = to_uri(&url); let mut req = ::hyper::Request::new(method.clone(), uri.clone()); *req.headers_mut() = headers.clone(); - let body = body.and_then(|body| { - let (resuable, body) = body::into_hyper(body); + let body = body.map(|body| { + let (reusable, body) = body::into_hyper(body); req.set_body(body); - resuable + reusable }); if proxy::is_proxied(&self.inner.proxies, &uri) { @@ -386,7 +386,7 @@ pub struct Pending { method: Method, url: Url, headers: Headers, - body: Option, + body: Option>, urls: Vec, @@ -419,8 +419,9 @@ impl Future for Pending { true }, StatusCode::TemporaryRedirect | - StatusCode::PermanentRedirect => { - self.body.is_some() + StatusCode::PermanentRedirect => match self.body { + Some(Some(_)) | None => true, + Some(None) => false, }, _ => false, }; @@ -449,7 +450,7 @@ impl Future for Pending { uri.clone() ); *req.headers_mut() = self.headers.clone(); - if let Some(ref body) = self.body { + if let Some(Some(ref body)) = self.body { req.set_body(body.clone()); } if proxy::is_proxied(&self.client.proxies, &uri) { diff --git a/tests/redirect.rs b/tests/redirect.rs index 1c7ed0d0c..4baecce75 100644 --- a/tests/redirect.rs +++ b/tests/redirect.rs @@ -58,6 +58,59 @@ fn test_redirect_301_and_302_and_303_changes_post_to_get() { } } +#[test] +fn test_redirect_307_and_308_tries_to_get_again() { + let client = reqwest::Client::new().unwrap(); + let codes = [307, 308]; + for code in codes.iter() { + let redirect = server! { + request: format!("\ + GET /{} HTTP/1.1\r\n\ + Host: $HOST\r\n\ + User-Agent: $USERAGENT\r\n\ + Accept: */*\r\n\ + Accept-Encoding: gzip\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\ + Accept: */*\r\n\ + Accept-Encoding: gzip\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 url = format!("http://{}/{}", redirect.addr(), code); + let dst = format!("http://{}/{}", redirect.addr(), "dst"); + let res = client.get(&url) + .unwrap() + .send() + .unwrap(); + assert_eq!(res.url().as_str(), dst); + assert_eq!(res.status(), reqwest::StatusCode::Ok); + assert_eq!(res.headers().get(), + Some(&reqwest::header::Server::new("test-dst".to_string()))); + } +} + #[test] fn test_redirect_307_and_308_tries_to_post_again() { let client = reqwest::Client::new().unwrap(); @@ -116,7 +169,6 @@ fn test_redirect_307_and_308_tries_to_post_again() { } } -/* #[test] fn test_redirect_307_does_not_try_if_reader_cannot_reset() { let client = reqwest::Client::new().unwrap(); @@ -127,7 +179,7 @@ fn test_redirect_307_does_not_try_if_reader_cannot_reset() { POST /{} HTTP/1.1\r\n\ Host: $HOST\r\n\ User-Agent: $USERAGENT\r\n\ - Accept: * / *\r\n\ + Accept: */*\r\n\ Accept-Encoding: gzip\r\n\ Transfer-Encoding: chunked\r\n\ \r\n\ @@ -156,7 +208,8 @@ fn test_redirect_307_does_not_try_if_reader_cannot_reset() { assert_eq!(res.status(), reqwest::StatusCode::try_from(code).unwrap()); } } -*/ + + #[test] fn test_redirect_removes_sensitive_headers() {