Tests and grumbles

ipfs/src/error.rs

@@ -28,6 +28,7 @@ pub enum ServerError {
 	Other(hyper::error::Error),
 }
 
+#[derive(Debug, PartialEq)]
 pub enum Error {
 	CidParsingFailed,
 	UnsupportedHash,
@@ -37,6 +38,8 @@ pub enum Error {
 	StateRootNotFound,
 }
 
+/// Convert Error into Out, handy when switching from Rust's Result-based
+/// error handling to Hyper's request handling.
 impl From<Error> for Out {
 	fn from(err: Error) -> Out {
 		use self::Error::*;
@@ -52,18 +55,21 @@ impl From<Error> for Out {
 	}
 }
 
+/// Convert Content ID errors.
 impl From<cid::Error> for Error {
 	fn from(_: cid::Error) -> Error {
 		Error::CidParsingFailed
 	}
 }
 
+/// Convert multihash errors (multihash being part of CID).
 impl From<multihash::Error> for Error {
 	fn from(_: multihash::Error) -> Error {
 		Error::CidParsingFailed
 	}
 }
 
+/// Handle IO errors (ports taken when starting the server).
 impl From<::std::io::Error> for ServerError {
 	fn from(err: ::std::io::Error) -> ServerError {
 		ServerError::IoError(err)

ipfs/src/handler.rs

@@ -19,7 +19,6 @@ use error::{Error, Result};
 use cid::{ToCid, Codec};
 
 use std::sync::Arc;
-use std::ops::Deref;
 use multihash::Hash;
 use hyper::Next;
 use util::{Bytes, H256};
@@ -27,12 +26,15 @@ use ethcore::client::{BlockId, TransactionId, BlockChainClient};
 
 type Reason = &'static str;
 
+/// Keeps the state of the response to send out
+#[derive(Debug, PartialEq)]
 pub enum Out {
 	OctetStream(Bytes),
 	NotFound(Reason),
 	Bad(Reason),
 }
 
+/// Request/response handler
 pub struct IpfsHandler {
 	client: Arc<BlockChainClient>,
 	out: Out,
@@ -42,34 +44,34 @@ impl IpfsHandler {
 	pub fn new(client: Arc<BlockChainClient>) -> Self {
 		IpfsHandler {
 			client: client,
-			out: Out::NotFound("Route not found")
+			out: Out::Bad("Invalid Request")
 		}
 	}
 
+	/// Exposes the outgoing state. The outgoing state should be immutable from the outside.
 	pub fn out(&self) -> &Out {
 		&self.out
 	}
 
+	/// Route path + query string to a specialized method
 	pub fn route(&mut self, path: &str, query: Option<&str>) -> Next {
-		let result = match path {
-			"/api/v0/block/get" => self.route_cid(query),
-			_ => return Next::write(),
+		self.out = match path {
+			"/api/v0/block/get" => {
+				let arg = query.and_then(|q| get_param(q, "arg")).unwrap_or("");
+
+				self.route_cid(arg).unwrap_or_else(Into::into)
+			},
+
+			_ => Out::NotFound("Route not found")
 		};
 
-		match result {
-			Ok(_) => Next::write(),
-			Err(err) => {
-				self.out = err.into();
-
-				Next::write()
-			}
-		}
+		Next::write()
 	}
 
-	fn route_cid(&mut self, query: Option<&str>) -> Result<()> {
-		let query = query.unwrap_or("");
-
-		let cid = get_param(&query, "arg").ok_or(Error::CidParsingFailed)?.to_cid()?;
+	/// Attempt to read Content ID from `arg` query parameter, get a hash and
+	/// route further by the CID's codec.
+	fn route_cid(&self, cid: &str) -> Result<Out> {
+		let cid = cid.to_cid()?;
 
 		let mh = multihash::decode(&cid.hash)?;
 
@@ -78,51 +80,47 @@ impl IpfsHandler {
 		let hash: H256 = mh.digest.into();
 
 		match cid.codec {
-			Codec::EthereumBlock => self.get_block(hash),
-			Codec::EthereumBlockList => self.get_block_list(hash),
-			Codec::EthereumTx => self.get_transaction(hash),
-			Codec::EthereumStateTrie => self.get_state_trie(hash),
+			Codec::EthereumBlock => self.block(hash),
+			Codec::EthereumBlockList => self.block_list(hash),
+			Codec::EthereumTx => self.transaction(hash),
+			Codec::EthereumStateTrie => self.state_trie(hash),
 			_ => return Err(Error::UnsupportedCid),
 		}
 	}
 
-	fn get_block(&mut self, hash: H256) -> Result<()> {
+	/// Get block header by hash as raw binary.
+	fn block(&self, hash: H256) -> Result<Out> {
 		let block_id = BlockId::Hash(hash);
 		let block = self.client.block_header(block_id).ok_or(Error::BlockNotFound)?;
 
-		self.out = Out::OctetStream(block.into_inner());
-
-		Ok(())
+		Ok(Out::OctetStream(block.into_inner()))
 	}
 
-	fn get_block_list(&mut self, hash: H256) -> Result<()> {
-		let ommers = self.client.find_uncles(&hash).ok_or(Error::BlockNotFound)?;
+	/// Get list of block ommers by hash as raw binary.
+	fn block_list(&self, hash: H256) -> Result<Out> {
+		let uncles = self.client.find_uncles(&hash).ok_or(Error::BlockNotFound)?;
 
-		self.out = Out::OctetStream(rlp::encode(&ommers).to_vec());
-
-		Ok(())
+		Ok(Out::OctetStream(rlp::encode(&uncles).to_vec()))
 	}
 
-	fn get_transaction(&mut self, hash: H256) -> Result<()> {
+	/// Get transaction by hash and return as raw binary.
+	fn transaction(&self, hash: H256) -> Result<Out> {
 		let tx_id = TransactionId::Hash(hash);
 		let tx = self.client.transaction(tx_id).ok_or(Error::TransactionNotFound)?;
 
-		self.out = Out::OctetStream(rlp::encode(tx.deref()).to_vec());
-
-		Ok(())
+		Ok(Out::OctetStream(rlp::encode(&*tx).to_vec()))
 	}
 
-	fn get_state_trie(&mut self, hash: H256) -> Result<()> {
+	/// Get state trie node by hash and return as raw binary.
+	fn state_trie(&self, hash: H256) -> Result<Out> {
 		let data = self.client.state_data(&hash).ok_or(Error::StateRootNotFound)?;
 
-		self.out = Out::OctetStream(data);
-
-		Ok(())
+		Ok(Out::OctetStream(data))
 	}
 }
 
 /// Get a query parameter's value by name.
-pub fn get_param<'a>(query: &'a str, name: &str) -> Option<&'a str> {
+fn get_param<'a>(query: &'a str, name: &str) -> Option<&'a str> {
 	query.split('&')
 		.find(|part| part.starts_with(name) && part[name.len()..].starts_with("="))
 		.map(|part| &part[name.len() + 1..])
@@ -131,8 +129,13 @@ pub fn get_param<'a>(query: &'a str, name: &str) -> Option<&'a str> {
 #[cfg(test)]
 mod tests {
 	use super::*;
+	use ethcore::client::TestBlockChainClient;
 
-   #[test]
+	fn get_mocked_handler() -> IpfsHandler {
+		IpfsHandler::new(Arc::new(TestBlockChainClient::new()))
+	}
+
+	#[test]
 	fn test_get_param() {
 		let query = "foo=100&bar=200&qux=300";
 
@@ -141,5 +144,105 @@ mod tests {
 		assert_eq!(get_param(query, "qux"), Some("300"));
 		assert_eq!(get_param(query, "bar="), None);
 		assert_eq!(get_param(query, "200"), None);
+		assert_eq!(get_param("", "foo"), None);
+		assert_eq!(get_param("foo", "foo"), None);
+		assert_eq!(get_param("foo&bar", "foo"), None);
+		assert_eq!(get_param("bar&foo", "foo"), None);
+	}
+
+	#[test]
+	fn cid_route_block() {
+		let handler = get_mocked_handler();
+
+		// `eth-block` with Keccak-256
+		let cid = "z43AaGF5tmkT9SEX6urrhwpEW5ZSaACY73Vw357ZXTsur2fR8BM";
+
+		assert_eq!(Err(Error::BlockNotFound), handler.route_cid(cid));
+	}
+
+	#[test]
+	fn cid_route_block_list() {
+		let handler = get_mocked_handler();
+
+		// `eth-block-list` with Keccak-256
+		let cid = "z43c7o7FsNxqdLJW8Ucj19tuCALtnmUb2EkDptj4W6xSkFVTqWs";
+
+		assert_eq!(Err(Error::BlockNotFound), handler.route_cid(cid));
+	}
+
+	#[test]
+	fn cid_route_tx() {
+		let handler = get_mocked_handler();
+
+		// `eth-tx` with Keccak-256
+		let cid = "z44VCrqbpbPcb8SUBc8Tba4EaKuoDz2grdEoQXx4TP7WYh9ZGBu";
+
+		assert_eq!(Err(Error::TransactionNotFound), handler.route_cid(cid));
+	}
+
+	#[test]
+	fn cid_route_state_trie() {
+		let handler = get_mocked_handler();
+
+		// `eth-state-trie` with Keccak-256
+		let cid = "z45oqTS7kR2n2peRGJQ4VCJEeaG9sorqcCyfmznZPJM7FMdhQCT";
+
+		assert_eq!(Err(Error::StateRootNotFound), handler.route_cid(&cid));
+	}
+
+	#[test]
+	fn cid_route_invalid_hash() {
+		let handler = get_mocked_handler();
+
+		// `eth-block` with SHA3-256 hash
+		let cid = "z43Aa9gr1MM7TENJh4Em9d9Ttr7p3UcfyMpNei6WLVeCmSEPu8F";
+
+		assert_eq!(Err(Error::UnsupportedHash), handler.route_cid(cid));
+	}
+
+	#[test]
+	fn cid_route_invalid_codec() {
+		let handler = get_mocked_handler();
+
+		// `bitcoin-block` with Keccak-256
+		let cid = "z4HFyHvb8CarYARyxz4cCcPaciduXd49TFPCKLhYmvNxf7Auvwu";
+
+		assert_eq!(Err(Error::UnsupportedCid), handler.route_cid(&cid));
+	}
+
+	#[test]
+	fn route_block() {
+		let mut handler = get_mocked_handler();
+
+		let _ = handler.route("/api/v0/block/get", Some("arg=z43AaGF5tmkT9SEX6urrhwpEW5ZSaACY73Vw357ZXTsur2fR8BM"));
+
+		assert_eq!(handler.out(), &Out::NotFound("Block not found"));
+	}
+
+	#[test]
+	fn route_block_missing_query() {
+		let mut handler = get_mocked_handler();
+
+		let _ = handler.route("/api/v0/block/get", None);
+
+		assert_eq!(handler.out(), &Out::Bad("CID parsing failed"));
+	}
+
+	#[test]
+	fn route_block_invalid_query() {
+		let mut handler = get_mocked_handler();
+
+		let _ = handler.route("/api/v0/block/get", Some("arg=foobarz43AaGF5tmkT9SEX6urrhwpEW5ZSaACY73Vw357ZXTsur2fR8BM"));
+
+		assert_eq!(handler.out(), &Out::Bad("CID parsing failed"));
+	}
+
+	#[test]
+	fn route_invalid_route() {
+		let mut handler = get_mocked_handler();
+
+		let _ = handler.route("/foo/bar/baz", Some("arg=z43AaGF5tmkT9SEX6urrhwpEW5ZSaACY73Vw357ZXTsur2fR8BM"));
+
+		assert_eq!(handler.out(), &Out::NotFound("Route not found"));
 	}
 }

ipfs/src/lib.rs

@@ -34,6 +34,7 @@ use hyper::header::{ContentLength, ContentType};
 use hyper::{Next, Encoder, Decoder, Method, RequestUri, StatusCode};
 use ethcore::client::BlockChainClient;
 
+/// Implement Hyper's HTTP handler
 impl Handler<HttpStream> for IpfsHandler {
 	fn on_request(&mut self, req: Request<HttpStream>) -> Next {
 		if *req.method() != Method::Get {
@@ -109,15 +110,16 @@ impl Handler<HttpStream> for IpfsHandler {
 pub fn start_server(client: Arc<BlockChainClient>) -> Result<Listening, ServerError> {
 	let addr = "0.0.0.0:5001".parse().expect("can't fail on static input; qed");
 
-	hyper::Server::http(&addr)?
-		.handle(move |_| IpfsHandler::new(client.clone()))
-		.map(|(listening, srv)| {
+	Ok(
+		hyper::Server::http(&addr)?
+			.handle(move |_| IpfsHandler::new(client.clone()))
+			.map(|(listening, srv)| {
 
-			::std::thread::spawn(move || {
-				srv.run();
-			});
+				::std::thread::spawn(move || {
+					srv.run();
+				});
 
-			listening
-		})
-		.map_err(Into::into)
+				listening
+			})?
+	)
 }

parity/run.rs

@@ -421,7 +421,6 @@ pub fn execute(cmd: RunCmd, can_restart: bool, logger: Arc<RotatingLogger>) -> R
 	};
 	let signer_server = signer::start(cmd.signer_conf.clone(), signer_deps)?;
 
-
 	// the ipfs server
 	let ipfs_server = ipfs::start_server(client.clone())?;