Move setting UserAgent header into http_loader::load,

closes #7541

components/net/http_loader.rs

@@ -11,7 +11,7 @@ use hyper::Error as HttpError;
 use hyper::client::{Request, Response, Pool};
 use hyper::error::Result as HttpResult;
 use hyper::header::{AcceptEncoding, Accept, ContentLength, ContentType, Host};
-use hyper::header::{Location, qitem, StrictTransportSecurity};
+use hyper::header::{Location, qitem, StrictTransportSecurity, UserAgent};
 use hyper::header::{Quality, QualityItem, Headers, ContentEncoding, Encoding};
 use hyper::http::RawStatus;
 use hyper::method::Method;
@@ -77,10 +77,10 @@ pub fn create_http_connector() -> Arc<Pool<Connector>> {
 pub fn factory(resource_mgr_chan: IpcSender<ControlMsg>,
                devtools_chan: Option<Sender<DevtoolsControlMsg>>,
                connector: Arc<Pool<Connector>>)
-               -> Box<FnBox(LoadData, LoadConsumer, Arc<MIMEClassifier>) + Send> {
-    box move |load_data: LoadData, senders, classifier| {
+               -> Box<FnBox(LoadData, LoadConsumer, Arc<MIMEClassifier>, String) + Send> {
+    box move |load_data: LoadData, senders, classifier, user_agent| {
         spawn_named(format!("http_loader for {}", load_data.url.serialize()), move || {
-            load_for_consumer(load_data, senders, classifier, connector, resource_mgr_chan, devtools_chan)
+            load_for_consumer(load_data, senders, classifier, connector, resource_mgr_chan, devtools_chan, user_agent)
         })
     }
 }
@@ -123,12 +123,13 @@ fn load_for_consumer(load_data: LoadData,
                      classifier: Arc<MIMEClassifier>,
                      connector: Arc<Pool<Connector>>,
                      resource_mgr_chan: IpcSender<ControlMsg>,
-                     devtools_chan: Option<Sender<DevtoolsControlMsg>>) {
+                     devtools_chan: Option<Sender<DevtoolsControlMsg>>,
+                     user_agent: String) {
 
     let factory = NetworkHttpRequestFactory {
         connector: connector,
     };
-    match load::<WrappedHttpRequest>(load_data, resource_mgr_chan, devtools_chan, &factory) {
+    match load::<WrappedHttpRequest>(load_data, resource_mgr_chan, devtools_chan, &factory, user_agent) {
         Err(LoadError::UnsupportedScheme(url)) => {
             let s = format!("{} request, but we don't support that scheme", &*url.scheme);
             send_error(url, s, start_chan)
@@ -477,7 +478,8 @@ fn send_response_to_devtools(devtools_chan: Option<Sender<DevtoolsControlMsg>>,
 pub fn load<A>(load_data: LoadData,
                resource_mgr_chan: IpcSender<ControlMsg>,
                devtools_chan: Option<Sender<DevtoolsControlMsg>>,
-               request_factory: &HttpRequestFactory<R=A>)
+               request_factory: &HttpRequestFactory<R=A>,
+               user_agent: String)
                -> Result<StreamedResponse<A::R>, LoadError> where A: HttpRequest + 'static {
     // FIXME: At the time of writing this FIXME, servo didn't have any central
     //        location for configuration. If you're reading this and such a
@@ -541,6 +543,8 @@ pub fn load<A>(load_data: LoadData,
 
         request_headers.set(host);
 
+        request_headers.set(UserAgent(user_agent.clone()));
+
         set_default_accept(&mut request_headers);
         set_default_accept_encoding(&mut request_headers);
         set_request_cookies(doc_url.clone(), &mut request_headers, &resource_mgr_chan);

components/net/resource_task.rs

@@ -23,7 +23,7 @@ use hsts::{HSTSList, HSTSEntry, preload_hsts_domains};
 
 use devtools_traits::{DevtoolsControlMsg};
 use hyper::client::pool::Pool;
-use hyper::header::{ContentType, Header, SetCookie, UserAgent};
+use hyper::header::{ContentType, Header, SetCookie};
 use hyper::mime::{Mime, TopLevel, SubLevel};
 use ipc_channel::ipc::{self, IpcReceiver, IpcSender};
 
@@ -233,12 +233,11 @@ impl ResourceManager {
         self.hsts_list.is_host_secure(host)
     }
 
-    fn load(&mut self, mut load_data: LoadData, consumer: LoadConsumer) {
-        load_data.preserved_headers.set(UserAgent(self.user_agent.clone()));
+    fn load(&mut self, load_data: LoadData, consumer: LoadConsumer) {
 
         fn from_factory(factory: fn(LoadData, LoadConsumer, Arc<MIMEClassifier>))
-                        -> Box<FnBox(LoadData, LoadConsumer, Arc<MIMEClassifier>) + Send> {
-            box move |load_data, senders, classifier| {
+                        -> Box<FnBox(LoadData, LoadConsumer, Arc<MIMEClassifier>, String) + Send> {
+            box move |load_data, senders, classifier, _user_agent| {
                 factory(load_data, senders, classifier)
             }
         }
@@ -260,6 +259,6 @@ impl ResourceManager {
         };
         debug!("resource_task: loading url: {}", load_data.url.serialize());
 
-        loader.call_box((load_data, consumer, self.mime_classifier.clone()));
+        loader.call_box((load_data, consumer, self.mime_classifier.clone(), self.user_agent.clone()));
     }
 }

tests/unit/net/http_loader.rs

@@ -17,6 +17,8 @@ use std::borrow::Cow;
 use std::io::{self, Write, Read, Cursor};
 use url::Url;
 
+const DEFAULT_USER_AGENT: &'static str = "Test-agent";
+
 fn respond_with(body: Vec<u8>) -> MockResponse {
     let mut headers = Headers::new();
     respond_with_headers(body, &mut headers)
@@ -211,7 +213,7 @@ impl HttpRequest for AssertMustHaveBodyRequest {
 #[test]
 fn test_load_when_request_is_not_get_or_head_and_there_is_no_body_content_length_should_be_set_to_0() {
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
 
     let mut load_data = LoadData::new(url.clone(), None);
     load_data.data = None;
@@ -225,7 +227,7 @@ fn test_load_when_request_is_not_get_or_head_and_there_is_no_body_content_length
         &AssertMustHaveHeadersRequestFactory {
             expected_headers: content_length,
             body: <[_]>::to_vec(&[])
-        });
+        }, DEFAULT_USER_AGENT.to_string());
 }
 
 #[test]
@@ -247,11 +249,11 @@ fn test_load_when_redirecting_from_a_post_should_rewrite_next_request_as_get() {
     }
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let mut load_data = LoadData::new(url.clone(), None);
     load_data.method = Method::Post;
 
-    let _ = load::<MockRequest>(load_data, resource_mgr, None, &Factory);
+    let _ = load::<MockRequest>(load_data, resource_mgr, None, &Factory, DEFAULT_USER_AGENT.to_string());
 }
 
 #[test]
@@ -273,9 +275,13 @@ fn test_load_should_decode_the_response_as_deflate_when_response_headers_have_co
     }
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let load_data = LoadData::new(url.clone(), None);
-    let mut response = load::<MockRequest>(load_data, resource_mgr.clone(), None, &Factory).unwrap();
+    let mut response = load::<MockRequest>(
+        load_data, resource_mgr.clone(), None,
+        &Factory,
+        DEFAULT_USER_AGENT.to_string())
+        .unwrap();
 
     assert_eq!(read_response(&mut response), "Yay!");
 }
@@ -299,9 +305,14 @@ fn test_load_should_decode_the_response_as_gzip_when_response_headers_have_conte
     }
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let load_data = LoadData::new(url.clone(), None);
-    let mut response = load::<MockRequest>(load_data, resource_mgr.clone(), None, &Factory).unwrap();
+    let mut response = load::<MockRequest>(
+        load_data,
+        resource_mgr.clone(),
+        None, &Factory,
+        DEFAULT_USER_AGENT.to_string())
+        .unwrap();
 
     assert_eq!(read_response(&mut response), "Yay!");
 }
@@ -333,12 +344,16 @@ fn test_load_doesnt_send_request_body_on_any_redirect() {
     }
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let mut load_data = LoadData::new(url.clone(), None);
     load_data.data = Some(<[_]>::to_vec("Body on POST!".as_bytes()));
 
 
-    let _ = load::<AssertMustHaveBodyRequest>(load_data, resource_mgr, None, &Factory);
+    let _ = load::<AssertMustHaveBodyRequest>(
+        load_data, resource_mgr,
+        None,
+        &Factory,
+        DEFAULT_USER_AGENT.to_string());
 }
 
 #[test]
@@ -357,11 +372,11 @@ fn test_load_doesnt_add_host_to_sts_list_when_url_is_http_even_if_sts_headers_ar
     }
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
 
     let load_data = LoadData::new(url.clone(), None);
 
-    let _ = load::<MockRequest>(load_data, resource_mgr.clone(), None, &Factory);
+    let _ = load::<MockRequest>(load_data, resource_mgr.clone(), None, &Factory, DEFAULT_USER_AGENT.to_string());
 
     let (tx, rx) = ipc::channel().unwrap();
     resource_mgr.send(ControlMsg::GetHostMustBeSecured("mozilla.com".to_string(), tx)).unwrap();
@@ -385,11 +400,11 @@ fn test_load_adds_host_to_sts_list_when_url_is_https_and_sts_headers_are_present
     }
 
     let url = Url::parse("https://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
 
     let load_data = LoadData::new(url.clone(), None);
 
-    let _ = load::<MockRequest>(load_data, resource_mgr.clone(), None, &Factory);
+    let _ = load::<MockRequest>(load_data, resource_mgr.clone(), None, &Factory, DEFAULT_USER_AGENT.to_string());
 
     let (tx, rx) = ipc::channel().unwrap();
     resource_mgr.send(ControlMsg::GetHostMustBeSecured("mozilla.com".to_string(), tx)).unwrap();
@@ -413,20 +428,20 @@ fn test_load_sets_cookies_in_the_resource_manager_when_it_get_set_cookie_header_
     }
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     assert_cookie_for_domain(&resource_mgr, "http://mozilla.com", "");
 
     let load_data = LoadData::new(url.clone(), None);
 
-    let _ = load::<MockRequest>(load_data, resource_mgr.clone(), None, &Factory);
+    let _ = load::<MockRequest>(load_data, resource_mgr.clone(), None, &Factory, DEFAULT_USER_AGENT.to_string());
 
     assert_cookie_for_domain(&resource_mgr, "http://mozilla.com", "mozillaIs=theBest");
 }
 
 #[test]
 fn test_load_sets_requests_cookies_header_for_url_by_getting_cookies_from_the_resource_manager() {
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     resource_mgr.send(ControlMsg::SetCookiesForUrl(Url::parse("http://mozilla.com").unwrap(),
                                                    "mozillaIs=theBest".to_string(),
                                                    CookieSource::HTTP)).unwrap();
@@ -442,15 +457,15 @@ fn test_load_sets_requests_cookies_header_for_url_by_getting_cookies_from_the_re
         &AssertMustHaveHeadersRequestFactory {
             expected_headers: cookie,
             body: <[_]>::to_vec(&*load_data.data.unwrap())
-        });
+        }, DEFAULT_USER_AGENT.to_string());
 }
 
 #[test]
 fn test_load_sets_content_length_to_length_of_request_body() {
     let content = "This is a request body";
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let mut load_data = LoadData::new(url.clone(), None);
     load_data.data = Some(<[_]>::to_vec(content.as_bytes()));
 
@@ -465,7 +480,7 @@ fn test_load_sets_content_length_to_length_of_request_body() {
         &AssertMustHaveHeadersRequestFactory {
             expected_headers: content_len_headers,
             body: <[_]>::to_vec(&*load_data.data.unwrap())
-        });
+        }, DEFAULT_USER_AGENT.to_string());
 }
 
 #[test]
@@ -474,15 +489,15 @@ fn test_load_uses_explicit_accept_from_headers_in_load_data() {
     accept_headers.set_raw("Accept".to_owned(), vec![b"text/html".to_vec()]);
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let mut load_data = LoadData::new(url.clone(), None);
     load_data.data = Some(<[_]>::to_vec("Yay!".as_bytes()));
     load_data.headers.set_raw("Accept".to_owned(), vec![b"text/html".to_vec()]);
 
     let _ = load::<AssertRequestMustHaveHeaders>(load_data, resource_mgr, None, &AssertMustHaveHeadersRequestFactory {
         expected_headers: accept_headers,
         body: <[_]>::to_vec("Yay!".as_bytes())
-    });
+    }, DEFAULT_USER_AGENT.to_string());
 }
 
 #[test]
@@ -493,14 +508,14 @@ fn test_load_sets_default_accept_to_html_xhtml_xml_and_then_anything_else() {
     );
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let mut load_data = LoadData::new(url.clone(), None);
     load_data.data = Some(<[_]>::to_vec("Yay!".as_bytes()));
 
     let _ = load::<AssertRequestMustHaveHeaders>(load_data, resource_mgr, None, &AssertMustHaveHeadersRequestFactory {
         expected_headers: accept_headers,
         body: <[_]>::to_vec("Yay!".as_bytes())
-    });
+    }, DEFAULT_USER_AGENT.to_string());
 }
 
 #[test]
@@ -509,15 +524,15 @@ fn test_load_uses_explicit_accept_encoding_from_load_data_headers() {
     accept_encoding_headers.set_raw("Accept-Encoding".to_owned(), vec![b"chunked".to_vec()]);
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let mut load_data = LoadData::new(url.clone(), None);
     load_data.data = Some(<[_]>::to_vec("Yay!".as_bytes()));
     load_data.headers.set_raw("Accept-Encoding".to_owned(), vec![b"chunked".to_vec()]);
 
     let _ = load::<AssertRequestMustHaveHeaders>(load_data, resource_mgr, None, &AssertMustHaveHeadersRequestFactory {
         expected_headers: accept_encoding_headers,
         body: <[_]>::to_vec("Yay!".as_bytes())
-    });
+    }, DEFAULT_USER_AGENT.to_string());
 }
 
 #[test]
@@ -526,14 +541,14 @@ fn test_load_sets_default_accept_encoding_to_gzip_and_deflate() {
     accept_encoding_headers.set_raw("Accept-Encoding".to_owned(), vec![b"gzip, deflate".to_vec()]);
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let mut load_data = LoadData::new(url.clone(), None);
     load_data.data = Some(<[_]>::to_vec("Yay!".as_bytes()));
 
     let _ = load::<AssertRequestMustHaveHeaders>(load_data, resource_mgr, None, &AssertMustHaveHeadersRequestFactory {
         expected_headers: accept_encoding_headers,
         body: <[_]>::to_vec("Yay!".as_bytes())
-    });
+    }, DEFAULT_USER_AGENT.to_string());
 }
 
 #[test]
@@ -555,10 +570,10 @@ fn test_load_errors_when_there_a_redirect_loop() {
     }
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let load_data = LoadData::new(url.clone(), None);
 
-    match load::<MockRequest>(load_data, resource_mgr, None, &Factory) {
+    match load::<MockRequest>(load_data, resource_mgr, None, &Factory, DEFAULT_USER_AGENT.to_string()) {
         Err(LoadError::InvalidRedirect(_, msg)) => {
             assert_eq!(msg, "redirect loop");
         },
@@ -583,10 +598,10 @@ fn test_load_errors_when_there_is_too_many_redirects() {
     }
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let load_data = LoadData::new(url.clone(), None);
 
-    match load::<MockRequest>(load_data, resource_mgr, None, &Factory) {
+    match load::<MockRequest>(load_data, resource_mgr, None, &Factory, DEFAULT_USER_AGENT.to_string()) {
         Err(LoadError::MaxRedirects(url)) => {
             assert_eq!(url.domain().unwrap(), "mozilla.com")
         },
@@ -619,10 +634,10 @@ fn test_load_follows_a_redirect() {
     }
 
     let url = Url::parse("http://mozilla.com").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let load_data = LoadData::new(url.clone(), None);
 
-    match load::<MockRequest>(load_data, resource_mgr, None, &Factory) {
+    match load::<MockRequest>(load_data, resource_mgr, None, &Factory, DEFAULT_USER_AGENT.to_string()) {
         Err(e) => panic!("expected to follow a redirect {:?}", e),
         Ok(mut lr) => {
             let response = read_response(&mut lr);
@@ -644,10 +659,10 @@ impl HttpRequestFactory for DontConnectFactory {
 #[test]
 fn test_load_errors_when_scheme_is_not_http_or_https() {
     let url = Url::parse("ftp://not-supported").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let load_data = LoadData::new(url.clone(), None);
 
-    match load::<MockRequest>(load_data, resource_mgr, None, &DontConnectFactory) {
+    match load::<MockRequest>(load_data, resource_mgr, None, &DontConnectFactory, DEFAULT_USER_AGENT.to_string()) {
         Err(LoadError::UnsupportedScheme(_)) => {}
         _ => panic!("expected ftp scheme to be unsupported")
     }
@@ -656,10 +671,10 @@ fn test_load_errors_when_scheme_is_not_http_or_https() {
 #[test]
 fn test_load_errors_when_viewing_source_and_inner_url_scheme_is_not_http_or_https() {
     let url = Url::parse("view-source:ftp://not-supported").unwrap();
-    let resource_mgr = new_resource_task("Test-agent".to_string(), None);
+    let resource_mgr = new_resource_task(DEFAULT_USER_AGENT.to_string(), None);
     let load_data = LoadData::new(url.clone(), None);
 
-    match load::<MockRequest>(load_data, resource_mgr, None, &DontConnectFactory) {
+    match load::<MockRequest>(load_data, resource_mgr, None, &DontConnectFactory, DEFAULT_USER_AGENT.to_string()) {
         Err(LoadError::UnsupportedScheme(_)) => {}
         _ => panic!("expected ftp scheme to be unsupported")
     }