From 2789824a51f06eacbf833a7c55ef1b32a8f55669 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 31 Aug 2016 16:53:22 +0200 Subject: [PATCH 1/7] Much nicer error pages --- dapps/src/apps/fetcher.rs | 10 ++++----- dapps/src/error_tpl.html | 22 +++++++++++++++++++ dapps/src/handlers/content.rs | 33 ++++++++++++++++++++-------- dapps/src/handlers/fetch.rs | 32 +++++++++++++++++---------- dapps/src/router/auth.rs | 5 ++--- dapps/src/router/host_validation.rs | 12 +++++----- signer/src/ws_server/session.rs | 34 +++++++++++++++++++++++++---- 7 files changed, 108 insertions(+), 40 deletions(-) create mode 100644 dapps/src/error_tpl.html diff --git a/dapps/src/apps/fetcher.rs b/dapps/src/apps/fetcher.rs index e31aae55d..6223fb5e8 100644 --- a/dapps/src/apps/fetcher.rs +++ b/dapps/src/apps/fetcher.rs @@ -98,13 +98,11 @@ impl AppFetcher { }, // App is already being fetched Some(&mut ContentStatus::Fetching(_)) => { - (None, Box::new(ContentHandler::html( + (None, Box::new(ContentHandler::error_with_refresh( StatusCode::ServiceUnavailable, - format!( - "{}{}", - "", - "

This dapp is already being downloaded.

Please wait...

", - ) + "Download In Progress", + "This dapp is already being downloaded. Please wait...", + None, )) as Box) }, // We need to start fetching app diff --git a/dapps/src/error_tpl.html b/dapps/src/error_tpl.html new file mode 100644 index 000000000..227764b9c --- /dev/null +++ b/dapps/src/error_tpl.html @@ -0,0 +1,22 @@ + + + + + + {meta} + {title} + + + +
+
+
+

{title}

+

{message}

+

{details}

+
+
+ {version} +
+ + diff --git a/dapps/src/handlers/content.rs b/dapps/src/handlers/content.rs index 092c417ac..f283fbb6a 100644 --- a/dapps/src/handlers/content.rs +++ b/dapps/src/handlers/content.rs @@ -21,6 +21,8 @@ use hyper::{header, server, Decoder, Encoder, Next}; use hyper::net::HttpStream; use hyper::status::StatusCode; +use util::version; + pub struct ContentHandler { code: StatusCode, content: String, @@ -38,15 +40,6 @@ impl ContentHandler { } } - pub fn forbidden(content: String, mimetype: String) -> Self { - ContentHandler { - code: StatusCode::Forbidden, - content: content, - mimetype: mimetype, - write_pos: 0 - } - } - pub fn not_found(content: String, mimetype: String) -> Self { ContentHandler { code: StatusCode::NotFound, @@ -60,6 +53,28 @@ impl ContentHandler { Self::new(code, content, "text/html".into()) } + pub fn error(code: StatusCode, title: &str, message: &str, details: Option<&str>) -> Self { + Self::html(code, format!( + include_str!("../error_tpl.html"), + title=title, + meta="", + message=message, + details=details.unwrap_or_else(|| ""), + version=version(), + )) + } + + pub fn error_with_refresh(code: StatusCode, title: &str, message: &str, details: Option<&str>) -> Self { + Self::html(code, format!( + include_str!("../error_tpl.html"), + title=title, + meta="", + message=message, + details=details.unwrap_or_else(|| ""), + version=version(), + )) + } + pub fn new(code: StatusCode, content: String, mimetype: String) -> Self { ContentHandler { code: code, diff --git a/dapps/src/handlers/fetch.rs b/dapps/src/handlers/fetch.rs index d4919562a..7cbb2537b 100644 --- a/dapps/src/handlers/fetch.rs +++ b/dapps/src/handlers/fetch.rs @@ -130,16 +130,20 @@ impl server::Handler for ContentFetcherHandler< deadline: Instant::now() + Duration::from_secs(FETCH_TIMEOUT), receiver: receiver, }, - Err(e) => FetchState::Error(ContentHandler::html( + Err(e) => FetchState::Error(ContentHandler::error( StatusCode::BadGateway, - format!("

Error starting dapp download.

{}
", e), + "Unable To Start Dapp Download", + "Could not initialize download of the dapp. It might be a problem with remote server.", + Some(&format!("{}", e)), )), } }, // or return error - _ => FetchState::Error(ContentHandler::html( + _ => FetchState::Error(ContentHandler::error( StatusCode::MethodNotAllowed, - "

Only GET requests are allowed.

".into(), + "Method Not Allowed", + "Only GET requests are allowed.", + None, )), }) } else { None }; @@ -156,10 +160,12 @@ impl server::Handler for ContentFetcherHandler< // Request may time out FetchState::InProgress { ref deadline, .. } if *deadline < Instant::now() => { trace!(target: "dapps", "Fetching dapp failed because of timeout."); - let timeout = ContentHandler::html( + let timeout = ContentHandler::error( StatusCode::GatewayTimeout, - format!("

Could not fetch app bundle within {} seconds.

", FETCH_TIMEOUT), - ); + "Download Timeout", + &format!("Could not fetch dapp bundle within {} seconds.", FETCH_TIMEOUT), + None + ); Self::close_client(&mut self.client); (Some(FetchState::Error(timeout)), Next::write()) }, @@ -175,9 +181,11 @@ impl server::Handler for ContentFetcherHandler< let state = match self.dapp.validate_and_install(path.clone()) { Err(e) => { trace!(target: "dapps", "Error while validating dapp: {:?}", e); - FetchState::Error(ContentHandler::html( + FetchState::Error(ContentHandler::error( StatusCode::BadGateway, - format!("

Downloaded bundle does not contain valid app.

{:?}
", e), + "Invalid Dapp", + "Downloaded bundle does not contain a valid dapp.", + Some(&format!("{:?}", e)) )) }, Ok(manifest) => FetchState::Done(manifest) @@ -188,9 +196,11 @@ impl server::Handler for ContentFetcherHandler< }, Ok(Err(e)) => { warn!(target: "dapps", "Unable to fetch new dapp: {:?}", e); - let error = ContentHandler::html( + let error = ContentHandler::error( StatusCode::BadGateway, - "

There was an error when fetching the dapp.

".into(), + "Download Error", + "There was an error when fetching the dapp.", + Some(&format!("{:?}", e)), ); (Some(FetchState::Error(error)), Next::write()) }, diff --git a/dapps/src/router/auth.rs b/dapps/src/router/auth.rs index d18424a00..596796eed 100644 --- a/dapps/src/router/auth.rs +++ b/dapps/src/router/auth.rs @@ -55,10 +55,9 @@ impl Authorization for HttpBasicAuth { match auth { Access::Denied => { - Authorized::No(Box::new(ContentHandler::new( + Authorized::No(Box::new(ContentHandler::error( status::StatusCode::Unauthorized, - "

Unauthorized

".into(), - "text/html".into(), + "Unauthorized", "You need to provide valid credentials to access this page.", None ))) }, Access::AuthRequired => { diff --git a/dapps/src/router/host_validation.rs b/dapps/src/router/host_validation.rs index e0f974482..d1d651c5d 100644 --- a/dapps/src/router/host_validation.rs +++ b/dapps/src/router/host_validation.rs @@ -16,7 +16,7 @@ use DAPPS_DOMAIN; -use hyper::{server, header}; +use hyper::{server, header, StatusCode}; use hyper::net::HttpStream; use jsonrpc_http_server::{is_host_header_valid}; @@ -38,11 +38,9 @@ pub fn is_valid(request: &server::Request, allowed_hosts: &[String], } pub fn host_invalid_response() -> Box + Send> { - Box::new(ContentHandler::forbidden( - r#" -

Request with disallowed Host header has been blocked.

-

Check the URL in your browser address bar.

- "#.into(), - "text/html".into() + Box::new(ContentHandler::error(StatusCode::Forbidden, + "Current host is disallowed", + "You are trying to access your node using incorrect address.", + Some("Use allowed URL or specify different hosts CLI options.") )) } diff --git a/signer/src/ws_server/session.rs b/signer/src/ws_server/session.rs index e7abea833..34097c24d 100644 --- a/signer/src/ws_server/session.rs +++ b/signer/src/ws_server/session.rs @@ -22,7 +22,7 @@ use std::path::{PathBuf, Path}; use std::sync::Arc; use std::str::FromStr; use jsonrpc_core::IoHandler; -use util::H256; +use util::{H256, version}; #[cfg(feature = "ui")] mod signer { @@ -112,7 +112,12 @@ impl ws::Handler for Session { if !self.skip_origin_validation { if !origin_is_allowed(&self.self_origin, origin) && !(origin.is_none() && origin_is_allowed(&self.self_origin, host)) { warn!(target: "signer", "Blocked connection to Signer API from untrusted origin."); - return Ok(ws::Response::forbidden(format!("You are not allowed to access system ui. Use: http://{}", self.self_origin))); + return Ok(error( + ErrorType::Forbidden, + "URL Blocked", + "You are not allowed to access Trusted Signer using this URL.", + Some(&format!("Use: http://{}", self.self_origin)), + )); } } @@ -121,7 +126,7 @@ impl ws::Handler for Session { // Check authorization if !auth_is_valid(&self.authcodes_path, req.protocols()) { info!(target: "signer", "Unauthorized connection to Signer API blocked."); - return Ok(ws::Response::forbidden("You are not authorized.".into())); + return Ok(error(ErrorType::Forbidden, "Not Authorized", "Request to this API was not authorized.", None)); } let protocols = req.protocols().expect("Existence checked by authorization."); @@ -137,7 +142,7 @@ impl ws::Handler for Session { Ok(signer::handle(req.resource()) .map_or_else( // return 404 not found - || add_headers(ws::Response::not_found("Not found".into()), "text/plain"), + || error(ErrorType::NotFound, "Not found", "Requested file was not found.", None), // or serve the file |f| add_headers(ws::Response::ok(f.content.into()), &f.mime) )) @@ -185,3 +190,24 @@ impl ws::Factory for Factory { } } } + +enum ErrorType { + NotFound, + Forbidden, +} + +fn error(error: ErrorType, title: &str, message: &str, details: Option<&str>) -> ws::Response { + let content = format!( + include_str!("../../../dapps/src/error_tpl.html"), + title=title, + meta="", + message=message, + details=details.unwrap_or(""), + version=version(), + ); + let res = match error { + ErrorType::NotFound => ws::Response::not_found(content), + ErrorType::Forbidden => ws::Response::forbidden(content), + }; + add_headers(res, "text/html") +} From 9c4d31f5489c7f7e45113c31f032c43f640df4b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 1 Sep 2016 10:16:04 +0200 Subject: [PATCH 2/7] Signer errors --- Cargo.lock | 12 +++-- dapps/Cargo.toml | 1 + dapps/src/error_tpl.html | 2 +- dapps/src/lib.rs | 2 + dapps/src/router/mod.rs | 13 +++-- dapps/src/tests/authorization.rs | 2 +- dapps/src/tests/helpers.rs | 50 ++---------------- dapps/src/tests/validation.rs | 23 +++++++- devtools/src/http_client.rs | 64 +++++++++++++++++++++++ devtools/src/lib.rs | 1 + signer/Cargo.toml | 1 + signer/src/lib.rs | 10 ++-- signer/src/tests/mod.rs | 81 +++++++++++++++++++++++++++++ signer/src/ws_server/error_tpl.html | 21 ++++++++ signer/src/ws_server/mod.rs | 9 +++- signer/src/ws_server/session.rs | 14 +++-- 16 files changed, 236 insertions(+), 70 deletions(-) create mode 100644 devtools/src/http_client.rs create mode 100644 signer/src/tests/mod.rs create mode 100644 signer/src/ws_server/error_tpl.html diff --git a/Cargo.lock b/Cargo.lock index 79bb6dba9..b9ad5d6bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -290,6 +290,7 @@ version = "1.4.0" dependencies = [ "clippy 0.0.85 (registry+https://github.com/rust-lang/crates.io-index)", "ethabi 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", + "ethcore-devtools 1.4.0", "ethcore-rpc 1.4.0", "ethcore-util 1.4.0", "hyper 0.9.4 (git+https://github.com/ethcore/hyper)", @@ -455,6 +456,7 @@ version = "1.4.0" dependencies = [ "clippy 0.0.85 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", + "ethcore-devtools 1.4.0", "ethcore-io 1.4.0", "ethcore-rpc 1.4.0", "ethcore-util 1.4.0", @@ -1068,7 +1070,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "parity-dapps" version = "1.4.0" -source = "git+https://github.com/ethcore/parity-ui.git#e4dddf36e7c9fa5c6e746790119c71f67438784a" +source = "git+https://github.com/ethcore/parity-ui.git#be8754c3ccd142c43a20b82e9b1fba416b5b332a" dependencies = [ "aster 0.17.0 (registry+https://github.com/rust-lang/crates.io-index)", "glob 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1082,7 +1084,7 @@ dependencies = [ [[package]] name = "parity-dapps-home" version = "1.4.0" -source = "git+https://github.com/ethcore/parity-ui.git#e4dddf36e7c9fa5c6e746790119c71f67438784a" +source = "git+https://github.com/ethcore/parity-ui.git#be8754c3ccd142c43a20b82e9b1fba416b5b332a" dependencies = [ "parity-dapps 1.4.0 (git+https://github.com/ethcore/parity-ui.git)", ] @@ -1090,7 +1092,7 @@ dependencies = [ [[package]] name = "parity-dapps-signer" version = "1.4.0" -source = "git+https://github.com/ethcore/parity-ui.git#e4dddf36e7c9fa5c6e746790119c71f67438784a" +source = "git+https://github.com/ethcore/parity-ui.git#be8754c3ccd142c43a20b82e9b1fba416b5b332a" dependencies = [ "parity-dapps 1.4.0 (git+https://github.com/ethcore/parity-ui.git)", ] @@ -1098,7 +1100,7 @@ dependencies = [ [[package]] name = "parity-dapps-status" version = "1.4.0" -source = "git+https://github.com/ethcore/parity-ui.git#e4dddf36e7c9fa5c6e746790119c71f67438784a" +source = "git+https://github.com/ethcore/parity-ui.git#be8754c3ccd142c43a20b82e9b1fba416b5b332a" dependencies = [ "parity-dapps 1.4.0 (git+https://github.com/ethcore/parity-ui.git)", ] @@ -1106,7 +1108,7 @@ dependencies = [ [[package]] name = "parity-dapps-wallet" version = "1.4.0" -source = "git+https://github.com/ethcore/parity-ui.git#e4dddf36e7c9fa5c6e746790119c71f67438784a" +source = "git+https://github.com/ethcore/parity-ui.git#be8754c3ccd142c43a20b82e9b1fba416b5b332a" dependencies = [ "parity-dapps 1.4.0 (git+https://github.com/ethcore/parity-ui.git)", ] diff --git a/dapps/Cargo.toml b/dapps/Cargo.toml index 1f5a0c491..f2604f07f 100644 --- a/dapps/Cargo.toml +++ b/dapps/Cargo.toml @@ -23,6 +23,7 @@ serde_macros = { version = "0.7.0", optional = true } zip = { version = "0.1", default-features = false } ethabi = "0.2.1" linked-hash-map = "0.3" +ethcore-devtools = { path = "../devtools" } ethcore-rpc = { path = "../rpc" } ethcore-util = { path = "../util" } parity-dapps = { git = "https://github.com/ethcore/parity-ui.git", version = "1.4" } diff --git a/dapps/src/error_tpl.html b/dapps/src/error_tpl.html index 227764b9c..6551431a6 100644 --- a/dapps/src/error_tpl.html +++ b/dapps/src/error_tpl.html @@ -16,7 +16,7 @@

{details}

- {version} + {version}
diff --git a/dapps/src/lib.rs b/dapps/src/lib.rs index bc54b0f37..e24def0a1 100644 --- a/dapps/src/lib.rs +++ b/dapps/src/lib.rs @@ -61,6 +61,8 @@ extern crate parity_dapps; extern crate ethcore_rpc; extern crate ethcore_util as util; extern crate linked_hash_map; +#[cfg(test)] +extern crate ethcore_devtools as devtools; mod endpoint; mod apps; diff --git a/dapps/src/router/mod.rs b/dapps/src/router/mod.rs index 94d0a0fc0..9326917bf 100644 --- a/dapps/src/router/mod.rs +++ b/dapps/src/router/mod.rs @@ -55,9 +55,16 @@ pub struct Router { impl server::Handler for Router { fn on_request(&mut self, req: server::Request) -> Next { + + // Choose proper handler depending on path / domain + let url = extract_url(&req); + let endpoint = extract_endpoint(&url); + let is_utils = endpoint.1 == SpecialEndpoint::Utils; + // Validate Host header if let Some(ref hosts) = self.allowed_hosts { - if !host_validation::is_valid(&req, hosts, self.endpoints.keys().cloned().collect()) { + let is_valid = is_utils || host_validation::is_valid(&req, hosts, self.endpoints.keys().cloned().collect()); + if !is_valid { self.handler = host_validation::host_invalid_response(); return self.handler.on_request(req); } @@ -70,10 +77,6 @@ impl server::Handler for Router { return self.handler.on_request(req); } - // Choose proper handler depending on path / domain - let url = extract_url(&req); - let endpoint = extract_endpoint(&url); - self.handler = match endpoint { // First check special endpoints (ref path, ref endpoint) if self.special.contains_key(endpoint) => { diff --git a/dapps/src/tests/authorization.rs b/dapps/src/tests/authorization.rs index dceb194b7..e7a70c9cd 100644 --- a/dapps/src/tests/authorization.rs +++ b/dapps/src/tests/authorization.rs @@ -54,7 +54,7 @@ fn should_reject_on_invalid_auth() { // then assert_eq!(response.status, "HTTP/1.1 401 Unauthorized".to_owned()); - assert_eq!(response.body, "15\n

Unauthorized

\n0\n\n".to_owned()); + assert!(response.body.contains("Unauthorized")); assert_eq!(response.headers_raw.contains("WWW-Authenticate"), false); } diff --git a/dapps/src/tests/helpers.rs b/dapps/src/tests/helpers.rs index 84f638b34..1296162d2 100644 --- a/dapps/src/tests/helpers.rs +++ b/dapps/src/tests/helpers.rs @@ -15,16 +15,15 @@ // along with Parity. If not, see . use std::env; -use std::io::{Read, Write}; -use std::str::{self, Lines}; +use std::str; use std::sync::Arc; -use std::net::TcpStream; use rustc_serialize::hex::{ToHex, FromHex}; use ServerBuilder; use Server; use apps::urlhint::ContractClient; use util::{Bytes, Address, Mutex, ToPretty}; +use devtools::http_client; const REGISTRAR: &'static str = "8e4e9b13d4b45cb0befc93c3061b1408f67316b2"; const URLHINT: &'static str = "deadbeefcafe0000000000000000000000000000"; @@ -77,47 +76,6 @@ pub fn serve() -> Server { serve_hosts(None) } -pub struct Response { - pub status: String, - pub headers: Vec, - pub headers_raw: String, - pub body: String, +pub fn request(server: Server, request: &str) -> http_client::Response { + http_client::request(server.addr(), request) } - -pub fn read_block(lines: &mut Lines, all: bool) -> String { - let mut block = String::new(); - loop { - let line = lines.next(); - match line { - None => break, - Some("") if !all => break, - Some(v) => { - block.push_str(v); - block.push_str("\n"); - }, - } - } - block -} - -pub fn request(server: Server, request: &str) -> Response { - let mut req = TcpStream::connect(server.addr()).unwrap(); - req.write_all(request.as_bytes()).unwrap(); - - let mut response = String::new(); - req.read_to_string(&mut response).unwrap(); - - let mut lines = response.lines(); - let status = lines.next().unwrap().to_owned(); - let headers_raw = read_block(&mut lines, false); - let headers = headers_raw.split('\n').map(|v| v.to_owned()).collect(); - let body = read_block(&mut lines, true); - - Response { - status: status, - headers: headers, - headers_raw: headers_raw, - body: body, - } -} - diff --git a/dapps/src/tests/validation.rs b/dapps/src/tests/validation.rs index b233a07d8..b6a8c825c 100644 --- a/dapps/src/tests/validation.rs +++ b/dapps/src/tests/validation.rs @@ -34,7 +34,7 @@ fn should_reject_invalid_host() { // then assert_eq!(response.status, "HTTP/1.1 403 Forbidden".to_owned()); - assert_eq!(response.body, "85\n\n\t\t

Request with disallowed Host header has been blocked.

\n\t\t

Check the URL in your browser address bar.

\n\t\t\n0\n\n".to_owned()); + assert!(response.body.contains("Current host is disallowed")); } #[test] @@ -77,3 +77,24 @@ fn should_serve_dapps_domains() { assert_eq!(response.status, "HTTP/1.1 200 OK".to_owned()); } +#[test] +// NOTE [todr] This is required for error pages to be styled properly. +fn should_allow_parity_utils_even_on_invalid_domain() { + // given + let server = serve_hosts(Some(vec!["localhost:8080".into()])); + + // when + let response = request(server, + "\ + GET /parity-utils/styles.css HTTP/1.1\r\n\ + Host: 127.0.0.1:8080\r\n\ + Connection: close\r\n\ + \r\n\ + {} + " + ); + + // then + assert_eq!(response.status, "HTTP/1.1 200 OK".to_owned()); +} + diff --git a/devtools/src/http_client.rs b/devtools/src/http_client.rs new file mode 100644 index 000000000..27fa6ec50 --- /dev/null +++ b/devtools/src/http_client.rs @@ -0,0 +1,64 @@ +// Copyright 2015, 2016 Ethcore (UK) Ltd. +// This file is part of Parity. + +// Parity is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity. If not, see . + +use std::io::{Read, Write}; +use std::str::{self, Lines}; +use std::net::{TcpStream, SocketAddr}; + +pub struct Response { + pub status: String, + pub headers: Vec, + pub headers_raw: String, + pub body: String, +} + +pub fn read_block(lines: &mut Lines, all: bool) -> String { + let mut block = String::new(); + loop { + let line = lines.next(); + match line { + None => break, + Some("") if !all => break, + Some(v) => { + block.push_str(v); + block.push_str("\n"); + }, + } + } + block +} + +pub fn request(address: &SocketAddr, request: &str) -> Response { + let mut req = TcpStream::connect(address).unwrap(); + req.write_all(request.as_bytes()).unwrap(); + + let mut response = String::new(); + req.read_to_string(&mut response).unwrap(); + + let mut lines = response.lines(); + let status = lines.next().unwrap().to_owned(); + let headers_raw = read_block(&mut lines, false); + let headers = headers_raw.split('\n').map(|v| v.to_owned()).collect(); + let body = read_block(&mut lines, true); + + Response { + status: status, + headers: headers, + headers_raw: headers_raw, + body: body, + } +} + diff --git a/devtools/src/lib.rs b/devtools/src/lib.rs index 831e4315e..87457f7f3 100644 --- a/devtools/src/lib.rs +++ b/devtools/src/lib.rs @@ -22,6 +22,7 @@ extern crate rand; mod random_path; mod test_socket; mod stop_guard; +pub mod http_client; pub use random_path::*; pub use test_socket::*; diff --git a/signer/Cargo.toml b/signer/Cargo.toml index 5bb6325f1..cfcbedff8 100644 --- a/signer/Cargo.toml +++ b/signer/Cargo.toml @@ -19,6 +19,7 @@ ws = { git = "https://github.com/ethcore/ws-rs.git", branch = "mio-upstream-stab ethcore-util = { path = "../util" } ethcore-io = { path = "../util/io" } ethcore-rpc = { path = "../rpc" } +ethcore-devtools = { path = "../devtools" } parity-dapps-signer = { git = "https://github.com/ethcore/parity-ui.git", version = "1.4", optional = true} clippy = { version = "0.0.85", optional = true} diff --git a/signer/src/lib.rs b/signer/src/lib.rs index 45383c7a1..abe84a03e 100644 --- a/signer/src/lib.rs +++ b/signer/src/lib.rs @@ -54,15 +54,13 @@ extern crate jsonrpc_core; extern crate ws; #[cfg(feature = "ui")] extern crate parity_dapps_signer as signer; +#[cfg(test)] +extern crate ethcore_devtools as devtools; mod authcode_store; mod ws_server; +#[cfg(test)] +mod tests; pub use authcode_store::*; pub use ws_server::*; - -#[cfg(test)] -mod tests { - #[test] - fn should_work() {} -} diff --git a/signer/src/tests/mod.rs b/signer/src/tests/mod.rs new file mode 100644 index 000000000..eaed49de8 --- /dev/null +++ b/signer/src/tests/mod.rs @@ -0,0 +1,81 @@ +// Copyright 2015, 2016 Ethcore (UK) Ltd. +// This file is part of Parity. + +// Parity is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity. If not, see . + +use std::env; +use std::thread; +use std::time::Duration; +use std::sync::Arc; +use devtools::http_client; +use rpc::ConfirmationsQueue; +use rand; + +use ServerBuilder; +use Server; + +pub fn serve() -> Server { + let queue = Arc::new(ConfirmationsQueue::default()); + let builder = ServerBuilder::new(queue, env::temp_dir()); + let port = 35000 + rand::random::() % 10000; + let res = builder.start(format!("127.0.0.1:{}", port).parse().unwrap()).unwrap(); + thread::sleep(Duration::from_millis(25)); + res +} + +pub fn request(server: Server, request: &str) -> http_client::Response { + http_client::request(server.addr(), request) +} + +#[test] +fn should_reject_invalid_host() { + // given + let server = serve(); + + // when + let response = request(server, + "\ + GET / HTTP/1.1\r\n\ + Host: test:8180\r\n\ + Connection: close\r\n\ + \r\n\ + {} + " + ); + + // then + assert_eq!(response.status, "HTTP/1.1 403 FORBIDDEN".to_owned()); + assert!(response.body.contains("URL Blocked")); +} + +#[test] +fn should_serve_styles_even_on_disallowed_domain() { + // given + let server = serve(); + + // when + let response = request(server, + "\ + GET /styles.css HTTP/1.1\r\n\ + Host: test:8180\r\n\ + Connection: close\r\n\ + \r\n\ + {} + " + ); + + // then + assert_eq!(response.status, "HTTP/1.1 200 OK".to_owned()); +} + diff --git a/signer/src/ws_server/error_tpl.html b/signer/src/ws_server/error_tpl.html new file mode 100644 index 000000000..04a0f3c30 --- /dev/null +++ b/signer/src/ws_server/error_tpl.html @@ -0,0 +1,21 @@ + + + + + + {meta} + {title} + + + +
+
+

{title}

+

{message}

+

{details}

+
+
+ {version} +
+ + diff --git a/signer/src/ws_server/mod.rs b/signer/src/ws_server/mod.rs index 4ebe96bfa..6d332adbe 100644 --- a/signer/src/ws_server/mod.rs +++ b/signer/src/ws_server/mod.rs @@ -93,9 +93,15 @@ pub struct Server { broadcaster_handle: Option>, queue: Arc, panic_handler: Arc, + addr: SocketAddr, } impl Server { + /// Returns the address this server is listening on + pub fn addr(&self) -> &SocketAddr { + &self.addr + } + /// Starts a new `WebSocket` server in separate thread. /// Returns a `Server` handle which closes the server when droped. fn start(addr: SocketAddr, handler: Arc, queue: Arc, authcodes_path: PathBuf, skip_origin_validation: bool) -> Result { @@ -121,7 +127,7 @@ impl Server { // Spawn a thread with event loop let handle = thread::spawn(move || { ph.catch_panic(move || { - match ws.listen(addr).map_err(ServerError::from) { + match ws.listen(addr.clone()).map_err(ServerError::from) { Err(ServerError::IoError(io)) => die(format!( "Signer: Could not start listening on specified address. Make sure that no other instance is running on Signer's port. Details: {:?}", io @@ -158,6 +164,7 @@ impl Server { broadcaster_handle: Some(broadcaster_handle), queue: queue, panic_handler: panic_handler, + addr: addr, }) } } diff --git a/signer/src/ws_server/session.rs b/signer/src/ws_server/session.rs index 34097c24d..b53f47db8 100644 --- a/signer/src/ws_server/session.rs +++ b/signer/src/ws_server/session.rs @@ -107,10 +107,15 @@ impl ws::Handler for Session { fn on_request(&mut self, req: &ws::Request) -> ws::Result<(ws::Response)> { let origin = req.header("origin").or_else(|| req.header("Origin")).map(|x| &x[..]); let host = req.header("host").or_else(|| req.header("Host")).map(|x| &x[..]); + // Styles file is allowed for error pages to display nicely. + let is_styles_file = req.resource() == "/styles.css"; // Check request origin and host header. if !self.skip_origin_validation { - if !origin_is_allowed(&self.self_origin, origin) && !(origin.is_none() && origin_is_allowed(&self.self_origin, host)) { + let is_valid = origin_is_allowed(&self.self_origin, origin) || (origin.is_none() && origin_is_allowed(&self.self_origin, host)); + let is_valid = is_styles_file || is_valid; + + if !is_valid { warn!(target: "signer", "Blocked connection to Signer API from untrusted origin."); return Ok(error( ErrorType::Forbidden, @@ -121,8 +126,9 @@ impl ws::Handler for Session { } } - // Detect if it's a websocket request. - if req.header("sec-websocket-key").is_some() { + // Detect if it's a websocket request + // (styles file skips origin validation, so make sure to prevent WS connections on this resource) + if req.header("sec-websocket-key").is_some() && !is_styles_file { // Check authorization if !auth_is_valid(&self.authcodes_path, req.protocols()) { info!(target: "signer", "Unauthorized connection to Signer API blocked."); @@ -198,7 +204,7 @@ enum ErrorType { fn error(error: ErrorType, title: &str, message: &str, details: Option<&str>) -> ws::Response { let content = format!( - include_str!("../../../dapps/src/error_tpl.html"), + include_str!("./error_tpl.html"), title=title, meta="", message=message, From 89f1444c516aef931f90279d0df399a2c5fe8d4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 1 Sep 2016 11:16:19 +0200 Subject: [PATCH 3/7] Displaying special page when syncing. 404 instead of redirection --- dapps/src/apps/fetcher.rs | 77 ++++++++++++++++++++++++---------- dapps/src/lib.rs | 26 ++++++++++-- dapps/src/router/mod.rs | 11 +++-- dapps/src/tests/fetch.rs | 38 +++++++++++++++++ dapps/src/tests/helpers.rs | 23 +++++++--- dapps/src/tests/mod.rs | 1 + dapps/src/tests/redirection.rs | 12 +++--- parity/dapps.rs | 11 +++-- parity/run.rs | 1 + 9 files changed, 157 insertions(+), 43 deletions(-) create mode 100644 dapps/src/tests/fetch.rs diff --git a/dapps/src/apps/fetcher.rs b/dapps/src/apps/fetcher.rs index 6223fb5e8..cb58af976 100644 --- a/dapps/src/apps/fetcher.rs +++ b/dapps/src/apps/fetcher.rs @@ -30,6 +30,7 @@ use hyper::Control; use hyper::status::StatusCode; use random_filename; +use SyncStatus; use util::{Mutex, H256}; use util::sha3::sha3; use page::LocalPageEndpoint; @@ -44,6 +45,7 @@ const MAX_CACHED_DAPPS: usize = 10; pub struct AppFetcher { dapps_path: PathBuf, resolver: R, + sync: Arc, dapps: Arc>, } @@ -56,13 +58,14 @@ impl Drop for AppFetcher { impl AppFetcher { - pub fn new(resolver: R) -> Self { + pub fn new(resolver: R, sync_status: Arc) -> Self { let mut dapps_path = env::temp_dir(); dapps_path.push(random_filename()); AppFetcher { dapps_path: dapps_path, resolver: resolver, + sync: sync_status, dapps: Arc::new(Mutex::new(ContentCache::default())), } } @@ -74,14 +77,20 @@ impl AppFetcher { pub fn contains(&self, app_id: &str) -> bool { let mut dapps = self.dapps.lock(); - match dapps.get(app_id) { - // Check if we already have the app - Some(_) => true, - // fallback to resolver - None => match app_id.from_hex() { - Ok(app_id) => self.resolver.resolve(app_id).is_some(), - _ => false, - }, + // Check if we already have the app + if dapps.get(app_id).is_some() { + return true; + } + // fallback to resolver + if let Ok(app_id) = app_id.from_hex() { + // if app_id is valid, but we are syncing always return true. + if self.sync.is_major_syncing() { + return true; + } + // else try to resolve the app_id + self.resolver.resolve(app_id).is_some() + } else { + false } } @@ -89,6 +98,15 @@ impl AppFetcher { let mut dapps = self.dapps.lock(); let app_id = path.app_id.clone(); + if self.sync.is_major_syncing() { + return Box::new(ContentHandler::error( + StatusCode::ServiceUnavailable, + "Sync in progress", + "Your node is still syncing. We cannot resolve any content before it's fully synced.", + None + )); + } + let (new_status, handler) = { let status = dapps.get(&app_id); match status { @@ -108,20 +126,32 @@ impl AppFetcher { // We need to start fetching app None => { let app_hex = app_id.from_hex().expect("to_handler is called only when `contains` returns true."); - let app = self.resolver.resolve(app_hex).expect("to_handler is called only when `contains` returns true."); - let abort = Arc::new(AtomicBool::new(false)); + let app = self.resolver.resolve(app_hex); - (Some(ContentStatus::Fetching(abort.clone())), Box::new(ContentFetcherHandler::new( - app, - abort, - control, - path.using_dapps_domains, - DappInstaller { - dapp_id: app_id.clone(), - dapps_path: self.dapps_path.clone(), - dapps: self.dapps.clone(), - } - )) as Box) + if let Some(app) = app { + let abort = Arc::new(AtomicBool::new(false)); + + (Some(ContentStatus::Fetching(abort.clone())), Box::new(ContentFetcherHandler::new( + app, + abort, + control, + path.using_dapps_domains, + DappInstaller { + dapp_id: app_id.clone(), + dapps_path: self.dapps_path.clone(), + dapps: self.dapps.clone(), + } + )) as Box) + } else { + // This may happen when sync status changes in between + // `contains` and `to_handler` + (None, Box::new(ContentHandler::error( + StatusCode::NotFound, + "Resource Not Found", + "Requested resource was not found.", + None + )) as Box) + } }, } }; @@ -274,6 +304,7 @@ impl ContentValidator for DappInstaller { #[cfg(test)] mod tests { use std::env; + use std::sync::Arc; use util::Bytes; use endpoint::EndpointInfo; use page::LocalPageEndpoint; @@ -292,7 +323,7 @@ mod tests { fn should_true_if_contains_the_app() { // given let path = env::temp_dir(); - let fetcher = AppFetcher::new(FakeResolver); + let fetcher = AppFetcher::new(FakeResolver, Arc::new(|| false)); let handler = LocalPageEndpoint::new(path, EndpointInfo { name: "fake".into(), description: "".into(), diff --git a/dapps/src/lib.rs b/dapps/src/lib.rs index e24def0a1..0a2297189 100644 --- a/dapps/src/lib.rs +++ b/dapps/src/lib.rs @@ -88,11 +88,22 @@ use ethcore_rpc::Extendable; static DAPPS_DOMAIN : &'static str = ".parity"; +/// Indicates sync status +pub trait SyncStatus: Send + Sync { + /// Returns true if there is a major sync happening. + fn is_major_syncing(&self) -> bool; +} + +impl SyncStatus for F where F: Fn() -> bool + Send + Sync { + fn is_major_syncing(&self) -> bool { self() } +} + /// Webapps HTTP+RPC server build. pub struct ServerBuilder { dapps_path: String, handler: Arc, registrar: Arc, + sync_status: Arc, } impl Extendable for ServerBuilder { @@ -108,9 +119,15 @@ impl ServerBuilder { dapps_path: dapps_path, handler: Arc::new(IoHandler::new()), registrar: registrar, + sync_status: Arc::new(|| false), } } + /// Change default sync status. + pub fn with_sync_status(&mut self, status: Arc) { + self.sync_status = status; + } + /// Asynchronously start server with no authentication, /// returns result with `Server` handle on success or an error. pub fn start_unsecured_http(&self, addr: &SocketAddr, hosts: Option>) -> Result { @@ -120,7 +137,8 @@ impl ServerBuilder { NoAuth, self.handler.clone(), self.dapps_path.clone(), - self.registrar.clone() + self.registrar.clone(), + self.sync_status.clone(), ) } @@ -133,7 +151,8 @@ impl ServerBuilder { HttpBasicAuth::single_user(username, password), self.handler.clone(), self.dapps_path.clone(), - self.registrar.clone() + self.registrar.clone(), + self.sync_status.clone(), ) } } @@ -167,10 +186,11 @@ impl Server { handler: Arc, dapps_path: String, registrar: Arc, + sync_status: Arc, ) -> Result { let panic_handler = Arc::new(Mutex::new(None)); let authorization = Arc::new(authorization); - let apps_fetcher = Arc::new(apps::fetcher::AppFetcher::new(apps::urlhint::URLHintContract::new(registrar))); + let apps_fetcher = Arc::new(apps::fetcher::AppFetcher::new(apps::urlhint::URLHintContract::new(registrar), sync_status)); let endpoints = Arc::new(apps::all_endpoints(dapps_path)); let special = Arc::new({ let mut special = HashMap::new(); diff --git a/dapps/src/router/mod.rs b/dapps/src/router/mod.rs index 9326917bf..5dc1018b0 100644 --- a/dapps/src/router/mod.rs +++ b/dapps/src/router/mod.rs @@ -24,12 +24,12 @@ use DAPPS_DOMAIN; use std::sync::Arc; use std::collections::HashMap; use url::{Url, Host}; -use hyper::{self, server, Next, Encoder, Decoder, Control}; +use hyper::{self, server, Next, Encoder, Decoder, Control, StatusCode}; use hyper::net::HttpStream; use apps; use apps::fetcher::AppFetcher; use endpoint::{Endpoint, Endpoints, EndpointPath}; -use handlers::{Redirection, extract_url}; +use handlers::{Redirection, extract_url, ContentHandler}; use self::auth::{Authorization, Authorized}; /// Special endpoints are accessible on every domain (every dapp) @@ -94,7 +94,12 @@ impl server::Handler for Router { // Redirection to main page (maybe 404 instead?) (Some(ref path), _) if *req.method() == hyper::method::Method::Get => { let address = apps::redirection_address(path.using_dapps_domains, self.main_page); - Redirection::new(address.as_str()) + Box::new(ContentHandler::error( + StatusCode::NotFound, + "404 Not Found", + "Requested content was not found on a server.", + Some(&format!("Go back to the Home Page.", address)) + )) }, // Redirect any GET request to home. _ if *req.method() == hyper::method::Method::Get => { diff --git a/dapps/src/tests/fetch.rs b/dapps/src/tests/fetch.rs new file mode 100644 index 000000000..6fd65c00f --- /dev/null +++ b/dapps/src/tests/fetch.rs @@ -0,0 +1,38 @@ +// Copyright 2015, 2016 Ethcore (UK) Ltd. +// This file is part of Parity. + +// Parity is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity. If not, see . + +use tests::helpers::{serve_with_registrar, request}; + +#[test] +fn should_resolve_dapp() { + // given + let (server, registrar) = serve_with_registrar(); + + // when + let response = request(server, + "\ + GET / HTTP/1.1\r\n\ + Host: 1472a9e190620cdf6b31f383373e45efcfe869a820c91f9ccd7eb9fb45e4985d.parity\r\n\ + Connection: close\r\n\ + \r\n\ + " + ); + + // then + assert_eq!(response.status, "HTTP/1.1 404 Not Found".to_owned()); + assert_eq!(registrar.calls.lock().len(), 2); +} + diff --git a/dapps/src/tests/helpers.rs b/dapps/src/tests/helpers.rs index 1296162d2..4cd21520c 100644 --- a/dapps/src/tests/helpers.rs +++ b/dapps/src/tests/helpers.rs @@ -58,22 +58,35 @@ impl ContractClient for FakeRegistrar { } } -pub fn serve_hosts(hosts: Option>) -> Server { +pub fn init_server(hosts: Option>) -> (Server, Arc) { let registrar = Arc::new(FakeRegistrar::new()); let mut dapps_path = env::temp_dir(); dapps_path.push("non-existent-dir-to-prevent-fs-files-from-loading"); - let builder = ServerBuilder::new(dapps_path.to_str().unwrap().into(), registrar); - builder.start_unsecured_http(&"127.0.0.1:0".parse().unwrap(), hosts).unwrap() + let builder = ServerBuilder::new(dapps_path.to_str().unwrap().into(), registrar.clone()); + ( + builder.start_unsecured_http(&"127.0.0.1:0".parse().unwrap(), hosts).unwrap(), + registrar, + ) } pub fn serve_with_auth(user: &str, pass: &str) -> Server { let registrar = Arc::new(FakeRegistrar::new()); - let builder = ServerBuilder::new(env::temp_dir().to_str().unwrap().into(), registrar); + let mut dapps_path = env::temp_dir(); + dapps_path.push("non-existent-dir-to-prevent-fs-files-from-loading"); + let builder = ServerBuilder::new(dapps_path.to_str().unwrap().into(), registrar); builder.start_basic_auth_http(&"127.0.0.1:0".parse().unwrap(), None, user, pass).unwrap() } +pub fn serve_hosts(hosts: Option>) -> Server { + init_server(hosts).0 +} + +pub fn serve_with_registrar() -> (Server, Arc) { + init_server(None) +} + pub fn serve() -> Server { - serve_hosts(None) + init_server(None).0 } pub fn request(server: Server, request: &str) -> http_client::Response { diff --git a/dapps/src/tests/mod.rs b/dapps/src/tests/mod.rs index 8c5bf2283..cc0e693c9 100644 --- a/dapps/src/tests/mod.rs +++ b/dapps/src/tests/mod.rs @@ -20,6 +20,7 @@ mod helpers; mod api; mod authorization; +mod fetch; mod redirection; mod validation; diff --git a/dapps/src/tests/redirection.rs b/dapps/src/tests/redirection.rs index 53aa393e2..2fdecbd4d 100644 --- a/dapps/src/tests/redirection.rs +++ b/dapps/src/tests/redirection.rs @@ -57,7 +57,7 @@ fn should_redirect_to_home_when_trailing_slash_is_missing() { } #[test] -fn should_redirect_to_home_on_invalid_dapp() { +fn should_display_404_on_invalid_dapp() { // given let server = serve(); @@ -72,12 +72,12 @@ fn should_redirect_to_home_on_invalid_dapp() { ); // then - assert_eq!(response.status, "HTTP/1.1 302 Found".to_owned()); - assert_eq!(response.headers.get(0).unwrap(), "Location: /home/"); + assert_eq!(response.status, "HTTP/1.1 404 Not Found".to_owned()); + assert!(response.body.contains("href=\"/home/")); } #[test] -fn should_redirect_to_home_on_invalid_dapp_with_domain() { +fn should_display_404_on_invalid_dapp_with_domain() { // given let server = serve(); @@ -92,8 +92,8 @@ fn should_redirect_to_home_on_invalid_dapp_with_domain() { ); // then - assert_eq!(response.status, "HTTP/1.1 302 Found".to_owned()); - assert_eq!(response.headers.get(0).unwrap(), "Location: http://home.parity/"); + assert_eq!(response.status, "HTTP/1.1 404 Not Found".to_owned()); + assert!(response.body.contains("href=\"http://home.parity/")); } #[test] diff --git a/parity/dapps.rs b/parity/dapps.rs index 7f759ed3c..fae395b5a 100644 --- a/parity/dapps.rs +++ b/parity/dapps.rs @@ -18,6 +18,7 @@ use std::sync::Arc; use io::PanicHandler; use rpc_apis; use ethcore::client::Client; +use ethsync::SyncProvider; use helpers::replace_home; #[derive(Debug, PartialEq, Clone)] @@ -49,6 +50,7 @@ pub struct Dependencies { pub panic_handler: Arc, pub apis: Arc, pub client: Arc, + pub sync: Arc, } pub fn new(configuration: Configuration, deps: Dependencies) -> Result, String> { @@ -117,9 +119,12 @@ mod server { ) -> Result { use ethcore_dapps as dapps; - let server = dapps::ServerBuilder::new(dapps_path, Arc::new(Registrar { - client: deps.client.clone(), - })); + let mut server = dapps::ServerBuilder::new( + dapps_path, + Arc::new(Registrar { client: deps.client.clone() }) + ); + let sync = deps.sync.clone(); + server.with_sync_status(Arc::new(move || sync.status().is_major_syncing())); let server = rpc_apis::setup_rpc(server, deps.apis.clone(), rpc_apis::ApiSet::UnsafeContext); let start_result = match auth { None => { diff --git a/parity/run.rs b/parity/run.rs index 71995cd5f..8a68fe1af 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -224,6 +224,7 @@ pub fn execute(cmd: RunCmd) -> Result<(), String> { panic_handler: panic_handler.clone(), apis: deps_for_rpc_apis.clone(), client: client.clone(), + sync: sync_provider.clone(), }; // start dapps server From 055ff91464f746845f1092e542c1912a6afbd68c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 1 Sep 2016 11:29:40 +0200 Subject: [PATCH 4/7] Bumping ui --- Cargo.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b9ad5d6bd..c5df53681 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1070,7 +1070,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "parity-dapps" version = "1.4.0" -source = "git+https://github.com/ethcore/parity-ui.git#be8754c3ccd142c43a20b82e9b1fba416b5b332a" +source = "git+https://github.com/ethcore/parity-ui.git#926b09b66c4940b09dc82c52adb4afd9e31155bc" dependencies = [ "aster 0.17.0 (registry+https://github.com/rust-lang/crates.io-index)", "glob 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1084,7 +1084,7 @@ dependencies = [ [[package]] name = "parity-dapps-home" version = "1.4.0" -source = "git+https://github.com/ethcore/parity-ui.git#be8754c3ccd142c43a20b82e9b1fba416b5b332a" +source = "git+https://github.com/ethcore/parity-ui.git#926b09b66c4940b09dc82c52adb4afd9e31155bc" dependencies = [ "parity-dapps 1.4.0 (git+https://github.com/ethcore/parity-ui.git)", ] @@ -1092,7 +1092,7 @@ dependencies = [ [[package]] name = "parity-dapps-signer" version = "1.4.0" -source = "git+https://github.com/ethcore/parity-ui.git#be8754c3ccd142c43a20b82e9b1fba416b5b332a" +source = "git+https://github.com/ethcore/parity-ui.git#926b09b66c4940b09dc82c52adb4afd9e31155bc" dependencies = [ "parity-dapps 1.4.0 (git+https://github.com/ethcore/parity-ui.git)", ] @@ -1100,7 +1100,7 @@ dependencies = [ [[package]] name = "parity-dapps-status" version = "1.4.0" -source = "git+https://github.com/ethcore/parity-ui.git#be8754c3ccd142c43a20b82e9b1fba416b5b332a" +source = "git+https://github.com/ethcore/parity-ui.git#926b09b66c4940b09dc82c52adb4afd9e31155bc" dependencies = [ "parity-dapps 1.4.0 (git+https://github.com/ethcore/parity-ui.git)", ] @@ -1108,7 +1108,7 @@ dependencies = [ [[package]] name = "parity-dapps-wallet" version = "1.4.0" -source = "git+https://github.com/ethcore/parity-ui.git#be8754c3ccd142c43a20b82e9b1fba416b5b332a" +source = "git+https://github.com/ethcore/parity-ui.git#926b09b66c4940b09dc82c52adb4afd9e31155bc" dependencies = [ "parity-dapps 1.4.0 (git+https://github.com/ethcore/parity-ui.git)", ] From a9bc021022414e64e13bde9b20a0a76ecf77ba21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 1 Sep 2016 11:54:09 +0200 Subject: [PATCH 5/7] 404 pages for dapps resources --- dapps/src/page/builtin.rs | 2 +- dapps/src/page/handler.rs | 52 ++++++++++++++++++++++++++++----------- dapps/src/page/local.rs | 2 +- 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/dapps/src/page/builtin.rs b/dapps/src/page/builtin.rs index 1c7ca32d4..3c8d66686 100644 --- a/dapps/src/page/builtin.rs +++ b/dapps/src/page/builtin.rs @@ -79,7 +79,7 @@ impl Endpoint for PageEndpoint { app: BuiltinDapp::new(self.app.clone()), prefix: self.prefix.clone(), path: path, - file: None, + file: Default::default(), safe_to_embed: self.safe_to_embed, }) } diff --git a/dapps/src/page/handler.rs b/dapps/src/page/handler.rs index eca242e7b..70487443e 100644 --- a/dapps/src/page/handler.rs +++ b/dapps/src/page/handler.rs @@ -22,6 +22,7 @@ use hyper::net::HttpStream; use hyper::status::StatusCode; use hyper::{Decoder, Encoder, Next}; use endpoint::EndpointPath; +use handlers::ContentHandler; /// Represents a file that can be sent to client. /// Implementation should keep track of bytes already sent internally. @@ -48,6 +49,25 @@ pub trait Dapp: Send + 'static { fn file(&self, path: &str) -> Option; } +/// Currently served by `PageHandler` file +pub enum ServedFile { + /// File from dapp + File(T::DappFile), + /// Error (404) + Error(ContentHandler), +} + +impl Default for ServedFile { + fn default() -> Self { + ServedFile::Error(ContentHandler::error( + StatusCode::NotFound, + "404 Not Found", + "Requested dapp resource was not found.", + None + )) + } +} + /// A handler for a single webapp. /// Resolves correct paths and serves as a plumbing code between /// hyper server and dapp. @@ -55,7 +75,7 @@ pub struct PageHandler { /// A Dapp. pub app: T, /// File currently being served (or `None` if file does not exist). - pub file: Option, + pub file: ServedFile, /// Optional prefix to strip from path. pub prefix: Option, /// Requested path. @@ -95,7 +115,7 @@ impl server::Handler for PageHandler { self.app.file(&self.extract_path(url.path())) }, _ => None, - }; + }.map_or_else(|| ServedFile::default(), |f| ServedFile::File(f)); Next::write() } @@ -104,24 +124,26 @@ impl server::Handler for PageHandler { } fn on_response(&mut self, res: &mut server::Response) -> Next { - if let Some(ref f) = self.file { - res.set_status(StatusCode::Ok); - res.headers_mut().set(header::ContentType(f.content_type().parse().unwrap())); - if !self.safe_to_embed { - res.headers_mut().set_raw("X-Frame-Options", vec![b"SAMEORIGIN".to_vec()]); + match self.file { + ServedFile::File(ref f) => { + res.set_status(StatusCode::Ok); + res.headers_mut().set(header::ContentType(f.content_type().parse().unwrap())); + if !self.safe_to_embed { + res.headers_mut().set_raw("X-Frame-Options", vec![b"SAMEORIGIN".to_vec()]); + } + Next::write() + }, + ServedFile::Error(ref mut handler) => { + handler.on_response(res) } - Next::write() - } else { - res.set_status(StatusCode::NotFound); - Next::write() } } fn on_response_writable(&mut self, encoder: &mut Encoder) -> Next { match self.file { - None => Next::end(), - Some(ref f) if f.is_drained() => Next::end(), - Some(ref mut f) => match encoder.write(f.next_chunk()) { + ServedFile::Error(ref mut handler) => handler.on_response_writable(encoder), + ServedFile::File(ref f) if f.is_drained() => Next::end(), + ServedFile::File(ref mut f) => match encoder.write(f.next_chunk()) { Ok(bytes) => { f.bytes_written(bytes); Next::write() @@ -190,7 +212,7 @@ fn should_extract_path_with_appid() { port: 8080, using_dapps_domains: true, }, - file: None, + file: Default::default(), safe_to_embed: true, }; diff --git a/dapps/src/page/local.rs b/dapps/src/page/local.rs index dcfd9bed2..86d4273d5 100644 --- a/dapps/src/page/local.rs +++ b/dapps/src/page/local.rs @@ -49,7 +49,7 @@ impl Endpoint for LocalPageEndpoint { app: LocalDapp::new(self.path.clone()), prefix: None, path: path, - file: None, + file: Default::default(), safe_to_embed: false, }) } From 9f8482e96843f077a8f80b63e3a9cf6276800040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 1 Sep 2016 15:33:26 +0200 Subject: [PATCH 6/7] Consistent capitalization of titles --- dapps/src/apps/fetcher.rs | 4 ++-- dapps/src/handlers/fetch.rs | 2 +- dapps/src/router/auth.rs | 4 +++- dapps/src/router/host_validation.rs | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/dapps/src/apps/fetcher.rs b/dapps/src/apps/fetcher.rs index 869a34df2..7d7db7b8b 100644 --- a/dapps/src/apps/fetcher.rs +++ b/dapps/src/apps/fetcher.rs @@ -101,9 +101,9 @@ impl AppFetcher { if self.sync.is_major_syncing() { return Box::new(ContentHandler::error( StatusCode::ServiceUnavailable, - "Sync in progress", + "Sync In Progress", "Your node is still syncing. We cannot resolve any content before it's fully synced.", - None + Some("Refresh") )); } diff --git a/dapps/src/handlers/fetch.rs b/dapps/src/handlers/fetch.rs index 7cbb2537b..ef37cfc61 100644 --- a/dapps/src/handlers/fetch.rs +++ b/dapps/src/handlers/fetch.rs @@ -133,7 +133,7 @@ impl server::Handler for ContentFetcherHandler< Err(e) => FetchState::Error(ContentHandler::error( StatusCode::BadGateway, "Unable To Start Dapp Download", - "Could not initialize download of the dapp. It might be a problem with remote server.", + "Could not initialize download of the dapp. It might be a problem with the remote server.", Some(&format!("{}", e)), )), } diff --git a/dapps/src/router/auth.rs b/dapps/src/router/auth.rs index 596796eed..ff05420bc 100644 --- a/dapps/src/router/auth.rs +++ b/dapps/src/router/auth.rs @@ -57,7 +57,9 @@ impl Authorization for HttpBasicAuth { Access::Denied => { Authorized::No(Box::new(ContentHandler::error( status::StatusCode::Unauthorized, - "Unauthorized", "You need to provide valid credentials to access this page.", None + "Unauthorized", + "You need to provide valid credentials to access this page.", + None ))) }, Access::AuthRequired => { diff --git a/dapps/src/router/host_validation.rs b/dapps/src/router/host_validation.rs index d1d651c5d..5be30ef8b 100644 --- a/dapps/src/router/host_validation.rs +++ b/dapps/src/router/host_validation.rs @@ -39,7 +39,7 @@ pub fn is_valid(request: &server::Request, allowed_hosts: &[String], pub fn host_invalid_response() -> Box + Send> { Box::new(ContentHandler::error(StatusCode::Forbidden, - "Current host is disallowed", + "Current Host Is Disallowed", "You are trying to access your node using incorrect address.", Some("Use allowed URL or specify different hosts CLI options.") )) From d0bc80e58a7fe34ca03021ab8f31f8a333493cac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 2 Sep 2016 10:10:51 +0200 Subject: [PATCH 7/7] Fixign tests --- dapps/src/tests/api.rs | 2 +- dapps/src/tests/authorization.rs | 2 +- dapps/src/tests/validation.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dapps/src/tests/api.rs b/dapps/src/tests/api.rs index a9d3eba3b..ab0d33726 100644 --- a/dapps/src/tests/api.rs +++ b/dapps/src/tests/api.rs @@ -57,7 +57,7 @@ fn should_serve_apps() { // then assert_eq!(response.status, "HTTP/1.1 200 OK".to_owned()); assert_eq!(response.headers.get(0).unwrap(), "Content-Type: application/json"); - assert!(response.body.contains("Parity Home Screen")); + assert!(response.body.contains("Parity Home Screen"), response.body); } #[test] diff --git a/dapps/src/tests/authorization.rs b/dapps/src/tests/authorization.rs index e7a70c9cd..9d5a46af4 100644 --- a/dapps/src/tests/authorization.rs +++ b/dapps/src/tests/authorization.rs @@ -54,7 +54,7 @@ fn should_reject_on_invalid_auth() { // then assert_eq!(response.status, "HTTP/1.1 401 Unauthorized".to_owned()); - assert!(response.body.contains("Unauthorized")); + assert!(response.body.contains("Unauthorized"), response.body); assert_eq!(response.headers_raw.contains("WWW-Authenticate"), false); } diff --git a/dapps/src/tests/validation.rs b/dapps/src/tests/validation.rs index b6a8c825c..c39350cce 100644 --- a/dapps/src/tests/validation.rs +++ b/dapps/src/tests/validation.rs @@ -34,7 +34,7 @@ fn should_reject_invalid_host() { // then assert_eq!(response.status, "HTTP/1.1 403 Forbidden".to_owned()); - assert!(response.body.contains("Current host is disallowed")); + assert!(response.body.contains("Current Host Is Disallowed"), response.body); } #[test]