Renamed 'InvalidQueryParams' to 'BadRequest', since it is a more general error that is returned in a number of cases.

This commit is contained in:
Luke Anderson 2019-09-13 20:52:12 +10:00
parent f48311900e
commit d3ce182ddc
No known key found for this signature in database
GPG Key ID: 44408169EC61E228
6 changed files with 34 additions and 41 deletions

View File

@ -139,10 +139,7 @@ pub fn get_validators<T: BeaconChainTypes + 'static>(req: Request<Body>) -> ApiR
.parse::<u64>() .parse::<u64>()
.map(Epoch::from) .map(Epoch::from)
.map_err(|e| { .map_err(|e| {
ApiError::InvalidQueryParams(format!( ApiError::BadRequest(format!("Invalid epoch parameter, must be a u64. {:?}", e))
"Invalid epoch parameter, must be a u64. {:?}",
e
))
})?, })?,
// In this case, our url query did not contain any parameters, so we take the default // In this case, our url query did not contain any parameters, so we take the default
Err(_) => beacon_chain.epoch().map_err(|e| { Err(_) => beacon_chain.epoch().map_err(|e| {
@ -181,7 +178,7 @@ pub fn get_state<T: BeaconChainTypes + 'static>(req: Request<Body>) -> ApiResult
let query_params = ["root", "slot"]; let query_params = ["root", "slot"];
query.first_of(&query_params)? query.first_of(&query_params)?
} }
Err(ApiError::InvalidQueryParams(_)) => { Err(ApiError::BadRequest(_)) => {
// No parameters provided at all, use current slot. // No parameters provided at all, use current slot.
(String::from("slot"), head_state.slot.to_string()) (String::from("slot"), head_state.slot.to_string())
} }

View File

@ -7,7 +7,7 @@ pub enum ApiError {
MethodNotAllowed(String), MethodNotAllowed(String),
ServerError(String), ServerError(String),
NotImplemented(String), NotImplemented(String),
InvalidQueryParams(String), BadRequest(String),
NotFound(String), NotFound(String),
UnsupportedType(String), UnsupportedType(String),
ImATeapot(String), // Just in case. ImATeapot(String), // Just in case.
@ -21,7 +21,7 @@ impl ApiError {
ApiError::MethodNotAllowed(desc) => (StatusCode::METHOD_NOT_ALLOWED, desc), ApiError::MethodNotAllowed(desc) => (StatusCode::METHOD_NOT_ALLOWED, desc),
ApiError::ServerError(desc) => (StatusCode::INTERNAL_SERVER_ERROR, desc), ApiError::ServerError(desc) => (StatusCode::INTERNAL_SERVER_ERROR, desc),
ApiError::NotImplemented(desc) => (StatusCode::NOT_IMPLEMENTED, desc), ApiError::NotImplemented(desc) => (StatusCode::NOT_IMPLEMENTED, desc),
ApiError::InvalidQueryParams(desc) => (StatusCode::BAD_REQUEST, desc), ApiError::BadRequest(desc) => (StatusCode::BAD_REQUEST, desc),
ApiError::NotFound(desc) => (StatusCode::NOT_FOUND, desc), ApiError::NotFound(desc) => (StatusCode::NOT_FOUND, desc),
ApiError::UnsupportedType(desc) => (StatusCode::UNSUPPORTED_MEDIA_TYPE, desc), ApiError::UnsupportedType(desc) => (StatusCode::UNSUPPORTED_MEDIA_TYPE, desc),
ApiError::ImATeapot(desc) => (StatusCode::IM_A_TEAPOT, desc), ApiError::ImATeapot(desc) => (StatusCode::IM_A_TEAPOT, desc),

View File

@ -21,7 +21,7 @@ pub fn parse_slot(string: &str) -> Result<Slot, ApiError> {
string string
.parse::<u64>() .parse::<u64>()
.map(Slot::from) .map(Slot::from)
.map_err(|e| ApiError::InvalidQueryParams(format!("Unable to parse slot: {:?}", e))) .map_err(|e| ApiError::BadRequest(format!("Unable to parse slot: {:?}", e)))
} }
/// Checks the provided request to ensure that the `content-type` header. /// Checks the provided request to ensure that the `content-type` header.
@ -31,7 +31,7 @@ pub fn parse_slot(string: &str) -> Result<Slot, ApiError> {
pub fn check_content_type_for_json(req: &Request<Body>) -> Result<(), ApiError> { pub fn check_content_type_for_json(req: &Request<Body>) -> Result<(), ApiError> {
match req.headers().get(header::CONTENT_TYPE) { match req.headers().get(header::CONTENT_TYPE) {
Some(h) if h == "application/json" => Ok(()), Some(h) if h == "application/json" => Ok(()),
Some(h) => Err(ApiError::InvalidQueryParams(format!( Some(h) => Err(ApiError::BadRequest(format!(
"The provided content-type {:?} is not available, this endpoint only supports json.", "The provided content-type {:?} is not available, this endpoint only supports json.",
h h
))), ))),
@ -49,9 +49,9 @@ pub fn parse_root(string: &str) -> Result<Hash256, ApiError> {
let trimmed = string.trim_start_matches(PREFIX); let trimmed = string.trim_start_matches(PREFIX);
trimmed trimmed
.parse() .parse()
.map_err(|e| ApiError::InvalidQueryParams(format!("Unable to parse root: {:?}", e))) .map_err(|e| ApiError::BadRequest(format!("Unable to parse root: {:?}", e)))
} else { } else {
Err(ApiError::InvalidQueryParams( Err(ApiError::BadRequest(
"Root must have a '0x' prefix".to_string(), "Root must have a '0x' prefix".to_string(),
)) ))
} }
@ -62,13 +62,13 @@ pub fn parse_pubkey(string: &str) -> Result<PublicKey, ApiError> {
const PREFIX: &str = "0x"; const PREFIX: &str = "0x";
if string.starts_with(PREFIX) { if string.starts_with(PREFIX) {
let pubkey_bytes = hex::decode(string.trim_start_matches(PREFIX)) let pubkey_bytes = hex::decode(string.trim_start_matches(PREFIX))
.map_err(|e| ApiError::InvalidQueryParams(format!("Invalid hex string: {:?}", e)))?; .map_err(|e| ApiError::BadRequest(format!("Invalid hex string: {:?}", e)))?;
let pubkey = PublicKey::from_bytes(pubkey_bytes.as_slice()).map_err(|e| { let pubkey = PublicKey::from_bytes(pubkey_bytes.as_slice()).map_err(|e| {
ApiError::InvalidQueryParams(format!("Unable to deserialize public key: {:?}.", e)) ApiError::BadRequest(format!("Unable to deserialize public key: {:?}.", e))
})?; })?;
return Ok(pubkey); return Ok(pubkey);
} else { } else {
return Err(ApiError::InvalidQueryParams( return Err(ApiError::BadRequest(
"Public key must have a '0x' prefix".to_string(), "Public key must have a '0x' prefix".to_string(),
)); ));
} }
@ -145,7 +145,7 @@ pub fn state_root_at_slot<T: BeaconChainTypes>(
// //
// We could actually speculate about future state roots by skipping slots, however that's // We could actually speculate about future state roots by skipping slots, however that's
// likely to cause confusion for API users. // likely to cause confusion for API users.
Err(ApiError::InvalidQueryParams(format!( Err(ApiError::BadRequest(format!(
"Requested slot {} is past the current slot {}", "Requested slot {} is past the current slot {}",
slot, current_slot slot, current_slot
))) )))

View File

@ -17,22 +17,23 @@ pub struct ResponseBuilder {
impl ResponseBuilder { impl ResponseBuilder {
pub fn new(req: &Request<Body>) -> Result<Self, ApiError> { pub fn new(req: &Request<Body>) -> Result<Self, ApiError> {
let content_header = req let content_header: String = req
.headers() .headers()
.get(header::CONTENT_TYPE) .get(header::CONTENT_TYPE)
.map_or(Ok(""), |h| h.to_str()) .map_or(Ok(""), |h| h.to_str())
.map_err(|e| { .map_err(|e| {
ApiError::InvalidQueryParams(format!( ApiError::BadRequest(format!(
"The content-type header contains invalid characters: {:?}", "The content-type header contains invalid characters: {:?}",
e e
)) ))
}) })
.map(|h| String::from(h))?; .map(|h| String::from(h))?;
// JSON is our default encoding, unless something else is requested.
let encoding = match content_header { let encoding = match content_header {
ref h if h.starts_with("application/ssz") => Encoding::SSZ, ref h if h.starts_with("application/ssz") => Encoding::SSZ,
ref h if h.starts_with("application/yaml") => Encoding::YAML, ref h if h.starts_with("application/yaml") => Encoding::YAML,
ref h if h.starts_with("text/plain") => Encoding::TEXT, ref h if h.starts_with("text/") => Encoding::TEXT,
_ => Encoding::JSON, _ => Encoding::JSON,
}; };
Ok(Self { encoding }) Ok(Self { encoding })

View File

@ -12,7 +12,7 @@ impl<'a> UrlQuery<'a> {
/// Returns `Err` if `req` does not contain any query parameters. /// Returns `Err` if `req` does not contain any query parameters.
pub fn from_request<T>(req: &'a Request<T>) -> Result<Self, ApiError> { pub fn from_request<T>(req: &'a Request<T>) -> Result<Self, ApiError> {
let query_str = req.uri().query().ok_or_else(|| { let query_str = req.uri().query().ok_or_else(|| {
ApiError::InvalidQueryParams( ApiError::BadRequest(
"URL query must be valid and contain at least one key.".to_string(), "URL query must be valid and contain at least one key.".to_string(),
) )
})?; })?;
@ -28,7 +28,7 @@ impl<'a> UrlQuery<'a> {
.find(|(key, _value)| keys.contains(&&**key)) .find(|(key, _value)| keys.contains(&&**key))
.map(|(key, value)| (key.into_owned(), value.into_owned())) .map(|(key, value)| (key.into_owned(), value.into_owned()))
.ok_or_else(|| { .ok_or_else(|| {
ApiError::InvalidQueryParams(format!( ApiError::BadRequest(format!(
"URL query must contain at least one of the following keys: {:?}", "URL query must contain at least one of the following keys: {:?}",
keys keys
)) ))
@ -48,13 +48,13 @@ impl<'a> UrlQuery<'a> {
if first_key == key { if first_key == key {
Ok(first_value.to_string()) Ok(first_value.to_string())
} else { } else {
Err(ApiError::InvalidQueryParams(format!( Err(ApiError::BadRequest(format!(
"Only the {} query parameter is supported", "Only the {} query parameter is supported",
key key
))) )))
} }
} else { } else {
Err(ApiError::InvalidQueryParams(format!( Err(ApiError::BadRequest(format!(
"Only one query parameter is allowed, {} supplied", "Only one query parameter is allowed, {} supplied",
queries.len() queries.len()
))) )))

View File

@ -58,10 +58,7 @@ pub fn get_validator_duties<T: BeaconChainTypes + 'static>(req: Request<Body>) -
slog::trace!(log, "Requested epoch {:?}", v); slog::trace!(log, "Requested epoch {:?}", v);
Epoch::new(v.parse::<u64>().map_err(|e| { Epoch::new(v.parse::<u64>().map_err(|e| {
slog::info!(log, "Invalid epoch {:?}", e); slog::info!(log, "Invalid epoch {:?}", e);
ApiError::InvalidQueryParams(format!( ApiError::BadRequest(format!("Invalid epoch parameter, must be a u64. {:?}", e))
"Invalid epoch parameter, must be a u64. {:?}",
e
))
})?) })?)
} }
Err(_) => { Err(_) => {
@ -72,7 +69,7 @@ pub fn get_validator_duties<T: BeaconChainTypes + 'static>(req: Request<Body>) -
}; };
let relative_epoch = RelativeEpoch::from_epoch(current_epoch, epoch).map_err(|e| { let relative_epoch = RelativeEpoch::from_epoch(current_epoch, epoch).map_err(|e| {
slog::info!(log, "Requested epoch out of range."); slog::info!(log, "Requested epoch out of range.");
ApiError::InvalidQueryParams(format!( ApiError::BadRequest(format!(
"Cannot get RelativeEpoch, epoch out of range: {:?}", "Cannot get RelativeEpoch, epoch out of range: {:?}",
e e
)) ))
@ -166,17 +163,17 @@ pub fn get_new_beacon_block<T: BeaconChainTypes + 'static>(req: Request<Body>) -
.parse::<u64>() .parse::<u64>()
.map(Slot::from) .map(Slot::from)
.map_err(|e| { .map_err(|e| {
ApiError::InvalidQueryParams(format!("Invalid slot parameter, must be a u64. {:?}", e)) ApiError::BadRequest(format!("Invalid slot parameter, must be a u64. {:?}", e))
})?; })?;
let randao_bytes = query let randao_bytes = query
.first_of(&["randao_reveal"]) .first_of(&["randao_reveal"])
.map(|(_key, value)| value) .map(|(_key, value)| value)
.map(hex::decode)? .map(hex::decode)?
.map_err(|e| { .map_err(|e| {
ApiError::InvalidQueryParams(format!("Invalid hex string for randao_reveal: {:?}", e)) ApiError::BadRequest(format!("Invalid hex string for randao_reveal: {:?}", e))
})?; })?;
let randao_reveal = Signature::from_bytes(randao_bytes.as_slice()).map_err(|e| { let randao_reveal = Signature::from_bytes(randao_bytes.as_slice()).map_err(|e| {
ApiError::InvalidQueryParams(format!("randao_reveal is not a valid signature: {:?}", e)) ApiError::BadRequest(format!("randao_reveal is not a valid signature: {:?}", e))
})?; })?;
let (new_block, _state) = beacon_chain let (new_block, _state) = beacon_chain
@ -216,7 +213,7 @@ pub fn publish_beacon_block<T: BeaconChainTypes + 'static>(req: Request<Body>) -
.map(|chunk| chunk.iter().cloned().collect::<Vec<u8>>()) .map(|chunk| chunk.iter().cloned().collect::<Vec<u8>>())
.and_then(|chunks| { .and_then(|chunks| {
serde_json::from_slice(&chunks.as_slice()).map_err(|e| { serde_json::from_slice(&chunks.as_slice()).map_err(|e| {
ApiError::InvalidQueryParams(format!( ApiError::BadRequest(format!(
"Unable to deserialize JSON into a BeaconBlock: {:?}", "Unable to deserialize JSON into a BeaconBlock: {:?}",
e e
)) ))
@ -233,7 +230,7 @@ pub fn publish_beacon_block<T: BeaconChainTypes + 'static>(req: Request<Body>) -
Ok(outcome) => { Ok(outcome) => {
warn!(log, "Block could not be processed, but is being sent to the network anyway."; "block_slot" => slot, "outcome" => format!("{:?}", outcome)); warn!(log, "Block could not be processed, but is being sent to the network anyway."; "block_slot" => slot, "outcome" => format!("{:?}", outcome));
//TODO need to send to network and return http 202 //TODO need to send to network and return http 202
Err(ApiError::InvalidQueryParams(format!( Err(ApiError::BadRequest(format!(
"The BeaconBlock could not be processed: {:?}", "The BeaconBlock could not be processed: {:?}",
outcome outcome
))) )))
@ -271,7 +268,7 @@ pub fn get_new_attestation<T: BeaconChainTypes + 'static>(req: Request<Body>) ->
.map_err(|e| { .map_err(|e| {
ApiError::ServerError(format!("Unable to read validator index cache. {:?}", e)) ApiError::ServerError(format!("Unable to read validator index cache. {:?}", e))
})? })?
.ok_or(ApiError::InvalidQueryParams( .ok_or(ApiError::BadRequest(
"The provided validator public key does not correspond to a validator index.".into(), "The provided validator public key does not correspond to a validator index.".into(),
))?; ))?;
@ -289,14 +286,14 @@ pub fn get_new_attestation<T: BeaconChainTypes + 'static>(req: Request<Body>) ->
e e
)) ))
})? })?
.ok_or(ApiError::InvalidQueryParams("No validator duties could be found for the requested validator. Cannot provide valid attestation.".into()))?; .ok_or(ApiError::BadRequest("No validator duties could be found for the requested validator. Cannot provide valid attestation.".into()))?;
// Check that we are requesting an attestation during the slot where it is relevant. // Check that we are requesting an attestation during the slot where it is relevant.
let present_slot = beacon_chain.slot().map_err(|e| ApiError::ServerError( let present_slot = beacon_chain.slot().map_err(|e| ApiError::ServerError(
format!("Beacon node is unable to determine present slot, either the state isn't generated or the chain hasn't begun. {:?}", e) format!("Beacon node is unable to determine present slot, either the state isn't generated or the chain hasn't begun. {:?}", e)
))?; ))?;
if val_duty.slot != present_slot { if val_duty.slot != present_slot {
return Err(ApiError::InvalidQueryParams(format!("Validator is only able to request an attestation during the slot they are allocated. Current slot: {:?}, allocated slot: {:?}", head_state.slot, val_duty.slot))); return Err(ApiError::BadRequest(format!("Validator is only able to request an attestation during the slot they are allocated. Current slot: {:?}, allocated slot: {:?}", head_state.slot, val_duty.slot)));
} }
// Parse the POC bit and insert it into the aggregation bits // Parse the POC bit and insert it into the aggregation bits
@ -305,7 +302,7 @@ pub fn get_new_attestation<T: BeaconChainTypes + 'static>(req: Request<Body>) ->
.map(|(_key, value)| value)? .map(|(_key, value)| value)?
.parse::<bool>() .parse::<bool>()
.map_err(|e| { .map_err(|e| {
ApiError::InvalidQueryParams(format!("Invalid slot parameter, must be a u64. {:?}", e)) ApiError::BadRequest(format!("Invalid slot parameter, must be a u64. {:?}", e))
})?; })?;
let mut aggregation_bits = BitList::with_capacity(val_duty.committee_len) let mut aggregation_bits = BitList::with_capacity(val_duty.committee_len)
@ -327,20 +324,18 @@ pub fn get_new_attestation<T: BeaconChainTypes + 'static>(req: Request<Body>) ->
.parse::<u64>() .parse::<u64>()
.map(Slot::from) .map(Slot::from)
.map_err(|e| { .map_err(|e| {
ApiError::InvalidQueryParams(format!("Invalid slot parameter, must be a u64. {:?}", e)) ApiError::BadRequest(format!("Invalid slot parameter, must be a u64. {:?}", e))
})?; })?;
let current_slot = beacon_chain.head().beacon_state.slot.as_u64(); let current_slot = beacon_chain.head().beacon_state.slot.as_u64();
if requested_slot != current_slot { if requested_slot != current_slot {
return Err(ApiError::InvalidQueryParams(format!("Attestation data can only be requested for the current slot ({:?}), not your requested slot ({:?})", current_slot, requested_slot))); return Err(ApiError::BadRequest(format!("Attestation data can only be requested for the current slot ({:?}), not your requested slot ({:?})", current_slot, requested_slot)));
} }
let shard = query let shard = query
.first_of(&["shard"]) .first_of(&["shard"])
.map(|(_key, value)| value)? .map(|(_key, value)| value)?
.parse::<u64>() .parse::<u64>()
.map_err(|e| { .map_err(|e| ApiError::BadRequest(format!("Shard is not a valid u64 value: {:?}", e)))?;
ApiError::InvalidQueryParams(format!("Shard is not a valid u64 value: {:?}", e))
})?;
let attestation_data = beacon_chain let attestation_data = beacon_chain
.produce_attestation_data(shard, current_slot.into()) .produce_attestation_data(shard, current_slot.into())