From 3067a8de3e135618ba00e9f493777d330a7da87a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 28 Dec 2016 13:45:25 +0100 Subject: [PATCH] Additional fetch tests (#3983) * First bunch of tests * Dapps zip tests --- dapps/res/gavcoin.zip | Bin 0 -> 434 bytes dapps/src/tests/fetch.rs | 191 ++++++++++++++++++++++++++++++- dapps/src/tests/helpers/fetch.rs | 68 ++++++++++- dapps/src/tests/helpers/mod.rs | 20 ++-- devtools/src/http_client.rs | 7 +- util/reactor/src/lib.rs | 24 ++++ 6 files changed, 291 insertions(+), 19 deletions(-) create mode 100644 dapps/res/gavcoin.zip diff --git a/dapps/res/gavcoin.zip b/dapps/res/gavcoin.zip new file mode 100644 index 0000000000000000000000000000000000000000..3ced8c5c1d8cf7d23d2f0db1b60fc4443b970e31 GIT binary patch literal 434 zcmWIWW@h1H0D;ZXb39x32=A5vvO$=OL53kSFD11?FQX(kCp3hUfmvuuas&vMR&X;g zvbG-G*9OGA zK(lib^D@&?i%ayfiu3cp#(M*e24OVgS3e612sq^vxG5;W%U{R)-1#%Xml!#psyh4_7_!3qfoG*?2*!{uFM^J0MJp?Mx? XBo>bcc(byBEN243#X$Neh{FH?UCDKQ literal 0 HcmV?d00001 diff --git a/dapps/src/tests/fetch.rs b/dapps/src/tests/fetch.rs index 56c96faff..f98fd473d 100644 --- a/dapps/src/tests/fetch.rs +++ b/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 +

Hello Gavcoin!

+ +"# + ); + + 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(); +} diff --git a/dapps/src/tests/helpers/fetch.rs b/dapps/src/tests/helpers/fetch.rs index 4f62be656..44e77a360 100644 --- a/dapps/src/tests/helpers/fetch.rs +++ b/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 . -use std::{io, thread}; -use std::sync::Arc; -use std::sync::atomic::{self, AtomicUsize}; +use std::{io, thread, time}; +use std::sync::{atomic, mpsc, Arc}; 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, + manual: Arc>>>, + response: Arc>>, + asserted: Arc, requested: Arc>>, } 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)); }); diff --git a/dapps/src/tests/helpers/mod.rs b/dapps/src/tests/helpers/mod.rs index cacfff02d..d3f97b35b 100644 --- a/dapps/src/tests/helpers/mod.rs +++ b/dapps/src/tests/helpers/mod.rs @@ -42,7 +42,7 @@ fn init_logger() { } } -pub fn init_server(hosts: Option>, process: F) -> (Server, Arc) where +pub fn init_server(hosts: Option>, process: F, remote: Remote) -> (Server, Arc) where F: FnOnce(ServerBuilder) -> ServerBuilder, B: Fetch, { @@ -51,7 +51,7 @@ pub fn init_server(hosts: Option>, 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>) -> Server { - init_server(hosts, |builder| builder).0 + init_server(hosts, |builder| builder, Remote::new_sync()).0 } pub fn serve_with_registrar() -> (Server, Arc) { - init_server(None, |builder| builder) + init_server(None, |builder| builder, Remote::new_sync()) } pub fn serve_with_registrar_and_sync() -> (Server, Arc) { init_server(None, |builder| { builder.sync_status(Arc::new(|| true)) - }) + }, Remote::new_sync()) } pub fn serve_with_registrar_and_fetch() -> (Server, FakeFetch, Arc) { + serve_with_registrar_and_fetch_and_threads(false) +} + +pub fn serve_with_registrar_and_fetch_and_threads(multi_threaded: bool) -> (Server, FakeFetch, Arc) { 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 { diff --git a/devtools/src/http_client.rs b/devtools/src/http_client.rs index 60a587e02..cd26b500d 100644 --- a/devtools/src/http_client.rs +++ b/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(); diff --git a/util/reactor/src/lib.rs b/util/reactor/src/lib.rs index 570d64af4..dc3339376 100644 --- a/util/reactor/src/lib.rs +++ b/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(&self, r: R) where R: IntoFuture + 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(); + }); + }, } } }