From aa8b871e490f8e1eec7e20e9a00b125fe01bd27f Mon Sep 17 00:00:00 2001 From: debris Date: Mon, 5 Sep 2016 17:41:34 +0200 Subject: [PATCH] handling invalid spec jsons properly, additional tests, closes #1840 --- ethcore/src/engines/basic_authority.rs | 5 ++++- ethcore/src/engines/instant_seal.rs | 5 ++++- ethcore/src/ethereum/mod.rs | 20 ++++++++++++-------- ethcore/src/spec/spec.rs | 21 +++++++++++++++------ parity/params.rs | 7 +++++-- util/src/misc.rs | 11 +---------- 6 files changed, 41 insertions(+), 28 deletions(-) diff --git a/ethcore/src/engines/basic_authority.rs b/ethcore/src/engines/basic_authority.rs index eef3df6b1..18dfeec46 100644 --- a/ethcore/src/engines/basic_authority.rs +++ b/ethcore/src/engines/basic_authority.rs @@ -187,7 +187,10 @@ mod tests { use spec::Spec; /// Create a new test chain spec with `BasicAuthority` consensus engine. - fn new_test_authority() -> Spec { Spec::load(include_bytes!("../../res/test_authority.json")) } + fn new_test_authority() -> Spec { + let bytes: &[u8] = include_bytes!("../../res/test_authority.json"); + Spec::load(bytes).expect("invalid chain spec") + } #[test] fn has_valid_metadata() { diff --git a/ethcore/src/engines/instant_seal.rs b/ethcore/src/engines/instant_seal.rs index bdb882ee7..3c95f3465 100644 --- a/ethcore/src/engines/instant_seal.rs +++ b/ethcore/src/engines/instant_seal.rs @@ -72,7 +72,10 @@ mod tests { use block::*; /// Create a new test chain spec with `BasicAuthority` consensus engine. - fn new_test_instant() -> Spec { Spec::load(include_bytes!("../../res/instant_seal.json")) } + fn new_test_instant() -> Spec { + let bytes: &[u8] = include_bytes!("../../res/instant_seal.json"); + Spec::load(bytes).expect("invalid chain spec") + } #[test] fn instant_can_seal() { diff --git a/ethcore/src/ethereum/mod.rs b/ethcore/src/ethereum/mod.rs index 1efe001e5..6d46d5551 100644 --- a/ethcore/src/ethereum/mod.rs +++ b/ethcore/src/ethereum/mod.rs @@ -29,29 +29,33 @@ pub use self::denominations::*; use super::spec::*; +fn load(b: &[u8]) -> Spec { + Spec::load(b).expect("chain spec is invalid") +} + /// Create a new Olympic chain spec. -pub fn new_olympic() -> Spec { Spec::load(include_bytes!("../../res/ethereum/olympic.json")) } +pub fn new_olympic() -> Spec { load(include_bytes!("../../res/ethereum/olympic.json")) } /// Create a new Frontier mainnet chain spec. -pub fn new_frontier() -> Spec { Spec::load(include_bytes!("../../res/ethereum/frontier.json")) } +pub fn new_frontier() -> Spec { load(include_bytes!("../../res/ethereum/frontier.json")) } /// Create a new Frontier mainnet chain spec without the DAO hardfork. -pub fn new_classic() -> Spec { Spec::load(include_bytes!("../../res/ethereum/classic.json")) } +pub fn new_classic() -> Spec { load(include_bytes!("../../res/ethereum/classic.json")) } /// Create a new Frontier chain spec as though it never changes to Homestead. -pub fn new_frontier_test() -> Spec { Spec::load(include_bytes!("../../res/ethereum/frontier_test.json")) } +pub fn new_frontier_test() -> Spec { load(include_bytes!("../../res/ethereum/frontier_test.json")) } /// Create a new Homestead chain spec as though it never changed from Frontier. -pub fn new_homestead_test() -> Spec { Spec::load(include_bytes!("../../res/ethereum/homestead_test.json")) } +pub fn new_homestead_test() -> Spec { load(include_bytes!("../../res/ethereum/homestead_test.json")) } /// Create a new Frontier/Homestead/DAO chain spec with transition points at #5 and #8. -pub fn new_daohardfork_test() -> Spec { Spec::load(include_bytes!("../../res/ethereum/daohardfork_test.json")) } +pub fn new_daohardfork_test() -> Spec { load(include_bytes!("../../res/ethereum/daohardfork_test.json")) } /// Create a new Frontier main net chain spec without genesis accounts. -pub fn new_mainnet_like() -> Spec { Spec::load(include_bytes!("../../res/ethereum/frontier_like_test.json")) } +pub fn new_mainnet_like() -> Spec { load(include_bytes!("../../res/ethereum/frontier_like_test.json")) } /// Create a new Morden chain spec. -pub fn new_morden() -> Spec { Spec::load(include_bytes!("../../res/ethereum/morden.json")) } +pub fn new_morden() -> Spec { load(include_bytes!("../../res/ethereum/morden.json")) } #[cfg(test)] mod tests { diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index d80ac0e33..58317e97b 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -244,18 +244,21 @@ impl Spec { } /// Loads spec from json file. - pub fn load(reader: &[u8]) -> Self { - From::from(ethjson::spec::Spec::load(reader).expect("invalid json file")) + pub fn load(reader: R) -> Result where R: Read { + match ethjson::spec::Spec::load(reader) { + Ok(spec) => Ok(spec.into()), + _ => Err("Spec json is invalid".into()), + } } /// Create a new Spec which conforms to the Frontier-era Morden chain except that it's a NullEngine consensus. - pub fn new_test() -> Spec { - Spec::load(include_bytes!("../../res/null_morden.json")) + pub fn new_test() -> Self { + Spec::load(include_bytes!("../../res/null_morden.json") as &[u8]).expect("null_morden.json is invalid") } /// Create a new Spec which is a NullEngine consensus with a premine of address whose secret is sha3(''). - pub fn new_null() -> Spec { - Spec::load(include_bytes!("../../res/null.json")) + pub fn new_null() -> Self { + Spec::load(include_bytes!("../../res/null.json") as &[u8]).expect("null.json is invalid") } } @@ -267,6 +270,12 @@ mod tests { use views::*; use super::*; + // https://github.com/ethcore/parity/issues/1840 + #[test] + fn test_load_empty() { + assert!(Spec::load(&vec![] as &[u8]).is_err()); + } + #[test] fn test_chain() { let test_spec = Spec::new_test(); diff --git a/parity/params.rs b/parity/params.rs index 54a680414..c67520aa1 100644 --- a/parity/params.rs +++ b/parity/params.rs @@ -17,7 +17,7 @@ use std::str::FromStr; use std::fs; use std::time::Duration; -use util::{contents, H256, Address, U256, version_data}; +use util::{H256, Address, U256, version_data}; use util::journaldb::Algorithm; use ethcore::spec::Spec; use ethcore::ethereum; @@ -61,7 +61,10 @@ impl SpecType { SpecType::Testnet => Ok(ethereum::new_morden()), SpecType::Olympic => Ok(ethereum::new_olympic()), SpecType::Classic => Ok(ethereum::new_classic()), - SpecType::Custom(ref file) => Ok(Spec::load(&try!(contents(file).map_err(|_| "Could not load specification file.")))) + SpecType::Custom(ref filename) => { + let file = try!(fs::File::open(filename).map_err(|_| "Could not load specification file.")); + Spec::load(file) + } } } } diff --git a/util/src/misc.rs b/util/src/misc.rs index 62e8542db..50b2e7e8d 100644 --- a/util/src/misc.rs +++ b/util/src/misc.rs @@ -16,7 +16,6 @@ //! Diff misc. -use std::fs::File; use common::*; use rlp::{Stream, RlpStream}; use target_info::Target; @@ -33,14 +32,6 @@ pub enum Filth { Dirty, } -/// Read the whole contents of a file `name`. -pub fn contents(name: &str) -> Result { - let mut file = try!(File::open(name)); - let mut ret: Vec = Vec::new(); - try!(file.read_to_end(&mut ret)); - Ok(ret) -} - /// Get the standard version string for this software. pub fn version() -> String { let sha3 = short_sha(); @@ -64,4 +55,4 @@ pub fn version_data() -> Bytes { s.append(&rustc_version()); s.append(&&Target::os()[0..2]); s.out() -} \ No newline at end of file +}