From b37124991cf7691777b66ce0efda3145d25d8849 Mon Sep 17 00:00:00 2001
From: Robert Habermeier <rphmeier@gmail.com>
Date: Tue, 7 Feb 2017 16:13:56 +0100
Subject: [PATCH] enforce validity of on_demand requests

---
 ethcore/light/src/on_demand/mod.rs     | 17 ++++---------
 ethcore/light/src/on_demand/request.rs | 35 +++++++++++++++++++++-----
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/ethcore/light/src/on_demand/mod.rs b/ethcore/light/src/on_demand/mod.rs
index e797fffc8..7e258282b 100644
--- a/ethcore/light/src/on_demand/mod.rs
+++ b/ethcore/light/src/on_demand/mod.rs
@@ -43,8 +43,6 @@ pub enum Error {
 	Canceled,
 	/// No suitable peers available to serve the request.
 	NoPeersAvailable,
-	/// Invalid request.
-	InvalidRequest,
 	/// Request timed out.
 	TimedOut,
 }
@@ -95,6 +93,7 @@ enum Pending {
 pub struct OnDemand {
 	peers: RwLock<HashMap<PeerId, Peer>>,
 	pending_requests: RwLock<HashMap<ReqId, Pending>>,
+	orphaned_requests: RwLock<Vec<Pending>>,
 }
 
 impl Default for OnDemand {
@@ -102,6 +101,7 @@ impl Default for OnDemand {
 		OnDemand {
 			peers: RwLock::new(HashMap::new()),
 			pending_requests: RwLock::new(HashMap::new()),
+			orphaned_requests: RwLock::new(Vec::new()),
 		}
 	}
 }
@@ -117,20 +117,13 @@ impl OnDemand {
 
 	// dispatch the request, completing the request if no peers available.
 	fn dispatch_header_by_number(&self, ctx: &BasicContext, req: request::HeaderByNumber, sender: Sender<(encoded::Header, U256)>) {
-		let num = req.num;
-		let cht_num = match ::cht::block_to_cht_number(req.num) {
-			Some(cht_num) => cht_num,
-			None => {
-				warn!(target: "on_demand", "Attempted to dispatch invalid header proof: req.num == 0");
-				sender.complete(Err(Error::InvalidRequest));
-				return;
-			}
-		};
+		let num = req.num();
+		let cht_num = req.cht_num();
 
 		let les_req = LesRequest::HeaderProofs(les_request::HeaderProofs {
 			requests: vec![les_request::HeaderProof {
 				cht_number: cht_num,
-				block_number: req.num,
+				block_number: num,
 				from_level: 0,
 			}],
 		});
diff --git a/ethcore/light/src/on_demand/request.rs b/ethcore/light/src/on_demand/request.rs
index b6f3f077c..3964137d9 100644
--- a/ethcore/light/src/on_demand/request.rs
+++ b/ethcore/light/src/on_demand/request.rs
@@ -59,12 +59,33 @@ impl From<Box<TrieError>> for Error {
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct HeaderByNumber {
 	/// The header's number.
-	pub num: u64,
+	num: u64,
+	/// The cht number for the given block number.
+	cht_num: u64,
 	/// The root of the CHT containing this header.
-	pub cht_root: H256,
+	cht_root: H256,
 }
 
 impl HeaderByNumber {
+	/// Construct a new header-by-number request. Fails if the given number is 0.
+	/// Provide the expected CHT root to compare against.
+	pub fn new(num: u64, cht_root: H256) -> Option<Self> {
+		::cht::block_to_cht_number(num).map(|cht_num| HeaderByNumber {
+			num: num,
+			cht_num: cht_num,
+			cht_root: cht_root,
+		})
+	}
+
+	/// Access the requested block number.
+	pub fn num(&self) -> u64 { self.num }
+
+	/// Access the CHT number.
+	pub fn cht_num(&self) -> u64 { self.cht_num }
+
+	/// Access the expected CHT root.
+	pub fn cht_root(&self) -> H256 { self.cht_root }
+
 	/// Check a response with a header and cht proof.
 	pub fn check_response(&self, header: &[u8], proof: &[Bytes]) -> Result<(encoded::Header, U256), Error> {
 		let (expected_hash, td) = match ::cht::check_proof(proof, self.num, self.cht_root) {
@@ -222,6 +243,11 @@ mod tests {
 	use ethcore::encoded;
 	use ethcore::receipt::Receipt;
 
+	#[test]
+	fn no_invalid_header_by_number() {
+		assert!(HeaderByNumber::new(0, Default::default()).is_none())
+	}
+
 	#[test]
 	fn check_header_by_number() {
 		use ::cht;
@@ -244,10 +270,7 @@ mod tests {
 		};
 
 		let proof = cht.prove(10_000, 0).unwrap().unwrap();
-		let req = HeaderByNumber {
-			num: 10_000,
-			cht_root: cht.root(),
-		};
+		let req = HeaderByNumber::new(10_000, cht.root()).unwrap();
 
 		let raw_header = test_client.block_header(::ethcore::ids::BlockId::Number(10_000)).unwrap();