Is there a neater way of handling the option response from req.cookie()
in this code block (line 3 onwards)? Id like to avoid the nested if statements, and multiple returns if at all possible. req.cookie()
returns an Option<Cookie>
and the Cookie
struct has a value function. If the cookie is present but has the wrong value, i want to return a bad request, however i also want to return a bad request if the cookie is not present at all. If the cookie value is OK, i want to continue and do further work:
#[get("/callback")]
async fn callback(req: HttpRequest, client: web::Data<BasicClient>, web::Query(cb): web::Query<Callback>) -> Result<HttpResponse> {
if let Some(state_cookie) = req.cookie(STATE_COOKIE_NAME) {
if state_cookie.value() != cb.state {
return Ok(HttpResponse::BadRequest().finish())
}
} else {
return Ok(HttpResponse::BadRequest().finish())
}
let token_result = client.exchange_code(AuthorizationCode::new(cb.code)).request(http_client);
return match token_result {
Ok(token_details) => Ok(handle_success(token_details)),
Err(token_error) => Ok(handle_error(token_error))
}
}
1 Answer 1
There is a way without multiple returns and nested if
statements.
You have three control flow pathways in your program, related to three states of state_cookie
respectively:
- cookie is
Some
andstate_cookie.value() == cb.state
- cookie is
Some
andstate_cookie.value() != cb.state
- cookie is
None
For the first, we proceed, whereas for second and third we branch to do the same return. Do the 2. and 3. have anything in common? They cover all except one value.
You may be coming from a language where data types lack expressiveness. Many languages don't have Option
at all. In Rust, we have deep operations on Algebraic Data Types like Option
.
To postpone comparison and branching until we have all information, we do a map
on the Option
: req.cookie(STATE_COOKIE_NAME).map(|sc| sc.value())
. Then, a simple comparison divides our function into two control flow paths. As an extra, for self-documenting code, we make a short-lived binding for state_cookie_value
:
#[get("/callback")]
async fn callback(req: HttpRequest, client: web::Data<BasicClient>, web::Query(cb): web::Query<Callback>) -> Result<HttpResponse> {
let state_cookie_value = req.cookie(STATE_COOKIE_NAME).map(|sc| sc.value());
if state_cookie_value != Some(cb.state) {
return Ok(HttpResponse::BadRequest().finish());
}
let token_result = client.exchange_code(AuthorizationCode::new(cb.code)).request(http_client);
match token_result {
Ok(token_details) => Ok(handle_success(token_details)),
Err(token_error) => Ok(handle_error(token_error))
}
}
This is basically all in this code.
I believe this may obscure the code's meaning for the reader, but you can as well write the match
statement as:
Ok(token_result.map_or_else(handle_error, handle_success))