Improve API handlers (#1364)

* Return different status codes depending on result, not just BAD_REQUEST, fixes #1247
* Reduce number of unwraps
* Refactoring
This commit is contained in:
hashmap 2018-08-17 19:04:02 +02:00 committed by Ignotus Peverell
parent 54d3fc0af2
commit f7161a9afb
2 changed files with 189 additions and 172 deletions

View file

@ -97,9 +97,14 @@ impl OutputHandler {
Err(ErrorKind::NotFound)? Err(ErrorKind::NotFound)?
} }
fn outputs_by_ids(&self, req: &Request<Body>) -> Vec<Output> { fn outputs_by_ids(&self, req: &Request<Body>) -> Result<Vec<Output>, Error> {
let mut commitments: Vec<String> = vec![]; let mut commitments: Vec<String> = vec![];
let params = form_urlencoded::parse(req.uri().query().unwrap().as_bytes())
let query = match req.uri().query() {
Some(q) => q,
None => return Err(ErrorKind::RequestError("no query string".to_owned()))?,
};
let params = form_urlencoded::parse(query.as_bytes())
.into_owned() .into_owned()
.collect::<Vec<(String, String)>>(); .collect::<Vec<(String, String)>>();
@ -119,7 +124,7 @@ impl OutputHandler {
outputs.push(output); outputs.push(output);
} }
} }
outputs Ok(outputs)
} }
fn outputs_at_height( fn outputs_at_height(
@ -127,58 +132,49 @@ impl OutputHandler {
block_height: u64, block_height: u64,
commitments: Vec<Commitment>, commitments: Vec<Commitment>,
include_proof: bool, include_proof: bool,
) -> BlockOutputs { ) -> Result<BlockOutputs, Error> {
let header = w(&self.chain).get_header_by_height(block_height).unwrap(); let header = w(&self.chain)
.get_header_by_height(block_height)
.map_err(|_| ErrorKind::NotFound)?;
// TODO - possible to compact away blocks we care about // TODO - possible to compact away blocks we care about
// in the period between accepting the block and refreshing the wallet // in the period between accepting the block and refreshing the wallet
if let Ok(block) = w(&self.chain).get_block(&header.hash()) { let block = w(&self.chain)
.get_block(&header.hash())
.map_err(|_| ErrorKind::NotFound)?;
let outputs = block let outputs = block
.outputs() .outputs()
.iter() .iter()
.filter(|output| commitments.is_empty() || commitments.contains(&output.commit)) .filter(|output| commitments.is_empty() || commitments.contains(&output.commit))
.map(|output| { .map(|output| {
OutputPrintable::from_output( OutputPrintable::from_output(output, w(&self.chain), Some(&header), include_proof)
output, }).collect();
w(&self.chain),
Some(&header),
include_proof,
)
})
.collect();
BlockOutputs { Ok(BlockOutputs {
header: BlockHeaderInfo::from_header(&header), header: BlockHeaderInfo::from_header(&header),
outputs: outputs, outputs: outputs,
} })
} else {
debug!(
LOGGER,
"could not find block {:?} at height {}, maybe compacted?",
&header.hash(),
block_height,
);
BlockOutputs {
header: BlockHeaderInfo::from_header(&header),
outputs: vec![],
}
}
} }
// returns outputs for a specified range of blocks // returns outputs for a specified range of blocks
fn outputs_block_batch(&self, req: &Request<Body>) -> Vec<BlockOutputs> { fn outputs_block_batch(&self, req: &Request<Body>) -> Result<Vec<BlockOutputs>, Error> {
let mut commitments: Vec<Commitment> = vec![]; let mut commitments: Vec<Commitment> = vec![];
let mut start_height = 1; let mut start_height = 1;
let mut end_height = 1; let mut end_height = 1;
let mut include_rp = false; let mut include_rp = false;
let params = form_urlencoded::parse(req.uri().query().unwrap().as_bytes()) let query = match req.uri().query() {
.into_owned() Some(q) => q,
.fold(HashMap::new(), |mut hm, (k, v)| { None => return Err(ErrorKind::RequestError("no query string".to_owned()))?,
};
let params = form_urlencoded::parse(query.as_bytes()).into_owned().fold(
HashMap::new(),
|mut hm, (k, v)| {
hm.entry(k).or_insert(vec![]).push(v); hm.entry(k).or_insert(vec![]).push(v);
hm hm
}); },
);
if let Some(ids) = params.get("id") { if let Some(ids) = params.get("id") {
for id in ids { for id in ids {
@ -191,12 +187,16 @@ impl OutputHandler {
} }
if let Some(heights) = params.get("start_height") { if let Some(heights) = params.get("start_height") {
for height in heights { for height in heights {
start_height = height.parse().unwrap(); start_height = height
.parse()
.map_err(|_| ErrorKind::RequestError("invalid start_height".to_owned()))?;
} }
} }
if let Some(heights) = params.get("end_height") { if let Some(heights) = params.get("end_height") {
for height in heights { for height in heights {
end_height = height.parse().unwrap(); end_height = height
.parse()
.map_err(|_| ErrorKind::RequestError("invalid end_height".to_owned()))?;
} }
} }
if let Some(_) = params.get("include_rp") { if let Some(_) = params.get("include_rp") {
@ -214,28 +214,26 @@ impl OutputHandler {
let mut return_vec = vec![]; let mut return_vec = vec![];
for i in (start_height..=end_height).rev() { for i in (start_height..=end_height).rev() {
let res = self.outputs_at_height(i, commitments.clone(), include_rp); if let Ok(res) = self.outputs_at_height(i, commitments.clone(), include_rp) {
if res.outputs.len() > 0 { if res.outputs.len() > 0 {
return_vec.push(res); return_vec.push(res);
} }
} }
}
return_vec Ok(return_vec)
} }
} }
impl Handler for OutputHandler { impl Handler for OutputHandler {
fn get(&self, req: Request<Body>) -> ResponseFuture { fn get(&self, req: Request<Body>) -> ResponseFuture {
match req let command = match req.uri().path().trim_right_matches("/").rsplit("/").next() {
.uri() Some(c) => c,
.path() None => return response(StatusCode::BAD_REQUEST, "invalid url"),
.trim_right_matches("/") };
.rsplit("/") match command {
.next() "byids" => result_to_response(self.outputs_by_ids(&req)),
.unwrap() "byheight" => result_to_response(self.outputs_block_batch(&req)),
{
"byids" => json_response(&self.outputs_by_ids(&req)),
"byheight" => json_response(&self.outputs_block_batch(&req)),
_ => response(StatusCode::BAD_REQUEST, ""), _ => response(StatusCode::BAD_REQUEST, ""),
} }
} }
@ -282,15 +280,15 @@ impl TxHashSetHandler {
} }
// allows traversal of utxo set // allows traversal of utxo set
fn outputs(&self, start_index: u64, mut max: u64) -> OutputListing { fn outputs(&self, start_index: u64, mut max: u64) -> Result<OutputListing, Error> {
//set a limit here //set a limit here
if max > 1000 { if max > 1000 {
max = 1000; max = 1000;
} }
let outputs = w(&self.chain) let outputs = w(&self.chain)
.unspent_outputs_by_insertion_index(start_index, max) .unspent_outputs_by_insertion_index(start_index, max)
.unwrap(); .context(ErrorKind::NotFound)?;
OutputListing { Ok(OutputListing {
last_retrieved_index: outputs.0, last_retrieved_index: outputs.0,
highest_index: outputs.1, highest_index: outputs.1,
outputs: outputs outputs: outputs
@ -298,7 +296,7 @@ impl TxHashSetHandler {
.iter() .iter()
.map(|x| OutputPrintable::from_output(x, w(&self.chain), None, true)) .map(|x| OutputPrintable::from_output(x, w(&self.chain), None, true))
.collect(), .collect(),
} })
} }
// return a dummy output with merkle proof for position filled out // return a dummy output with merkle proof for position filled out
@ -309,7 +307,8 @@ impl TxHashSetHandler {
id id
)))?; )))?;
let commit = Commitment::from_vec(c); let commit = Commitment::from_vec(c);
let merkle_proof = chain::Chain::get_merkle_proof_for_pos(&w(&self.chain), commit).unwrap(); let merkle_proof = chain::Chain::get_merkle_proof_for_pos(&w(&self.chain), commit)
.map_err(|_| ErrorKind::NotFound)?;
Ok(OutputPrintable { Ok(OutputPrintable {
output_type: OutputType::Coinbase, output_type: OutputType::Coinbase,
commit: Commitment::from_vec(vec![]), commit: Commitment::from_vec(vec![]),
@ -363,21 +362,25 @@ impl Handler for TxHashSetHandler {
} }
} }
} }
match req let command = match req
.uri() .uri()
.path() .path()
.trim_right() .trim_right()
.trim_right_matches("/") .trim_right_matches("/")
.rsplit("/") .rsplit("/")
.next() .next()
.unwrap()
{ {
Some(c) => c,
None => return response(StatusCode::BAD_REQUEST, "invalid url"),
};
match command {
"roots" => json_response_pretty(&self.get_roots()), "roots" => json_response_pretty(&self.get_roots()),
"lastoutputs" => json_response_pretty(&self.get_last_n_output(last_n)), "lastoutputs" => json_response_pretty(&self.get_last_n_output(last_n)),
"lastrangeproofs" => json_response_pretty(&self.get_last_n_rangeproof(last_n)), "lastrangeproofs" => json_response_pretty(&self.get_last_n_rangeproof(last_n)),
"lastkernels" => json_response_pretty(&self.get_last_n_kernel(last_n)), "lastkernels" => json_response_pretty(&self.get_last_n_kernel(last_n)),
"outputs" => json_response_pretty(&self.outputs(start_index, max)), "outputs" => result_to_response(self.outputs(start_index, max)),
"merkleproof" => json_response_pretty(&self.get_merkle_proof_for_output(&id).unwrap()), "merkleproof" => result_to_response(self.get_merkle_proof_for_output(&id)),
_ => response(StatusCode::BAD_REQUEST, ""), _ => response(StatusCode::BAD_REQUEST, ""),
} }
} }
@ -420,47 +423,48 @@ pub struct PeerHandler {
impl Handler for PeerHandler { impl Handler for PeerHandler {
fn get(&self, req: Request<Body>) -> ResponseFuture { fn get(&self, req: Request<Body>) -> ResponseFuture {
if let Ok(addr) = req let command = match req.uri().path().trim_right_matches("/").rsplit("/").next() {
.uri() Some(c) => c,
.path() None => return response(StatusCode::BAD_REQUEST, "invalid url"),
.trim_right_matches("/") };
.rsplit("/") if let Ok(addr) = command.parse() {
.next()
.unwrap()
.parse()
{
match w(&self.peers).get_peer(addr) { match w(&self.peers).get_peer(addr) {
Ok(peer) => json_response(&peer), Ok(peer) => json_response(&peer),
Err(_) => response(StatusCode::BAD_REQUEST, ""), Err(_) => response(StatusCode::NOT_FOUND, "peer not found"),
} }
} else { } else {
response( response(
StatusCode::BAD_REQUEST, StatusCode::BAD_REQUEST,
format!("url unrecognized: {}", req.uri().path()), format!("peer address unrecognized: {}", req.uri().path()),
) )
} }
} }
fn post(&self, req: Request<Body>) -> ResponseFuture { fn post(&self, req: Request<Body>) -> ResponseFuture {
let mut path_elems = req.uri().path().trim_right_matches("/").rsplit("/"); let mut path_elems = req.uri().path().trim_right_matches("/").rsplit("/");
match path_elems.next().unwrap() { let command = match path_elems.next() {
"ban" => { None => return response(StatusCode::BAD_REQUEST, "invalid url"),
if let Ok(addr) = path_elems.next().unwrap().parse() { Some(c) => c,
w(&self.peers).ban_peer(&addr, ReasonForBan::ManualBan); };
let addr = match path_elems.next() {
None => return response(StatusCode::BAD_REQUEST, "invalid url"),
Some(a) => match a.parse() {
Err(e) => {
return response(
StatusCode::BAD_REQUEST,
format!("invalid peer address: {}", e),
)
}
Ok(addr) => addr,
},
};
match command {
"ban" => w(&self.peers).ban_peer(&addr, ReasonForBan::ManualBan),
"unban" => w(&self.peers).unban_peer(&addr),
_ => return response(StatusCode::BAD_REQUEST, "invalid command"),
};
response(StatusCode::OK, "") response(StatusCode::OK, "")
} else {
response(StatusCode::BAD_REQUEST, "bad address to ban")
}
}
"unban" => {
if let Ok(addr) = path_elems.next().unwrap().parse() {
w(&self.peers).unban_peer(&addr);
response(StatusCode::OK, "")
} else {
response(StatusCode::BAD_REQUEST, "bad address to unban")
}
}
_ => response(StatusCode::BAD_REQUEST, "unrecognized command"),
}
} }
} }
@ -472,14 +476,20 @@ pub struct StatusHandler {
} }
impl StatusHandler { impl StatusHandler {
fn get_status(&self) -> Status { fn get_status(&self) -> Result<Status, Error> {
Status::from_tip_and_peers(w(&self.chain).head().unwrap(), w(&self.peers).peer_count()) let head = w(&self.chain)
.head()
.map_err(|e| ErrorKind::Internal(format!("can't get head: {}", e)))?;
Ok(Status::from_tip_and_peers(
head,
w(&self.peers).peer_count(),
))
} }
} }
impl Handler for StatusHandler { impl Handler for StatusHandler {
fn get(&self, _req: Request<Body>) -> ResponseFuture { fn get(&self, _req: Request<Body>) -> ResponseFuture {
json_response(&self.get_status()) result_to_response(self.get_status())
} }
} }
@ -490,14 +500,17 @@ pub struct ChainHandler {
} }
impl ChainHandler { impl ChainHandler {
fn get_tip(&self) -> Tip { fn get_tip(&self) -> Result<Tip, Error> {
Tip::from_tip(w(&self.chain).head().unwrap()) let head = w(&self.chain)
.head()
.map_err(|e| ErrorKind::Internal(format!("can't get head: {}", e)))?;
Ok(Tip::from_tip(head))
} }
} }
impl Handler for ChainHandler { impl Handler for ChainHandler {
fn get(&self, _req: Request<Body>) -> ResponseFuture { fn get(&self, _req: Request<Body>) -> ResponseFuture {
json_response(&self.get_tip()) result_to_response(self.get_tip())
} }
} }
@ -510,8 +523,13 @@ pub struct ChainValidationHandler {
impl Handler for ChainValidationHandler { impl Handler for ChainValidationHandler {
fn get(&self, _req: Request<Body>) -> ResponseFuture { fn get(&self, _req: Request<Body>) -> ResponseFuture {
// TODO - read skip_rproofs from query params // TODO - read skip_rproofs from query params
w(&self.chain).validate(true).unwrap(); match w(&self.chain).validate(true) {
response(StatusCode::OK, "") Ok(_) => response(StatusCode::OK, ""),
Err(e) => response(
StatusCode::INTERNAL_SERVER_ERROR,
format!("validate failed: {}", e),
),
}
} }
} }
@ -524,8 +542,13 @@ pub struct ChainCompactHandler {
impl Handler for ChainCompactHandler { impl Handler for ChainCompactHandler {
fn get(&self, _req: Request<Body>) -> ResponseFuture { fn get(&self, _req: Request<Body>) -> ResponseFuture {
w(&self.chain).compact().unwrap(); match w(&self.chain).compact() {
response(StatusCode::OK, "") Ok(_) => response(StatusCode::OK, ""),
Err(e) => response(
StatusCode::INTERNAL_SERVER_ERROR,
format!("compact failed: {}", e),
),
}
} }
} }
@ -546,7 +569,8 @@ impl HeaderHandler {
} }
} }
check_block_param(&input)?; check_block_param(&input)?;
let vec = util::from_hex(input).unwrap(); let vec = util::from_hex(input)
.map_err(|e| ErrorKind::Argument(format!("invalid input: {}", e)))?;
let h = Hash::from_vec(&vec); let h = Hash::from_vec(&vec);
let header = w(&self.chain) let header = w(&self.chain)
.get_block_header(&h) .get_block_header(&h)
@ -588,7 +612,8 @@ impl BlockHandler {
} }
} }
check_block_param(&input)?; check_block_param(&input)?;
let vec = util::from_hex(input).unwrap(); let vec = util::from_hex(input)
.map_err(|e| ErrorKind::Argument(format!("invalid input: {}", e)))?;
Ok(Hash::from_vec(&vec)) Ok(Hash::from_vec(&vec))
} }
} }
@ -607,71 +632,43 @@ fn check_block_param(input: &String) -> Result<(), Error> {
impl Handler for BlockHandler { impl Handler for BlockHandler {
fn get(&self, req: Request<Body>) -> ResponseFuture { fn get(&self, req: Request<Body>) -> ResponseFuture {
let el = req let el = match req.uri().path().trim_right_matches("/").rsplit("/").next() {
.uri() None => return response(StatusCode::BAD_REQUEST, "invalid url"),
.path() Some(el) => el,
.trim_right_matches("/") };
.rsplit("/")
.next()
.unwrap();
let h = self.parse_input(el.to_string()); let h = match self.parse_input(el.to_string()) {
if h.is_err() { Err(e) => {
error!(LOGGER, "block_handler: bad parameter {}", el.to_string()); return response(
return response(StatusCode::BAD_REQUEST, ""); StatusCode::BAD_REQUEST,
format!("failed to parse input: {}", e),
)
} }
Ok(h) => h,
let h = h.unwrap(); };
if let Some(param) = req.uri().query() { if let Some(param) = req.uri().query() {
if param == "compact" { if param == "compact" {
match self.get_compact_block(&h) { result_to_response(self.get_compact_block(&h))
Ok(b) => json_response(&b), } else {
Err(_) => { response(
error!(LOGGER, "block_handler: can not get compact block {}", h); StatusCode::BAD_REQUEST,
response(StatusCode::INTERNAL_SERVER_ERROR, "") format!("unsupported query parameter: {}", param),
} )
} }
} else { } else {
debug!( result_to_response(self.get_block(&h))
LOGGER,
"block_handler: unsupported query parameter {}", param
);
response(StatusCode::BAD_REQUEST, "")
}
} else {
match self.get_block(&h) {
Ok(b) => json_response(&b),
Err(_) => {
error!(LOGGER, "block_handler: can not get block {}", h);
response(StatusCode::INTERNAL_SERVER_ERROR, "")
}
}
} }
} }
} }
impl Handler for HeaderHandler { impl Handler for HeaderHandler {
fn get(&self, req: Request<Body>) -> ResponseFuture { fn get(&self, req: Request<Body>) -> ResponseFuture {
let el = req let el = match req.uri().path().trim_right_matches("/").rsplit("/").next() {
.uri() None => return response(StatusCode::BAD_REQUEST, "invalid url"),
.path() Some(el) => el,
.trim_right_matches("/") };
.rsplit("/") result_to_response(self.get_header(el.to_string()))
.next()
.unwrap();
match self.get_header(el.to_string()) {
Ok(h) => json_response(&h),
Err(_) => {
error!(
LOGGER,
"header_handler: can not get header {}",
el.to_string()
);
response(StatusCode::INTERNAL_SERVER_ERROR, "")
}
}
} }
} }
@ -728,12 +725,10 @@ where
.and_then(move |wrapper: TxWrapper| { .and_then(move |wrapper: TxWrapper| {
util::from_hex(wrapper.tx_hex) util::from_hex(wrapper.tx_hex)
.map_err(|_| ErrorKind::RequestError("Bad request".to_owned()).into()) .map_err(|_| ErrorKind::RequestError("Bad request".to_owned()).into())
}) }).and_then(move |tx_bin| {
.and_then(move |tx_bin| {
ser::deserialize(&mut &tx_bin[..]) ser::deserialize(&mut &tx_bin[..])
.map_err(|_| ErrorKind::RequestError("Bad request".to_owned()).into()) .map_err(|_| ErrorKind::RequestError("Bad request".to_owned()).into())
}) }).and_then(move |tx: Transaction| {
.and_then(move |tx: Transaction| {
let source = pool::TxSource { let source = pool::TxSource {
debug_name: "push-api".to_string(), debug_name: "push-api".to_string(),
identifier: "?.?.?.?".to_string(), identifier: "?.?.?.?".to_string(),
@ -763,7 +758,12 @@ where
Box::new( Box::new(
self.update_pool(req) self.update_pool(req)
.and_then(|_| ok(just_response(StatusCode::OK, ""))) .and_then(|_| ok(just_response(StatusCode::OK, "")))
.or_else(|_| ok(just_response(StatusCode::BAD_REQUEST, ""))), .or_else(|e| {
ok(just_response(
StatusCode::INTERNAL_SERVER_ERROR,
format!("failed: {}", e),
))
}),
) )
} }
} }
@ -780,6 +780,24 @@ where
} }
} }
fn result_to_response<T>(res: Result<T, Error>) -> ResponseFuture
where
T: Serialize,
{
match res {
Ok(s) => json_response_pretty(&s),
Err(e) => match e.kind() {
ErrorKind::Argument(msg) => response(StatusCode::BAD_REQUEST, msg.clone()),
ErrorKind::RequestError(msg) => response(StatusCode::BAD_REQUEST, msg.clone()),
ErrorKind::NotFound => response(StatusCode::NOT_FOUND, ""),
ErrorKind::Internal(msg) => response(StatusCode::INTERNAL_SERVER_ERROR, msg.clone()),
ErrorKind::ResponseError(msg) => {
response(StatusCode::INTERNAL_SERVER_ERROR, msg.clone())
}
},
}
}
// pretty-printed version of above // pretty-printed version of above
fn json_response_pretty<T>(s: &T) -> ResponseFuture fn json_response_pretty<T>(s: &T) -> ResponseFuture
where where
@ -787,7 +805,10 @@ where
{ {
match serde_json::to_string_pretty(s) { match serde_json::to_string_pretty(s) {
Ok(json) => response(StatusCode::OK, json), Ok(json) => response(StatusCode::OK, json),
Err(_) => response(StatusCode::INTERNAL_SERVER_ERROR, ""), Err(e) => response(
StatusCode::INTERNAL_SERVER_ERROR,
format!("can't create json response: {}", e),
),
} }
} }
@ -796,7 +817,6 @@ fn response<T: Into<Body> + Debug>(status: StatusCode, text: T) -> ResponseFutur
} }
fn just_response<T: Into<Body> + Debug>(status: StatusCode, text: T) -> Response<Body> { fn just_response<T: Into<Body> + Debug>(status: StatusCode, text: T) -> Response<Body> {
debug!(LOGGER, "HTTP API -> status: {}, text: {:?}", status, text);
let mut resp = Response::new(text.into()); let mut resp = Response::new(text.into());
*resp.status_mut() = status; *resp.status_mut() = status;
resp resp

View file

@ -1132,8 +1132,7 @@ fn check_and_remove_files(txhashset_path: &PathBuf, header: &BlockHeader) -> Res
.file_name() .file_name()
.and_then(|n| n.to_str().map(|s| String::from(s))) .and_then(|n| n.to_str().map(|s| String::from(s)))
}) })
}) }).collect();
.collect();
let dir_difference: Vec<String> = subdirectories_found let dir_difference: Vec<String> = subdirectories_found
.difference(&subdirectories_expected) .difference(&subdirectories_expected)
@ -1162,8 +1161,7 @@ fn check_and_remove_files(txhashset_path: &PathBuf, header: &BlockHeader) -> Res
} else { } else {
String::from(s) String::from(s)
} }
}) }).collect();
.collect();
let subdirectories = fs::read_dir(txhashset_path)?; let subdirectories = fs::read_dir(txhashset_path)?;
for subdirectory in subdirectories { for subdirectory in subdirectories {
@ -1176,8 +1174,7 @@ fn check_and_remove_files(txhashset_path: &PathBuf, header: &BlockHeader) -> Res
.file_name() .file_name()
.and_then(|n| n.to_str().map(|s| String::from(s))) .and_then(|n| n.to_str().map(|s| String::from(s)))
}) })
}) }).collect();
.collect();
let difference: Vec<String> = pmmr_files_found let difference: Vec<String> = pmmr_files_found
.difference(&pmmr_files_expected) .difference(&pmmr_files_expected)
.cloned() .cloned()