diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index 3bda8705cc5..1fbe2ec26e8 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -433,12 +433,22 @@ pub fn load(mut load_data: LoadData, info!("{:?}", load_data.data); } - // TODO: Avoid automatically sending request body if a redirect has occurred. - if let Some(ref data) = load_data.data { - req.headers_mut().set(ContentLength(data.len() as u64)); - } - - let response = try!(req.send(&load_data.data)); + // Avoid automatically sending request body if a redirect has occurred. + // + // TODO - This is the wrong behaviour according to the RFC. However, I'm not + // sure how much "correctness" vs. real-world is important in this case. + // + // http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html + let is_redirected_request = iters != 1; + let response = match load_data.data { + Some(ref data) if !is_redirected_request => { + req.headers_mut().set(ContentLength(data.len() as u64)); + try!(req.send(&load_data.data)) + } + _ => { + try!(req.send(&None)) + } + }; // --- Tell devtools we've made a request // Send an HttpRequest message to devtools with a unique request_id diff --git a/tests/unit/net/http_loader.rs b/tests/unit/net/http_loader.rs index 1ffab3fdd37..564b87de79f 100644 --- a/tests/unit/net/http_loader.rs +++ b/tests/unit/net/http_loader.rs @@ -188,6 +188,65 @@ fn assert_cookie_for_domain(resource_mgr: &ResourceTask, domain: &str, cookie: & } } +struct AssertMustHaveBodyRequest { + expected_body: Option>, + headers: Headers, + t: RequestType +} + +impl AssertMustHaveBodyRequest { + fn new(t: RequestType, expected_body: Option>) -> Self { + AssertMustHaveBodyRequest { expected_body: expected_body, headers: Headers::new(), t: t } + } +} + +impl HttpRequest for AssertMustHaveBodyRequest { + type R=MockResponse; + + fn headers_mut(&mut self) -> &mut Headers { &mut self.headers} + + fn send(self, body: &Option>) -> Result { + assert_eq!(self.expected_body, *body); + + response_for_request_type(self.t) + } +} + +#[test] +fn test_load_doesnt_send_request_body_on_any_redirect() { + struct Factory; + + impl HttpRequestFactory for Factory { + type R=AssertMustHaveBodyRequest; + + fn create(&self, url: Url, _: Method) -> Result { + if url.domain().unwrap() == "mozilla.com" { + Ok( + AssertMustHaveBodyRequest::new( + RequestType::Redirect("http://mozilla.org".to_string()), + Some(<[_]>::to_vec("Body on POST!".as_bytes())) + ) + ) + } else { + Ok( + AssertMustHaveBodyRequest::new( + RequestType::Text(<[_]>::to_vec("Yay!".as_bytes())), + None + ) + ) + } + } + } + + let url = Url::parse("http://mozilla.com").unwrap(); + let resource_mgr = new_resource_task(None, None); + let mut load_data = LoadData::new(url.clone(), None); + load_data.data = Some(<[_]>::to_vec("Body on POST!".as_bytes())); + + + let _ = load::(load_data, resource_mgr, None, &Factory); +} + #[test] fn test_load_doesnt_add_host_to_sts_list_when_url_is_http_even_if_sts_headers_are_present() { struct Factory;