From a8f655caf3a646fdecb6b1f77c64b30f2a074939 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 3 Nov 2016 11:24:15 +0100 Subject: [PATCH 1/6] Move some helper functions to the root of the net tests crate. --- tests/unit/net/fetch.rs | 43 ++----------------------------------- tests/unit/net/lib.rs | 47 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 41 deletions(-) diff --git a/tests/unit/net/fetch.rs b/tests/unit/net/fetch.rs index 8ab9c4e8795..0be39b1710f 100644 --- a/tests/unit/net/fetch.rs +++ b/tests/unit/net/fetch.rs @@ -2,10 +2,10 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +use {DEFAULT_USER_AGENT, FetchResponseCollector, new_fetch_context, fetch_async, fetch_sync}; use devtools_traits::DevtoolsControlMsg; use devtools_traits::HttpRequest as DevtoolsHttpRequest; use devtools_traits::HttpResponse as DevtoolsHttpResponse; -use filemanager_thread::{TestProvider, TEST_PROVIDER}; use http_loader::{expect_devtools_http_request, expect_devtools_http_response}; use hyper::LanguageTag; use hyper::header::{Accept, AccessControlAllowCredentials, AccessControlAllowHeaders, AccessControlAllowOrigin}; @@ -22,10 +22,7 @@ use hyper::status::StatusCode; use hyper::uri::RequestUri; use msg::constellation_msg::{ReferrerPolicy, TEST_PIPELINE_ID}; use net::fetch::cors_cache::CORSCache; -use net::fetch::methods::{FetchContext, fetch, fetch_with_cors_cache}; -use net::filemanager_thread::FileManager; -use net::http_loader::HttpState; -use net_traits::FetchTaskTarget; +use net::fetch::methods::{fetch, fetch_with_cors_cache}; use net_traits::request::{Origin, RedirectMode, Referrer, Request, RequestMode}; use net_traits::response::{CacheState, Response, ResponseBody, ResponseType}; use std::fs::File; @@ -34,49 +31,13 @@ use std::rc::Rc; use std::sync::{Arc, Mutex}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::mpsc::{Sender, channel}; -use std::thread; use time::{self, Duration}; use unicase::UniCase; use url::{Origin as UrlOrigin, Url}; use util::resource_files::resources_dir_path; -const DEFAULT_USER_AGENT: &'static str = "Such Browser. Very Layout. Wow."; - // TODO write a struct that impls Handler for storing test values -struct FetchResponseCollector { - sender: Sender, -} - -fn new_fetch_context(dc: Option>) -> FetchContext { - FetchContext { - state: HttpState::new(), - user_agent: DEFAULT_USER_AGENT.into(), - devtools_chan: dc, - filemanager: FileManager::new(TEST_PROVIDER), - } -} -impl FetchTaskTarget for FetchResponseCollector { - fn process_request_body(&mut self, _: &Request) {} - fn process_request_eof(&mut self, _: &Request) {} - fn process_response(&mut self, _: &Response) {} - fn process_response_chunk(&mut self, _: Vec) {} - /// Fired when the response is fully fetched - fn process_response_eof(&mut self, response: &Response) { - let _ = self.sender.send(response.clone()); - } -} - -fn fetch_async(request: Request, target: Box, dc: Option>) { - thread::spawn(move || { - fetch(Rc::new(request), &mut Some(target), new_fetch_context(dc)); - }); -} - -fn fetch_sync(request: Request, dc: Option>) -> Response { - fetch(Rc::new(request), &mut None, new_fetch_context(dc)) -} - fn make_server(handler: H) -> (Listening, Url) { // this is a Listening server because of handle_threads() let server = Server::http("0.0.0.0:0").unwrap().handle_threads(handler, 1).unwrap(); diff --git a/tests/unit/net/lib.rs b/tests/unit/net/lib.rs index f7bdec62382..14319d51f15 100644 --- a/tests/unit/net/lib.rs +++ b/tests/unit/net/lib.rs @@ -32,3 +32,50 @@ extern crate util; #[cfg(test)] mod hsts; #[cfg(test)] mod http_loader; #[cfg(test)] mod filemanager_thread; + +use devtools_traits::DevtoolsControlMsg; +use filemanager_thread::{TestProvider, TEST_PROVIDER}; +use net::fetch::methods::{FetchContext, fetch}; +use net::filemanager_thread::FileManager; +use net::http_loader::HttpState; +use net_traits::FetchTaskTarget; +use net_traits::request::Request; +use net_traits::response::Response; +use std::rc::Rc; +use std::sync::mpsc::Sender; +use std::thread; + +const DEFAULT_USER_AGENT: &'static str = "Such Browser. Very Layout. Wow."; + +struct FetchResponseCollector { + sender: Sender, +} + +fn new_fetch_context(dc: Option>) -> FetchContext { + FetchContext { + state: HttpState::new(), + user_agent: DEFAULT_USER_AGENT.into(), + devtools_chan: dc, + filemanager: FileManager::new(TEST_PROVIDER), + } +} +impl FetchTaskTarget for FetchResponseCollector { + fn process_request_body(&mut self, _: &Request) {} + fn process_request_eof(&mut self, _: &Request) {} + fn process_response(&mut self, _: &Response) {} + fn process_response_chunk(&mut self, _: Vec) {} + /// Fired when the response is fully fetched + fn process_response_eof(&mut self, response: &Response) { + let _ = self.sender.send(response.clone()); + } +} + +fn fetch_async(request: Request, target: Box, dc: Option>) { + thread::spawn(move || { + fetch(Rc::new(request), &mut Some(target), new_fetch_context(dc)); + }); +} + +fn fetch_sync(request: Request, dc: Option>) -> Response { + fetch(Rc::new(request), &mut None, new_fetch_context(dc)) +} From 7e1c7075e5970f2872451e8fce9f7a6e0957fc3e Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 3 Nov 2016 11:26:49 +0100 Subject: [PATCH 2/6] Remove unnecessary extern crates from networking unit tests. --- tests/unit/net/cookie.rs | 3 +-- tests/unit/net/data_loader.rs | 7 ++----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/unit/net/cookie.rs b/tests/unit/net/cookie.rs index 8d94f9cba70..e96ebdd4fbd 100644 --- a/tests/unit/net/cookie.rs +++ b/tests/unit/net/cookie.rs @@ -2,8 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -extern crate cookie as cookie_rs; - +use cookie_rs; use net::cookie::Cookie; use net::cookie_storage::CookieStorage; use net_traits::CookieSource; diff --git a/tests/unit/net/data_loader.rs b/tests/unit/net/data_loader.rs index 663d9390a40..2f688fa42ae 100644 --- a/tests/unit/net/data_loader.rs +++ b/tests/unit/net/data_loader.rs @@ -2,17 +2,14 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -extern crate hyper; -extern crate hyper_serde; - +use hyper::header::ContentType; +use hyper::mime::{Attr, Mime, SubLevel, TopLevel, Value}; use hyper_serde::Serde; use ipc_channel::ipc; use msg::constellation_msg::{PipelineId, ReferrerPolicy}; use net_traits::{LoadContext, LoadData, LoadOrigin, NetworkError}; use net_traits::LoadConsumer::Channel; use net_traits::ProgressMsg::{Done, Payload}; -use self::hyper::header::ContentType; -use self::hyper::mime::{Attr, Mime, SubLevel, TopLevel, Value}; use url::Url; struct DataLoadTest; From 7a311ea9d0904ab0f3ecb3eb6284ba82bd22d42b Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 3 Nov 2016 11:40:10 +0100 Subject: [PATCH 3/6] Make the data_loader test more efficient and readable. --- tests/unit/net/data_loader.rs | 41 ++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/tests/unit/net/data_loader.rs b/tests/unit/net/data_loader.rs index 2f688fa42ae..bbbb4eb9938 100644 --- a/tests/unit/net/data_loader.rs +++ b/tests/unit/net/data_loader.rs @@ -10,6 +10,7 @@ use msg::constellation_msg::{PipelineId, ReferrerPolicy}; use net_traits::{LoadContext, LoadData, LoadOrigin, NetworkError}; use net_traits::LoadConsumer::Channel; use net_traits::ProgressMsg::{Done, Payload}; +use std::ops::Deref; use url::Url; struct DataLoadTest; @@ -29,8 +30,8 @@ impl LoadOrigin for DataLoadTest { #[cfg(test)] fn assert_parse(url: &'static str, content_type: Option, - charset: Option, - data: Option>) { + charset: Option<&str>, + data: Option<&[u8]>) { use net::data_loader::load; use net::mime_classifier::MimeClassifier; use net::resource_thread::CancellationListener; @@ -45,7 +46,7 @@ fn assert_parse(url: &'static str, let response = start_port.recv().unwrap(); assert_eq!(&response.metadata.content_type.map(Serde::into_inner), &content_type); - assert_eq!(&response.metadata.charset, &charset); + assert_eq!(response.metadata.charset.as_ref().map(String::deref), charset); let progress = response.progress_port.recv().unwrap(); @@ -54,7 +55,10 @@ fn assert_parse(url: &'static str, assert_eq!(progress, Done(Err(NetworkError::Internal("invalid data uri".to_owned())))); } Some(dat) => { - assert_eq!(progress, Payload(dat)); + match progress { + Payload(d) => assert_eq!(d, dat), + _ => panic!(), + } assert_eq!(response.progress_port.recv().unwrap(), Done(Ok(()))); } } @@ -71,7 +75,8 @@ fn plain() { "data:,hello%20world", Some(ContentType(Mime(TopLevel::Text, SubLevel::Plain, vec!((Attr::Charset, Value::Ext("us-ascii".to_owned())))))), - Some("US-ASCII".to_owned()), Some(b"hello world".iter().map(|&x| x).collect())); + Some("US-ASCII"), + Some(b"hello world")); } #[test] @@ -80,16 +85,18 @@ fn plain_ct() { "data:text/plain,hello", Some(ContentType(Mime(TopLevel::Text, SubLevel::Plain, vec!()))), None, - Some(b"hello".iter().map(|&x| x).collect())); + Some(b"hello")); } #[test] fn plain_charset() { - assert_parse("data:text/plain;charset=latin1,hello", + assert_parse( + "data:text/plain;charset=latin1,hello", Some(ContentType(Mime(TopLevel::Text, SubLevel::Plain, vec!((Attr::Charset, Value::Ext("latin1".to_owned())))))), - Some("latin1".to_owned()), Some(b"hello".iter().map(|&x| x).collect())); + Some("latin1"), + Some(b"hello")); } #[test] @@ -99,7 +106,8 @@ fn plain_only_charset() { Some(ContentType(Mime(TopLevel::Text, SubLevel::Plain, vec!((Attr::Charset, Value::Utf8))))), - Some("utf-8".to_owned()), Some(b"hello".iter().map(|&x| x).collect())); + Some("utf-8"), + Some(b"hello")); } #[test] @@ -109,22 +117,25 @@ fn base64() { Some(ContentType(Mime(TopLevel::Text, SubLevel::Plain, vec!((Attr::Charset, Value::Ext("us-ascii".to_owned())))))), - Some("US-ASCII".to_owned()), Some(vec!(0x0B, 0xAD, 0xBE, 0xEF))); + Some("US-ASCII"), + Some(&[0x0B, 0xAD, 0xBE, 0xEF])); } #[test] fn base64_ct() { - assert_parse("data:application/octet-stream;base64,C62+7w==", + assert_parse( + "data:application/octet-stream;base64,C62+7w==", Some(ContentType(Mime(TopLevel::Application, SubLevel::Ext("octet-stream".to_owned()), vec!()))), None, - Some(vec!(0x0B, 0xAD, 0xBE, 0xEF))); + Some(&[0x0B, 0xAD, 0xBE, 0xEF])); } #[test] fn base64_charset() { - assert_parse("data:text/plain;charset=koi8-r;base64,8PLl9+XkIO3l5Pfl5A==", + assert_parse( + "data:text/plain;charset=koi8-r;base64,8PLl9+XkIO3l5Pfl5A==", Some(ContentType(Mime(TopLevel::Text, SubLevel::Plain, vec!((Attr::Charset, Value::Ext("koi8-r".to_owned())))))), - Some("koi8-r".to_owned()), - Some(vec!(0xF0, 0xF2, 0xE5, 0xF7, 0xE5, 0xE4, 0x20, 0xED, 0xE5, 0xE4, 0xF7, 0xE5, 0xE4))); + Some("koi8-r"), + Some(&[0xF0, 0xF2, 0xE5, 0xF7, 0xE5, 0xE4, 0x20, 0xED, 0xE5, 0xE4, 0xF7, 0xE5, 0xE4])); } From f304151fe2e0c2ca6538753aa4156459ffd37d99 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 3 Nov 2016 11:32:36 +0100 Subject: [PATCH 4/6] Rewrite the data_loader test with fetch. --- tests/unit/net/data_loader.rs | 79 +++++++++++++++-------------------- 1 file changed, 34 insertions(+), 45 deletions(-) diff --git a/tests/unit/net/data_loader.rs b/tests/unit/net/data_loader.rs index bbbb4eb9938..8dd833d3818 100644 --- a/tests/unit/net/data_loader.rs +++ b/tests/unit/net/data_loader.rs @@ -2,65 +2,54 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +use fetch_sync; use hyper::header::ContentType; use hyper::mime::{Attr, Mime, SubLevel, TopLevel, Value}; use hyper_serde::Serde; -use ipc_channel::ipc; -use msg::constellation_msg::{PipelineId, ReferrerPolicy}; -use net_traits::{LoadContext, LoadData, LoadOrigin, NetworkError}; -use net_traits::LoadConsumer::Channel; -use net_traits::ProgressMsg::{Done, Payload}; +use net_traits::{FetchMetadata, FilteredMetadata, NetworkError}; +use net_traits::request::{Origin, Request}; +use net_traits::response::ResponseBody; use std::ops::Deref; use url::Url; -struct DataLoadTest; - -impl LoadOrigin for DataLoadTest { - fn referrer_url(&self) -> Option { - None - } - fn referrer_policy(&self) -> Option { - None - } - fn pipeline_id(&self) -> Option { - None - } -} - #[cfg(test)] fn assert_parse(url: &'static str, content_type: Option, charset: Option<&str>, data: Option<&[u8]>) { - use net::data_loader::load; - use net::mime_classifier::MimeClassifier; - use net::resource_thread::CancellationListener; - use std::sync::Arc; + let url = Url::parse(url).unwrap(); + let origin = Origin::Origin(url.origin()); + let request = Request::new(url, Some(origin), false, None); - let (start_chan, start_port) = ipc::channel().unwrap(); - let classifier = Arc::new(MimeClassifier::new()); - load(LoadData::new(LoadContext::Browsing, Url::parse(url).unwrap(), &DataLoadTest), - Channel(start_chan), - classifier, CancellationListener::new(None)); - - let response = start_port.recv().unwrap(); - assert_eq!(&response.metadata.content_type.map(Serde::into_inner), - &content_type); - assert_eq!(response.metadata.charset.as_ref().map(String::deref), charset); - - let progress = response.progress_port.recv().unwrap(); + let response = fetch_sync(request, None); match data { - None => { - assert_eq!(progress, Done(Err(NetworkError::Internal("invalid data uri".to_owned())))); - } - Some(dat) => { - match progress { - Payload(d) => assert_eq!(d, dat), + Some(data) => { + assert!(!response.is_network_error()); + assert_eq!(response.headers.len(), 1); + + let header_content_type = response.headers.get::(); + assert_eq!(header_content_type, content_type.as_ref()); + + let metadata = match response.metadata() { + Ok(FetchMetadata::Filtered { filtered: FilteredMetadata::Transparent(m), .. }) => m, + result => panic!(result), + }; + assert_eq!(metadata.content_type.map(Serde::into_inner), content_type); + assert_eq!(metadata.charset.as_ref().map(String::deref), charset); + + let resp_body = response.body.lock().unwrap(); + match *resp_body { + ResponseBody::Done(ref val) => { + assert_eq!(val, &data); + }, _ => panic!(), } - assert_eq!(response.progress_port.recv().unwrap(), Done(Ok(()))); - } + }, + None => { + assert!(response.is_network_error()); + assert_eq!(response.metadata().err(), Some(NetworkError::Internal("Decoding data URL failed".to_owned()))); + }, } } @@ -74,7 +63,7 @@ fn plain() { assert_parse( "data:,hello%20world", Some(ContentType(Mime(TopLevel::Text, SubLevel::Plain, - vec!((Attr::Charset, Value::Ext("us-ascii".to_owned())))))), + vec!((Attr::Charset, Value::Ext("US-ASCII".to_owned())))))), Some("US-ASCII"), Some(b"hello world")); } @@ -116,7 +105,7 @@ fn base64() { "data:;base64,C62+7w==", Some(ContentType(Mime(TopLevel::Text, SubLevel::Plain, - vec!((Attr::Charset, Value::Ext("us-ascii".to_owned())))))), + vec!((Attr::Charset, Value::Ext("US-ASCII".to_owned())))))), Some("US-ASCII"), Some(&[0x0B, 0xAD, 0xBE, 0xEF])); } From bb9b0b3467a773041430dcf11edf1fdec7385a73 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 3 Nov 2016 11:33:52 +0100 Subject: [PATCH 5/6] Move the test_fetch_data test. There is no longer any point in keeping it apart from the other data URL tests. --- tests/unit/net/data_loader.rs | 9 +++++++++ tests/unit/net/fetch.rs | 25 ------------------------- 2 files changed, 9 insertions(+), 25 deletions(-) diff --git a/tests/unit/net/data_loader.rs b/tests/unit/net/data_loader.rs index 8dd833d3818..91edcdf619c 100644 --- a/tests/unit/net/data_loader.rs +++ b/tests/unit/net/data_loader.rs @@ -77,6 +77,15 @@ fn plain_ct() { Some(b"hello")); } +#[test] +fn plain_html() { + assert_parse( + "data:text/html,

Servo

", + Some(ContentType(Mime(TopLevel::Text, SubLevel::Html, vec!()))), + None, + Some(b"

Servo

")); +} + #[test] fn plain_charset() { assert_parse( diff --git a/tests/unit/net/fetch.rs b/tests/unit/net/fetch.rs index 0be39b1710f..38b1c858c71 100644 --- a/tests/unit/net/fetch.rs +++ b/tests/unit/net/fetch.rs @@ -103,31 +103,6 @@ fn test_fetch_aboutblank() { assert!(*fetch_response.body.lock().unwrap() == ResponseBody::Done(vec![])); } -#[test] -fn test_fetch_data() { - let url = Url::parse("data:text/html,

Servo

").unwrap(); - let origin = Origin::Origin(url.origin()); - let request = Request::new(url, Some(origin), false, None); - let expected_resp_body = "

Servo

".to_owned(); - let fetch_response = fetch_sync(request, None); - - assert!(!fetch_response.is_network_error()); - assert_eq!(fetch_response.headers.len(), 1); - let content_type: &ContentType = fetch_response.headers.get().unwrap(); - assert!(**content_type == Mime(TopLevel::Text, SubLevel::Html, vec![])); - let resp_body = fetch_response.body.lock().unwrap(); - - match *resp_body { - ResponseBody::Done(ref val) => { - assert_eq!(val, &expected_resp_body.into_bytes()); - } - ResponseBody::Receiving(_) => { - panic!(); - }, - ResponseBody::Empty => panic!(), - } -} - #[test] fn test_fetch_blob() { use ipc_channel::ipc; From 6fcbc5b3eabe54ec2bc00c774246eee56092f09b Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 3 Nov 2016 11:39:34 +0100 Subject: [PATCH 6/6] Make the data_loader module private. --- components/net/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/net/lib.rs b/components/net/lib.rs index 4ca85975344..27340b26d24 100644 --- a/components/net/lib.rs +++ b/components/net/lib.rs @@ -56,7 +56,7 @@ pub mod connector; pub mod content_blocker; pub mod cookie; pub mod cookie_storage; -pub mod data_loader; +mod data_loader; pub mod file_loader; pub mod filemanager_thread; pub mod hsts;