From d3ce182ddc80417fa091d3d8818c6a098cd00f15 Mon Sep 17 00:00:00 2001 From: Luke Anderson Date: Fri, 13 Sep 2019 20:52:12 +1000 Subject: [PATCH] Renamed 'InvalidQueryParams' to 'BadRequest', since it is a more general error that is returned in a number of cases. --- beacon_node/rest_api/src/beacon.rs | 7 ++--- beacon_node/rest_api/src/error.rs | 4 +-- beacon_node/rest_api/src/helpers.rs | 16 +++++----- beacon_node/rest_api/src/response_builder.rs | 7 +++-- beacon_node/rest_api/src/url_query.rs | 8 ++--- beacon_node/rest_api/src/validator.rs | 33 +++++++++----------- 6 files changed, 34 insertions(+), 41 deletions(-) diff --git a/beacon_node/rest_api/src/beacon.rs b/beacon_node/rest_api/src/beacon.rs index 13f52dc9a..c1a9da6ee 100644 --- a/beacon_node/rest_api/src/beacon.rs +++ b/beacon_node/rest_api/src/beacon.rs @@ -139,10 +139,7 @@ pub fn get_validators(req: Request) -> ApiR .parse::() .map(Epoch::from) .map_err(|e| { - ApiError::InvalidQueryParams(format!( - "Invalid epoch parameter, must be a u64. {:?}", - e - )) + ApiError::BadRequest(format!("Invalid epoch parameter, must be a u64. {:?}", e)) })?, // In this case, our url query did not contain any parameters, so we take the default Err(_) => beacon_chain.epoch().map_err(|e| { @@ -181,7 +178,7 @@ pub fn get_state(req: Request) -> ApiResult let query_params = ["root", "slot"]; query.first_of(&query_params)? } - Err(ApiError::InvalidQueryParams(_)) => { + Err(ApiError::BadRequest(_)) => { // No parameters provided at all, use current slot. (String::from("slot"), head_state.slot.to_string()) } diff --git a/beacon_node/rest_api/src/error.rs b/beacon_node/rest_api/src/error.rs index e52ba4af6..70384dce9 100644 --- a/beacon_node/rest_api/src/error.rs +++ b/beacon_node/rest_api/src/error.rs @@ -7,7 +7,7 @@ pub enum ApiError { MethodNotAllowed(String), ServerError(String), NotImplemented(String), - InvalidQueryParams(String), + BadRequest(String), NotFound(String), UnsupportedType(String), ImATeapot(String), // Just in case. @@ -21,7 +21,7 @@ impl ApiError { ApiError::MethodNotAllowed(desc) => (StatusCode::METHOD_NOT_ALLOWED, desc), ApiError::ServerError(desc) => (StatusCode::INTERNAL_SERVER_ERROR, 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::UnsupportedType(desc) => (StatusCode::UNSUPPORTED_MEDIA_TYPE, desc), ApiError::ImATeapot(desc) => (StatusCode::IM_A_TEAPOT, desc), diff --git a/beacon_node/rest_api/src/helpers.rs b/beacon_node/rest_api/src/helpers.rs index 3385e2017..3f76f4e25 100644 --- a/beacon_node/rest_api/src/helpers.rs +++ b/beacon_node/rest_api/src/helpers.rs @@ -21,7 +21,7 @@ pub fn parse_slot(string: &str) -> Result { string .parse::() .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. @@ -31,7 +31,7 @@ pub fn parse_slot(string: &str) -> Result { pub fn check_content_type_for_json(req: &Request) -> Result<(), ApiError> { match req.headers().get(header::CONTENT_TYPE) { 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.", h ))), @@ -49,9 +49,9 @@ pub fn parse_root(string: &str) -> Result { let trimmed = string.trim_start_matches(PREFIX); trimmed .parse() - .map_err(|e| ApiError::InvalidQueryParams(format!("Unable to parse root: {:?}", e))) + .map_err(|e| ApiError::BadRequest(format!("Unable to parse root: {:?}", e))) } else { - Err(ApiError::InvalidQueryParams( + Err(ApiError::BadRequest( "Root must have a '0x' prefix".to_string(), )) } @@ -62,13 +62,13 @@ pub fn parse_pubkey(string: &str) -> Result { const PREFIX: &str = "0x"; if string.starts_with(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| { - ApiError::InvalidQueryParams(format!("Unable to deserialize public key: {:?}.", e)) + ApiError::BadRequest(format!("Unable to deserialize public key: {:?}.", e)) })?; return Ok(pubkey); } else { - return Err(ApiError::InvalidQueryParams( + return Err(ApiError::BadRequest( "Public key must have a '0x' prefix".to_string(), )); } @@ -145,7 +145,7 @@ pub fn state_root_at_slot( // // We could actually speculate about future state roots by skipping slots, however that's // likely to cause confusion for API users. - Err(ApiError::InvalidQueryParams(format!( + Err(ApiError::BadRequest(format!( "Requested slot {} is past the current slot {}", slot, current_slot ))) diff --git a/beacon_node/rest_api/src/response_builder.rs b/beacon_node/rest_api/src/response_builder.rs index 793360cd2..d5b530f8a 100644 --- a/beacon_node/rest_api/src/response_builder.rs +++ b/beacon_node/rest_api/src/response_builder.rs @@ -17,22 +17,23 @@ pub struct ResponseBuilder { impl ResponseBuilder { pub fn new(req: &Request) -> Result { - let content_header = req + let content_header: String = req .headers() .get(header::CONTENT_TYPE) .map_or(Ok(""), |h| h.to_str()) .map_err(|e| { - ApiError::InvalidQueryParams(format!( + ApiError::BadRequest(format!( "The content-type header contains invalid characters: {:?}", e )) }) .map(|h| String::from(h))?; + // JSON is our default encoding, unless something else is requested. let encoding = match content_header { 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("text/plain") => Encoding::TEXT, + ref h if h.starts_with("text/") => Encoding::TEXT, _ => Encoding::JSON, }; Ok(Self { encoding }) diff --git a/beacon_node/rest_api/src/url_query.rs b/beacon_node/rest_api/src/url_query.rs index 3802ff831..f0c587a32 100644 --- a/beacon_node/rest_api/src/url_query.rs +++ b/beacon_node/rest_api/src/url_query.rs @@ -12,7 +12,7 @@ impl<'a> UrlQuery<'a> { /// Returns `Err` if `req` does not contain any query parameters. pub fn from_request(req: &'a Request) -> Result { 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(), ) })?; @@ -28,7 +28,7 @@ impl<'a> UrlQuery<'a> { .find(|(key, _value)| keys.contains(&&**key)) .map(|(key, value)| (key.into_owned(), value.into_owned())) .ok_or_else(|| { - ApiError::InvalidQueryParams(format!( + ApiError::BadRequest(format!( "URL query must contain at least one of the following keys: {:?}", keys )) @@ -48,13 +48,13 @@ impl<'a> UrlQuery<'a> { if first_key == key { Ok(first_value.to_string()) } else { - Err(ApiError::InvalidQueryParams(format!( + Err(ApiError::BadRequest(format!( "Only the {} query parameter is supported", key ))) } } else { - Err(ApiError::InvalidQueryParams(format!( + Err(ApiError::BadRequest(format!( "Only one query parameter is allowed, {} supplied", queries.len() ))) diff --git a/beacon_node/rest_api/src/validator.rs b/beacon_node/rest_api/src/validator.rs index b79466b4d..53d9e8b8b 100644 --- a/beacon_node/rest_api/src/validator.rs +++ b/beacon_node/rest_api/src/validator.rs @@ -58,10 +58,7 @@ pub fn get_validator_duties(req: Request) - slog::trace!(log, "Requested epoch {:?}", v); Epoch::new(v.parse::().map_err(|e| { slog::info!(log, "Invalid epoch {:?}", e); - ApiError::InvalidQueryParams(format!( - "Invalid epoch parameter, must be a u64. {:?}", - e - )) + ApiError::BadRequest(format!("Invalid epoch parameter, must be a u64. {:?}", e)) })?) } Err(_) => { @@ -72,7 +69,7 @@ pub fn get_validator_duties(req: Request) - }; let relative_epoch = RelativeEpoch::from_epoch(current_epoch, epoch).map_err(|e| { slog::info!(log, "Requested epoch out of range."); - ApiError::InvalidQueryParams(format!( + ApiError::BadRequest(format!( "Cannot get RelativeEpoch, epoch out of range: {:?}", e )) @@ -166,17 +163,17 @@ pub fn get_new_beacon_block(req: Request) - .parse::() .map(Slot::from) .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 .first_of(&["randao_reveal"]) .map(|(_key, value)| value) .map(hex::decode)? .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| { - 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 @@ -216,7 +213,7 @@ pub fn publish_beacon_block(req: Request) - .map(|chunk| chunk.iter().cloned().collect::>()) .and_then(|chunks| { serde_json::from_slice(&chunks.as_slice()).map_err(|e| { - ApiError::InvalidQueryParams(format!( + ApiError::BadRequest(format!( "Unable to deserialize JSON into a BeaconBlock: {:?}", e )) @@ -233,7 +230,7 @@ pub fn publish_beacon_block(req: Request) - Ok(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 - Err(ApiError::InvalidQueryParams(format!( + Err(ApiError::BadRequest(format!( "The BeaconBlock could not be processed: {:?}", outcome ))) @@ -271,7 +268,7 @@ pub fn get_new_attestation(req: Request) -> .map_err(|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(), ))?; @@ -289,14 +286,14 @@ pub fn get_new_attestation(req: Request) -> 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. 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) ))?; 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 @@ -305,7 +302,7 @@ pub fn get_new_attestation(req: Request) -> .map(|(_key, value)| value)? .parse::() .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) @@ -327,20 +324,18 @@ pub fn get_new_attestation(req: Request) -> .parse::() .map(Slot::from) .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(); 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 .first_of(&["shard"]) .map(|(_key, value)| value)? .parse::() - .map_err(|e| { - ApiError::InvalidQueryParams(format!("Shard is not a valid u64 value: {:?}", e)) - })?; + .map_err(|e| ApiError::BadRequest(format!("Shard is not a valid u64 value: {:?}", e)))?; let attestation_data = beacon_chain .produce_attestation_data(shard, current_slot.into())