Merge pull request #6970 from paritytech/validate_node_url

Adds validate_node_url() and refactors boot node check (#6907)

parity/configuration.rs

@@ -28,7 +28,7 @@ use bigint::hash::H256;
 use util::{version_data, Address};
 use bytes::Bytes;
 use ansi_term::Colour;
-use ethsync::{NetworkConfiguration, is_valid_node_url};
+use ethsync::{NetworkConfiguration, validate_node_url, NetworkError};
 use ethcore::ethstore::ethkey::{Secret, Public};
 use ethcore::client::{VMType};
 use ethcore::miner::{MinerOptions, Banning, StratumOptions};
@@ -698,9 +698,15 @@ impl Configuration {
 				let mut node_file = File::open(path).map_err(|e| format!("Error opening reserved nodes file: {}", e))?;
 				node_file.read_to_string(&mut buffer).map_err(|_| "Error reading reserved node file")?;
 				let lines = buffer.lines().map(|s| s.trim().to_owned()).filter(|s| !s.is_empty() && !s.starts_with("#")).collect::<Vec<_>>();
-				if let Some(invalid) = lines.iter().find(|s| !is_valid_node_url(s)) {
+
-					return Err(format!("Invalid node address format given for a boot node: {}", invalid));
+				for line in &lines {
+					match validate_node_url(line) {
+						None => continue,
+						Some(NetworkError::AddressResolve(_)) => return Err(format!("Failed to resolve hostname of a boot node: {}", line)),
+						Some(_) => return Err(format!("Invalid node address format given for a boot node: {}", line)),
+					}
 				}
+
 				Ok(lines)
 			},
 			None => Ok(Vec::new())

parity/helpers.rs

@@ -29,7 +29,7 @@ use cache::CacheConfig;
 use dir::DatabaseDirectories;
 use upgrade::{upgrade, upgrade_data_paths};
 use migration::migrate;
-use ethsync::is_valid_node_url;
+use ethsync::{validate_node_url, NetworkError};
 use path;
 
 pub fn to_duration(s: &str) -> Result<Duration, String> {
@@ -181,10 +181,10 @@ pub fn parity_ipc_path(base: &str, path: &str, shift: u16) -> String {
 pub fn to_bootnodes(bootnodes: &Option<String>) -> Result<Vec<String>, String> {
 	match *bootnodes {
 		Some(ref x) if !x.is_empty() => x.split(',').map(|s| {
-			if is_valid_node_url(s) {
+			match validate_node_url(s) {
-				Ok(s.to_owned())
+				None => Ok(s.to_owned()),
-			} else {
+				Some(NetworkError::AddressResolve(_)) => Err(format!("Failed to resolve hostname of a boot node: {}", s)),
-				Err(format!("Invalid node address format given for a boot node: {}", s))
+				Some(_) => Err(format!("Invalid node address format given for a boot node: {}", s)),
 			}
 		}).collect(),
 		Some(_) => Ok(vec![]),

sync/src/lib.rs

@@ -73,7 +73,7 @@ mod api;
 
 pub use api::*;
 pub use chain::{SyncStatus, SyncState};
-pub use network::{is_valid_node_url, NonReservedPeerMode, NetworkError, ConnectionFilter, ConnectionDirection};
+pub use network::{validate_node_url, NonReservedPeerMode, NetworkError, ConnectionFilter, ConnectionDirection};
 
 #[cfg(test)]
 pub(crate) type Address = bigint::hash::H160;

util/network/src/lib.rs

@@ -115,7 +115,7 @@ pub use session::SessionInfo;
 pub use connection_filter::{ConnectionFilter, ConnectionDirection};
 
 pub use io::TimerToken;
-pub use node_table::{is_valid_node_url, NodeId};
+pub use node_table::{validate_node_url, NodeId};
 use ipnetwork::{IpNetwork, IpNetworkError};
 use std::str::FromStr;
 

util/network/src/node_table.rs

@@ -28,7 +28,7 @@ use std::io::{Read, Write};
 use bigint::hash::*;
 use rlp::*;
 use time::Tm;
-use error::NetworkError;
+use NetworkError;
 use {AllowIP, IpFilter};
 use discovery::{TableUpdates, NodeEntry};
 use ip_utils::*;
@@ -363,9 +363,12 @@ impl Drop for NodeTable {
 }
 
 /// Check if node url is valid
-pub fn is_valid_node_url(url: &str) -> bool {
+pub fn validate_node_url(url: &str) -> Option<NetworkError> {
 	use std::str::FromStr;
-	Node::from_str(url).is_ok()
+	match Node::from_str(url) {
+		Ok(_) => None,
+		Err(e) => Some(e)
+	}
 }
 
 #[cfg(test)]
@@ -390,7 +393,7 @@ mod tests {
 
 	#[test]
 	fn node_parse() {
-		assert!(is_valid_node_url("enode://a979fb575495b8d6db44f750317d0f4622bf4c2aa3365d6af7c284339968eef29b69ad0dce72a4d8db5ebb4968de0e3bec910127f134779fbcb0cb6d3331163c@22.99.55.44:7770"));
+		assert!(validate_node_url("enode://a979fb575495b8d6db44f750317d0f4622bf4c2aa3365d6af7c284339968eef29b69ad0dce72a4d8db5ebb4968de0e3bec910127f134779fbcb0cb6d3331163c@22.99.55.44:7770").is_none());
 		let node = Node::from_str("enode://a979fb575495b8d6db44f750317d0f4622bf4c2aa3365d6af7c284339968eef29b69ad0dce72a4d8db5ebb4968de0e3bec910127f134779fbcb0cb6d3331163c@22.99.55.44:7770");
 		assert!(node.is_ok());
 		let node = node.unwrap();