Handle socket address parsing errors (#8545)

Unpack errors and check for io::ErrorKind::InvalidInput and return our own AddressParse error. Remove the foreign link to std::net::AddrParseError and add an `impl From` for that error. Test parsing properly.

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)" = "<none>"
 "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"

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 = []

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;

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 <http://www.gnu.org/licenses/>.
 
+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();

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<io::Error>) {
 			description("Failed to resolve network address"),
@@ -163,6 +168,10 @@ impl From<crypto::error::SymmError> for Error {
 	}
 }
 
+impl From<net::AddrParseError> for Error {
+	fn from(_err: net::AddrParseError) -> Self { ErrorKind::AddressParse.into() }
+}
+
 #[test]
 fn test_errors() {
 	assert_eq!(DisconnectReason::ClientQuit, DisconnectReason::from_u8(8));