diff --git a/Cargo.lock b/Cargo.lock index b02fcbd24..edd76f9f4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -35,6 +35,11 @@ dependencies = [ "nodrop 0.1.12 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "assert_matches" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "aster" version = "0.41.0" @@ -699,6 +704,7 @@ name = "ethcore-network-devp2p" version = "1.12.0" dependencies = [ "ansi_term 0.10.2 (registry+https://github.com/rust-lang/crates.io-index)", + "assert_matches 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "bytes 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", "ethcore-bytes 0.1.0", @@ -3767,6 +3773,7 @@ dependencies = [ "checksum ansi_term 0.10.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6b3568b48b7cefa6b8ce125f9bb4989e52fbcc29ebea88df04cc7c5f12f70455" "checksum app_dirs 1.2.1 (git+https://github.com/paritytech/app-dirs-rs)" = "" "checksum arrayvec 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)" = "a1e964f9e24d588183fcb43503abda40d288c8657dfc27311516ce2f05675aef" +"checksum assert_matches 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "664470abf00fae0f31c0eb6e1ca12d82961b2a2541ef898bc9dd51a9254d218b" "checksum aster 0.41.0 (registry+https://github.com/rust-lang/crates.io-index)" = "4ccfdf7355d9db158df68f976ed030ab0f6578af811f5a7bb6dcf221ec24e0e0" "checksum atty 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "af80143d6f7608d746df1520709e5d141c96f240b0e62b0aa41bdfb53374d9d4" "checksum backtrace 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)" = "ebbbf59b1c43eefa8c3ede390fcc36820b4999f7914104015be25025e0d62af2" diff --git a/util/network-devp2p/Cargo.toml b/util/network-devp2p/Cargo.toml index a8207ca61..3ec968439 100644 --- a/util/network-devp2p/Cargo.toml +++ b/util/network-devp2p/Cargo.toml @@ -38,6 +38,7 @@ error-chain = { version = "0.11", default-features = false } [dev-dependencies] tempdir = "0.3" +assert_matches = "1.2" [features] default = [] diff --git a/util/network-devp2p/src/lib.rs b/util/network-devp2p/src/lib.rs index 8faf21e46..239763cbe 100644 --- a/util/network-devp2p/src/lib.rs +++ b/util/network-devp2p/src/lib.rs @@ -95,6 +95,8 @@ extern crate serde_derive; #[cfg(test)] extern crate tempdir; +#[cfg(test)] #[macro_use] +extern crate assert_matches; mod host; mod connection; diff --git a/util/network-devp2p/src/node_table.rs b/util/network-devp2p/src/node_table.rs index 507945586..8640901cd 100644 --- a/util/network-devp2p/src/node_table.rs +++ b/util/network-devp2p/src/node_table.rs @@ -14,6 +14,12 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +use discovery::{TableUpdates, NodeEntry}; +use ethereum_types::H512; +use ip_utils::*; +use network::{Error, ErrorKind, AllowIP, IpFilter}; +use rlp::{Rlp, RlpStream, DecoderError}; +use serde_json; use std::collections::{HashMap, HashSet}; use std::fmt::{self, Display, Formatter}; use std::hash::{Hash, Hasher}; @@ -23,12 +29,6 @@ use std::str::FromStr; use std::{fs, mem, slice}; use std::time::{self, Duration, SystemTime}; use rand::{self, Rng}; -use ethereum_types::H512; -use rlp::{Rlp, RlpStream, DecoderError}; -use network::{Error, ErrorKind, AllowIP, IpFilter}; -use discovery::{TableUpdates, NodeEntry}; -use ip_utils::*; -use serde_json; /// Node public key pub type NodeId = H512; @@ -124,8 +124,8 @@ impl FromStr for NodeEndpoint { address: a, udp_port: a.port() }), - Ok(_) => Err(ErrorKind::AddressResolve(None).into()), - Err(e) => Err(ErrorKind::AddressResolve(Some(e)).into()) + Ok(None) => bail!(ErrorKind::AddressResolve(None)), + Err(_) => Err(ErrorKind::AddressParse.into()) // always an io::Error of InvalidInput kind } } } @@ -534,11 +534,34 @@ mod tests { assert!(endpoint.is_ok()); let v4 = match endpoint.unwrap().address { SocketAddr::V4(v4address) => v4address, - _ => panic!("should ve v4 address") + _ => panic!("should be v4 address") }; assert_eq!(SocketAddrV4::new(Ipv4Addr::new(123, 99, 55, 44), 7770), v4); } + #[test] + fn endpoint_parse_empty_ip_string_returns_error() { + let endpoint = NodeEndpoint::from_str(""); + assert!(endpoint.is_err()); + assert_matches!(endpoint.unwrap_err().kind(), &ErrorKind::AddressParse); + } + + #[test] + fn endpoint_parse_invalid_ip_string_returns_error() { + let endpoint = NodeEndpoint::from_str("beef"); + assert!(endpoint.is_err()); + assert_matches!(endpoint.unwrap_err().kind(), &ErrorKind::AddressParse); + } + + #[test] + fn endpoint_parse_valid_ip_without_port_returns_error() { + let endpoint = NodeEndpoint::from_str("123.123.123.123"); + assert!(endpoint.is_err()); + assert_matches!(endpoint.unwrap_err().kind(), &ErrorKind::AddressParse); + let endpoint = NodeEndpoint::from_str("123.123.123.123:123"); + assert!(endpoint.is_ok()) + } + #[test] fn node_parse() { assert!(validate_node_url("enode://a979fb575495b8d6db44f750317d0f4622bf4c2aa3365d6af7c284339968eef29b69ad0dce72a4d8db5ebb4968de0e3bec910127f134779fbcb0cb6d3331163c@22.99.55.44:7770").is_none()); @@ -555,6 +578,17 @@ mod tests { node.id); } + #[test] + fn node_parse_fails_for_invalid_urls() { + let node = Node::from_str("foo"); + assert!(node.is_err()); + assert_matches!(node.unwrap_err().kind(), &ErrorKind::AddressParse); + + let node = Node::from_str("enode://foo@bar"); + assert!(node.is_err()); + assert_matches!(node.unwrap_err().kind(), &ErrorKind::AddressParse); + } + #[test] fn table_last_contact_order() { let node1 = Node::from_str("enode://a979fb575495b8d6db44f750317d0f4622bf4c2aa3365d6af7c284339968eef29b69ad0dce72a4d8db5ebb4968de0e3bec910127f134779fbcb0cb6d3331163c@22.99.55.44:7770").unwrap(); diff --git a/util/network/src/error.rs b/util/network/src/error.rs index 6342bfe4a..610448669 100644 --- a/util/network/src/error.rs +++ b/util/network/src/error.rs @@ -84,11 +84,16 @@ error_chain! { foreign_links { SocketIo(IoError) #[doc = "Socket IO error."]; Io(io::Error) #[doc = "Error concerning the Rust standard library's IO subsystem."]; - AddressParse(net::AddrParseError) #[doc = "Error concerning the network address parsing subsystem."]; Decompression(snappy::InvalidInput) #[doc = "Decompression error."]; } errors { + #[doc = "Error concerning the network address parsing subsystem."] + AddressParse { + description("Failed to parse network address"), + display("Failed to parse network address"), + } + #[doc = "Error concerning the network address resolution subsystem."] AddressResolve(err: Option) { description("Failed to resolve network address"), @@ -163,6 +168,10 @@ impl From for Error { } } +impl From for Error { + fn from(_err: net::AddrParseError) -> Self { ErrorKind::AddressParse.into() } +} + #[test] fn test_errors() { assert_eq!(DisconnectReason::ClientQuit, DisconnectReason::from_u8(8));