From 1516fc1c5790028064c5ea483cad9be148baa987 Mon Sep 17 00:00:00 2001 From: Dmitry Kashitsyn Date: Tue, 31 Oct 2017 11:09:31 +0700 Subject: [PATCH 1/3] Adds validate_node_url() and refactors boot node check (#6907) --- parity/configuration.rs | 12 +++++++++--- parity/helpers.rs | 10 +++++----- sync/src/lib.rs | 2 +- util/network/src/lib.rs | 2 +- util/network/src/node_table.rs | 11 ++++++++++- 5 files changed, 26 insertions(+), 11 deletions(-) diff --git a/parity/configuration.rs b/parity/configuration.rs index 31febd375..189b45ac8 100644 --- a/parity/configuration.rs +++ b/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::>(); - 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()) diff --git a/parity/helpers.rs b/parity/helpers.rs index f04027029..8527850e8 100644 --- a/parity/helpers.rs +++ b/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 { @@ -181,10 +181,10 @@ pub fn parity_ipc_path(base: &str, path: &str, shift: u16) -> String { pub fn to_bootnodes(bootnodes: &Option) -> Result, String> { match *bootnodes { Some(ref x) if !x.is_empty() => x.split(',').map(|s| { - if is_valid_node_url(s) { - Ok(s.to_owned()) - } else { - Err(format!("Invalid node address format given for a boot node: {}", s)) + match validate_node_url(s) { + None => Ok(s.to_owned()), + Some(NetworkError::AddressResolve(_)) => Err(format!("Failed to resolve hostname of a boot node: {}", s)), + Some(_) => Err(format!("Invalid node address format given for a boot node: {}", s)), } }).collect(), Some(_) => Ok(vec![]), diff --git a/sync/src/lib.rs b/sync/src/lib.rs index 091ce5ba9..45f7a8d98 100644 --- a/sync/src/lib.rs +++ b/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::{is_valid_node_url, validate_node_url, NonReservedPeerMode, NetworkError, ConnectionFilter, ConnectionDirection}; #[cfg(test)] pub(crate) type Address = bigint::hash::H160; diff --git a/util/network/src/lib.rs b/util/network/src/lib.rs index bc5459d5a..a69663121 100644 --- a/util/network/src/lib.rs +++ b/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::{is_valid_node_url, validate_node_url, NodeId}; use ipnetwork::{IpNetwork, IpNetworkError}; use std::str::FromStr; diff --git a/util/network/src/node_table.rs b/util/network/src/node_table.rs index 94ad4d30a..a1a1903d3 100644 --- a/util/network/src/node_table.rs +++ b/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::*; @@ -368,6 +368,15 @@ pub fn is_valid_node_url(url: &str) -> bool { Node::from_str(url).is_ok() } +/// Same as `is_valid_node_url` but returns detailed `NetworkError` +pub fn validate_node_url(url: &str) -> Option { + use std::str::FromStr; + match Node::from_str(url) { + Ok(_) => None, + Err(e) => Some(e) + } +} + #[cfg(test)] mod tests { use super::*; From 851401dded71cde146e0dfc389fed3f462cda79f Mon Sep 17 00:00:00 2001 From: Dmitry Kashitsyn Date: Mon, 6 Nov 2017 13:01:37 +0700 Subject: [PATCH 2/3] Removes obsolete is_valid_node_url() --- sync/src/lib.rs | 2 +- util/network/src/lib.rs | 2 +- util/network/src/node_table.rs | 8 +------- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/sync/src/lib.rs b/sync/src/lib.rs index 45f7a8d98..0495d72a7 100644 --- a/sync/src/lib.rs +++ b/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, validate_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; diff --git a/util/network/src/lib.rs b/util/network/src/lib.rs index a69663121..6cf81fa9d 100644 --- a/util/network/src/lib.rs +++ b/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, validate_node_url, NodeId}; +pub use node_table::{validate_node_url, NodeId}; use ipnetwork::{IpNetwork, IpNetworkError}; use std::str::FromStr; diff --git a/util/network/src/node_table.rs b/util/network/src/node_table.rs index a1a1903d3..9ac0ac9f6 100644 --- a/util/network/src/node_table.rs +++ b/util/network/src/node_table.rs @@ -363,12 +363,6 @@ impl Drop for NodeTable { } /// Check if node url is valid -pub fn is_valid_node_url(url: &str) -> bool { - use std::str::FromStr; - Node::from_str(url).is_ok() -} - -/// Same as `is_valid_node_url` but returns detailed `NetworkError` pub fn validate_node_url(url: &str) -> Option { use std::str::FromStr; match Node::from_str(url) { @@ -399,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_some()); let node = Node::from_str("enode://a979fb575495b8d6db44f750317d0f4622bf4c2aa3365d6af7c284339968eef29b69ad0dce72a4d8db5ebb4968de0e3bec910127f134779fbcb0cb6d3331163c@22.99.55.44:7770"); assert!(node.is_ok()); let node = node.unwrap(); From 8fe40a64d0a2da818be411e22d303a2e1d003870 Mon Sep 17 00:00:00 2001 From: Dmitry Kashitsyn Date: Mon, 6 Nov 2017 13:51:26 +0700 Subject: [PATCH 3/3] Fixes test --- util/network/src/node_table.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/network/src/node_table.rs b/util/network/src/node_table.rs index 9ac0ac9f6..6ba598291 100644 --- a/util/network/src/node_table.rs +++ b/util/network/src/node_table.rs @@ -393,7 +393,7 @@ mod tests { #[test] fn node_parse() { - assert!(validate_node_url("enode://a979fb575495b8d6db44f750317d0f4622bf4c2aa3365d6af7c284339968eef29b69ad0dce72a4d8db5ebb4968de0e3bec910127f134779fbcb0cb6d3331163c@22.99.55.44:7770").is_some()); + 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();