Additional fetch tests (#3983)

* First bunch of tests

* Dapps zip tests

dapps/src/tests/fetch.rs

@@ -17,7 +17,8 @@
 use devtools::http_client;
 use rustc_serialize::hex::FromHex;
 use tests::helpers::{
-	serve_with_registrar, serve_with_registrar_and_sync, serve_with_registrar_and_fetch, serve_with_fetch,
+	serve_with_registrar, serve_with_registrar_and_sync, serve_with_fetch,
+	serve_with_registrar_and_fetch, serve_with_registrar_and_fetch_and_threads,
 	request, assert_security_headers_for_embed,
 };
 
@@ -128,6 +129,70 @@ fn should_return_error_for_invalid_dapp_zip() {
 	assert_security_headers_for_embed(&response.headers);
 }
 
+#[test]
+fn should_return_fetched_dapp_content() {
+	// given
+	let (server, fetch, registrar) = serve_with_registrar_and_fetch();
+	let gavcoin = GAVCOIN_DAPP.from_hex().unwrap();
+	registrar.set_result(
+		"9c94e154dab8acf859b30ee80fc828fb1d38359d938751b65db71d460588d82a".parse().unwrap(),
+		Ok(gavcoin.clone())
+	);
+	fetch.set_response(include_bytes!("../../res/gavcoin.zip"));
+
+	// when
+	let response1 = http_client::request(server.addr(),
+		"\
+			GET /index.html HTTP/1.1\r\n\
+			Host: 9c94e154dab8acf859b30ee80fc828fb1d38359d938751b65db71d460588d82a.parity\r\n\
+			Connection: close\r\n\
+			\r\n\
+		"
+	);
+	let response2 = http_client::request(server.addr(),
+		"\
+			GET /manifest.json HTTP/1.1\r\n\
+			Host: 9c94e154dab8acf859b30ee80fc828fb1d38359d938751b65db71d460588d82a.parity\r\n\
+			Connection: close\r\n\
+			\r\n\
+		"
+	);
+
+	// then
+	assert_eq!(registrar.calls.lock().len(), 4);
+
+	fetch.assert_requested("https://codeload.github.com/gavofyork/gavcoin/zip/9faf32e1e3845e237cc6efd27187cee13b3b99db");
+	fetch.assert_no_more_requests();
+
+	response1.assert_status("HTTP/1.1 200 OK");
+	assert_security_headers_for_embed(&response1.headers);
+	assert_eq!(
+		response1.body,
+		r#"18
+<h1>Hello Gavcoin!</h1>
+
+"#
+	);
+
+	response2.assert_status("HTTP/1.1 200 OK");
+	assert_security_headers_for_embed(&response2.headers);
+	assert_eq!(
+		response2.body,
+		r#"BE
+{
+  "id": "9c94e154dab8acf859b30ee80fc828fb1d38359d938751b65db71d460588d82a",
+  "name": "Gavcoin",
+  "description": "Gavcoin",
+  "version": "1.0.0",
+  "author": "",
+  "iconUrl": "icon.png"
+}
+0
+
+"#
+	);
+}
+
 #[test]
 fn should_return_fetched_content() {
 	// given
@@ -187,6 +252,52 @@ fn should_cache_content() {
 	response.assert_status("HTTP/1.1 200 OK");
 }
 
+#[test]
+fn should_not_request_content_twice() {
+	use std::thread;
+
+	// given
+	let (server, fetch, registrar) = serve_with_registrar_and_fetch_and_threads(true);
+	let gavcoin = GAVCOIN_ICON.from_hex().unwrap();
+	registrar.set_result(
+		"2be00befcf008bc0e7d9cdefc194db9c75352e8632f48498b5a6bfce9f02c88e".parse().unwrap(),
+		Ok(gavcoin.clone())
+	);
+	let request_str = "\
+		GET / HTTP/1.1\r\n\
+		Host: 2be00befcf008bc0e7d9cdefc194db9c75352e8632f48498b5a6bfce9f02c88e.parity\r\n\
+		Connection: close\r\n\
+		\r\n\
+	";
+	let fire_request = || {
+		let addr = server.addr().to_owned();
+		let req = request_str.to_owned();
+		thread::spawn(move || {
+			http_client::request(&addr, &req)
+		})
+	};
+	let control = fetch.manual();
+
+	// when
+
+	// Fire two requests at the same time
+	let r1 = fire_request();
+	let r2 = fire_request();
+
+	// wait for single request in fetch, the second one should go into waiting state.
+	control.wait_for_requests(1);
+	control.respond();
+
+	let response1 = r1.join().unwrap();
+	let response2 = r2.join().unwrap();
+
+	// then
+	fetch.assert_requested("https://raw.githubusercontent.com/ethcore/dapp-assets/b88e983abaa1a6a6345b8d9448c15b117ddb540e/tokens/gavcoin-64x64.png");
+	fetch.assert_no_more_requests();
+	response1.assert_status("HTTP/1.1 200 OK");
+	response2.assert_status("HTTP/1.1 200 OK");
+}
+
 #[test]
 fn should_stream_web_content() {
 	// given
@@ -195,7 +306,7 @@ fn should_stream_web_content() {
 	// when
 	let response = request(server,
 		"\
-			GET /web/token/https/ethcore.io/ HTTP/1.1\r\n\
+			GET /web/token/https/parity.io/ HTTP/1.1\r\n\
 			Host: localhost:8080\r\n\
 			Connection: close\r\n\
 			\r\n\
@@ -206,7 +317,7 @@ fn should_stream_web_content() {
 	response.assert_status("HTTP/1.1 200 OK");
 	assert_security_headers_for_embed(&response.headers);
 
-	fetch.assert_requested("https://ethcore.io/");
+	fetch.assert_requested("https://parity.io/");
 	fetch.assert_no_more_requests();
 }
 
@@ -218,7 +329,7 @@ fn should_return_error_on_invalid_token() {
 	// when
 	let response = request(server,
 		"\
-			GET /web/invalidtoken/https/ethcore.io/ HTTP/1.1\r\n\
+			GET /web/invalidtoken/https/parity.io/ HTTP/1.1\r\n\
 			Host: localhost:8080\r\n\
 			Connection: close\r\n\
 			\r\n\
@@ -240,7 +351,7 @@ fn should_return_error_on_invalid_protocol() {
 	// when
 	let response = request(server,
 		"\
-			GET /web/token/ftp/ethcore.io/ HTTP/1.1\r\n\
+			GET /web/token/ftp/parity.io/ HTTP/1.1\r\n\
 			Host: localhost:8080\r\n\
 			Connection: close\r\n\
 			\r\n\
@@ -253,3 +364,73 @@ fn should_return_error_on_invalid_protocol() {
 
 	fetch.assert_no_more_requests();
 }
+
+#[test]
+fn should_redirect_if_trailing_slash_is_missing() {
+	// given
+	let (server, fetch) = serve_with_fetch("token");
+
+	// when
+	let response = request(server,
+		"\
+			GET /web/token/https/parity.io HTTP/1.1\r\n\
+			Host: localhost:8080\r\n\
+			Connection: close\r\n\
+			\r\n\
+		"
+	);
+
+	// then
+	response.assert_status("HTTP/1.1 302 Found");
+	response.assert_header("Location", "/web/token/https/parity.io/");
+
+	fetch.assert_no_more_requests();
+}
+
+#[test]
+fn should_disallow_non_get_requests() {
+	// given
+	let (server, fetch) = serve_with_fetch("token");
+
+	// when
+	let response = request(server,
+		"\
+			POST /token/https/parity.io/ HTTP/1.1\r\n\
+			Host: web.parity\r\n\
+			Content-Type: application/json\r\n\
+			Connection: close\r\n\
+			\r\n\
+			123\r\n\
+			\r\n\
+		"
+	);
+
+	// then
+	response.assert_status("HTTP/1.1 405 Method Not Allowed");
+	assert_security_headers_for_embed(&response.headers);
+
+	fetch.assert_no_more_requests();
+}
+
+#[test]
+fn should_fix_absolute_requests_based_on_referer() {
+	// given
+	let (server, fetch) = serve_with_fetch("token");
+
+	// when
+	let response = request(server,
+		"\
+			GET /styles.css HTTP/1.1\r\n\
+			Host: localhost:8080\r\n\
+			Connection: close\r\n\
+			Referer: http://localhost:8080/web/token/https/parity.io/\r\n\
+			\r\n\
+		"
+	);
+
+	// then
+	response.assert_status("HTTP/1.1 302 Found");
+	response.assert_header("Location", "/web/token/https/parity.io/styles.css");
+
+	fetch.assert_no_more_requests();
+}

dapps/src/tests/helpers/fetch.rs

@@ -14,21 +14,71 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-use std::{io, thread};
+use std::{io, thread, time};
-use std::sync::Arc;
+use std::sync::{atomic, mpsc, Arc};
-use std::sync::atomic::{self, AtomicUsize};
 use util::Mutex;
 
 use futures::{self, Future};
 use fetch::{self, Fetch};
 
+pub struct FetchControl {
+	sender: mpsc::Sender<()>,
+	fetch: FakeFetch,
+}
+
+impl FetchControl {
+	pub fn respond(self) {
+		self.sender.send(())
+			.expect("Fetch cannot be finished without sending a response at least once.");
+	}
+
+	pub fn wait_for_requests(&self, len: usize) {
+		const MAX_TIMEOUT_MS: u64 = 5000;
+		const ATTEMPTS: u64 = 10;
+		let mut attempts_left = ATTEMPTS;
+		loop {
+			let current = self.fetch.requested.lock().len();
+
+			if current == len {
+				break;
+			} else if attempts_left == 0 {
+				panic!(
+					"Timeout reached when waiting for pending requests. Expected: {}, current: {}",
+					len, current
+				);
+			} else {
+				attempts_left -= 1;
+				// Should we handle spurious timeouts better?
+				thread::park_timeout(time::Duration::from_millis(MAX_TIMEOUT_MS / ATTEMPTS));
+			}
+		}
+	}
+}
+
 #[derive(Clone, Default)]
 pub struct FakeFetch {
-	asserted: Arc<AtomicUsize>,
+	manual: Arc<Mutex<Option<mpsc::Receiver<()>>>>,
+	response: Arc<Mutex<Option<&'static [u8]>>>,
+	asserted: Arc<atomic::AtomicUsize>,
 	requested: Arc<Mutex<Vec<String>>>,
 }
 
 impl FakeFetch {
+	pub fn set_response(&self, data: &'static [u8]) {
+		*self.response.lock() = Some(data);
+	}
+
+	pub fn manual(&self) -> FetchControl {
+		assert!(self.manual.lock().is_none(), "Only one manual control may be active.");
+		let (tx, rx) = mpsc::channel();
+		*self.manual.lock() = Some(rx);
+
+		FetchControl {
+			sender: tx,
+			fetch: self.clone(),
+		}
+	}
+
 	pub fn assert_requested(&self, url: &str) {
 		let requests = self.requested.lock();
 		let idx = self.asserted.fetch_add(1, atomic::Ordering::SeqCst);
@@ -52,10 +102,18 @@ impl Fetch for FakeFetch {
 
 	fn fetch_with_abort(&self, url: &str, _abort: fetch::Abort) -> Self::Result {
 		self.requested.lock().push(url.into());
+		let manual = self.manual.clone();
+		let response = self.response.clone();
 
 		let (tx, rx) = futures::oneshot();
 		thread::spawn(move || {
-			let cursor = io::Cursor::new(b"Some content");
+			if let Some(rx) = manual.lock().take() {
+				// wait for manual resume
+				let _ = rx.recv();
+			}
+
+			let data = response.lock().take().unwrap_or(b"Some content");
+			let cursor = io::Cursor::new(data);
 			tx.complete(fetch::Response::from_reader(cursor));
 		});
 

dapps/src/tests/helpers/mod.rs

@@ -42,7 +42,7 @@ fn init_logger() {
 	}
 }
 
-pub fn init_server<F, B>(hosts: Option<Vec<String>>, process: F) -> (Server, Arc<FakeRegistrar>) where
+pub fn init_server<F, B>(hosts: Option<Vec<String>>, process: F, remote: Remote) -> (Server, Arc<FakeRegistrar>) where
 	F: FnOnce(ServerBuilder) -> ServerBuilder<B>,
 	B: Fetch,
 {
@@ -51,7 +51,7 @@ pub fn init_server<F, B>(hosts: Option<Vec<String>>, process: F) -> (Server, Arc
 	let mut dapps_path = env::temp_dir();
 	dapps_path.push("non-existent-dir-to-prevent-fs-files-from-loading");
 	let server = process(ServerBuilder::new(
-		dapps_path.to_str().unwrap().into(), registrar.clone(), Remote::new_sync()
+		dapps_path.to_str().unwrap().into(), registrar.clone(), remote,
 	))
 		.signer_address(Some(("127.0.0.1".into(), SIGNER_PORT)))
 		.start_unsecured_http(&"127.0.0.1:0".parse().unwrap(), hosts).unwrap();
@@ -72,25 +72,29 @@ pub fn serve_with_auth(user: &str, pass: &str) -> Server {
 }
 
 pub fn serve_hosts(hosts: Option<Vec<String>>) -> Server {
-	init_server(hosts, |builder| builder).0
+	init_server(hosts, |builder| builder, Remote::new_sync()).0
 }
 
 pub fn serve_with_registrar() -> (Server, Arc<FakeRegistrar>) {
-	init_server(None, |builder| builder)
+	init_server(None, |builder| builder, Remote::new_sync())
 }
 
 pub fn serve_with_registrar_and_sync() -> (Server, Arc<FakeRegistrar>) {
 	init_server(None, |builder| {
 		builder.sync_status(Arc::new(|| true))
-	})
+	}, Remote::new_sync())
 }
 
 pub fn serve_with_registrar_and_fetch() -> (Server, FakeFetch, Arc<FakeRegistrar>) {
+	serve_with_registrar_and_fetch_and_threads(false)
+}
+
+pub fn serve_with_registrar_and_fetch_and_threads(multi_threaded: bool) -> (Server, FakeFetch, Arc<FakeRegistrar>) {
 	let fetch = FakeFetch::default();
 	let f = fetch.clone();
 	let (server, reg) = init_server(None, move |builder| {
 		builder.fetch(f.clone())
-	});
+	}, if multi_threaded { Remote::new_thread_per_future() } else { Remote::new_sync() });
 
 	(server, fetch, reg)
 }
@@ -102,13 +106,13 @@ pub fn serve_with_fetch(web_token: &'static str) -> (Server, FakeFetch) {
 		builder
 			.fetch(f.clone())
 			.web_proxy_tokens(Arc::new(move |token| &token == web_token))
-	});
+	}, Remote::new_sync());
 
 	(server, fetch)
 }
 
 pub fn serve() -> Server {
-	init_server(None, |builder| builder).0
+	init_server(None, |builder| builder, Remote::new_sync()).0
 }
 
 pub fn request(server: Server, request: &str) -> http_client::Response {

devtools/src/http_client.rs

@@ -28,6 +28,11 @@ pub struct Response {
 }
 
 impl Response {
+	pub fn assert_header(&self, header: &str, value: &str) {
+		let header = format!("{}: {}", header, value);
+		assert!(self.headers.iter().find(|h| *h == &header).is_some(), "Couldn't find header {} in {:?}", header, &self.headers)
+	}
+
 	pub fn assert_status(&self, status: &str) {
 		assert_eq!(self.status, status.to_owned(), "Got unexpected code. Body: {:?}", self.body);
 	}
@@ -75,7 +80,7 @@ fn connect(address: &SocketAddr) -> TcpStream {
 
 pub fn request(address: &SocketAddr, request: &str) -> Response {
 	let mut req = connect(address);
-	req.set_read_timeout(Some(Duration::from_secs(1))).unwrap();
+	req.set_read_timeout(Some(Duration::from_secs(2))).unwrap();
 	req.write_all(request.as_bytes()).unwrap();
 
 	let mut response = String::new();

util/reactor/src/lib.rs

@@ -67,6 +67,7 @@ impl EventLoop {
 enum Mode {
 	Tokio(TokioRemote),
 	Sync,
+	ThreadPerFuture,
 }
 
 #[derive(Clone)]
@@ -82,6 +83,14 @@ impl Remote {
 		}
 	}
 
+	/// Spawns a new thread for each future (use only for tests).
+	pub fn new_thread_per_future() -> Self {
+		Remote {
+			inner: Mode::ThreadPerFuture,
+		}
+	}
+
+
 	/// Spawn a future to this event loop
 	pub fn spawn<R>(&self, r: R) where
         R: IntoFuture<Item=(), Error=()> + Send + 'static,
@@ -92,6 +101,11 @@ impl Remote {
 			Mode::Sync => {
 				let _= r.into_future().wait();
 			},
+			Mode::ThreadPerFuture => {
+				thread::spawn(move || {
+					let _= r.into_future().wait();
+				});
+			},
 		}
 	}
 
@@ -106,6 +120,11 @@ impl Remote {
 			Mode::Sync => {
 				let _ = f().into_future().wait();
 			},
+			Mode::ThreadPerFuture => {
+				thread::spawn(move || {
+					let _= f().into_future().wait();
+				});
+			},
 		}
 	}
 
@@ -128,6 +147,11 @@ impl Remote {
 			Mode::Sync => {
 				let _ = f().into_future().wait();
 			},
+			Mode::ThreadPerFuture => {
+				thread::spawn(move || {
+					let _ = f().into_future().wait();
+				});
+			},
 		}
 	}
 }