Skip to content

Commit

Permalink
Report error if user sends an empty custom command. (#897)
Browse files Browse the repository at this point in the history
* Report error if user sends an empty custom command.

* clippy fixes
  • Loading branch information
nihohit authored Feb 4, 2024
1 parent da08f73 commit 69c77a5
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 13 deletions.
40 changes: 27 additions & 13 deletions glide-core/src/socket_listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,22 @@ async fn write_result(
None
}
}
Err(ClienUsageError::InternalError(error_message)) => {
Err(ClienUsageError::Internal(error_message)) => {
log_error("internal error", &error_message);
Some(response::response::Value::ClosingError(
error_message.into(),
))
}
Err(ClienUsageError::RedisError(err)) => {
Err(ClienUsageError::User(error_message)) => {
log_error("user error", &error_message);
let request_error = response::RequestError {
type_: response::RequestErrorType::Unspecified.into(),
message: error_message.into(),
..Default::default()
};
Some(response::response::Value::RequestError(request_error))
}
Err(ClienUsageError::Redis(err)) => {
let error_message = err.to_string();
log_warn("received error", error_message.as_str());
log_debug("received error", format!("for callback {}", callback_index));
Expand Down Expand Up @@ -342,7 +351,7 @@ fn get_command(request: &Command) -> Option<Cmd> {

fn get_redis_command(command: &Command) -> Result<Cmd, ClienUsageError> {
let Some(mut cmd) = get_command(command) else {
return Err(ClienUsageError::InternalError(format!(
return Err(ClienUsageError::Internal(format!(
"Received invalid request type: {:?}",
command.request_type
)));
Expand All @@ -361,12 +370,18 @@ fn get_redis_command(command: &Command) -> Result<Cmd, ClienUsageError> {
}
}
None => {
return Err(ClienUsageError::InternalError(
return Err(ClienUsageError::Internal(
"Failed to get request arguments, no arguments are set".to_string(),
));
}
};

if cmd.args_iter().next().is_none() {
return Err(ClienUsageError::User(
"Received command without a command name or arguments".into(),
));
}

Ok(cmd)
}

Expand Down Expand Up @@ -416,9 +431,7 @@ fn get_slot_addr(slot_type: &protobuf::EnumOrUnknown<SlotTypes>) -> ClientUsageR
SlotTypes::Primary => SlotAddr::Master,
SlotTypes::Replica => SlotAddr::ReplicaRequired,
})
.map_err(|id| {
ClienUsageError::InternalError(format!("Received unexpected slot id type {id}"))
})
.map_err(|id| ClienUsageError::Internal(format!("Received unexpected slot id type {id}")))
}

fn get_route(
Expand All @@ -438,9 +451,7 @@ fn get_route(
match route {
Value::SimpleRoutes(simple_route) => {
let simple_route = simple_route.enum_value().map_err(|id| {
ClienUsageError::InternalError(format!(
"Received unexpected simple route type {id}"
))
ClienUsageError::Internal(format!("Received unexpected simple route type {id}"))
})?;
match simple_route {
crate::redis_request::SimpleRoutes::AllNodes => Ok(Some(RoutingInfo::MultiNode((
Expand Down Expand Up @@ -507,7 +518,7 @@ fn handle_request(request: RedisRequest, client: Client, writer: Rc<Writer>) {
request.callback_idx
),
);
Err(ClienUsageError::InternalError(
Err(ClienUsageError::Internal(
"Received empty request".to_string(),
))
}
Expand Down Expand Up @@ -791,10 +802,13 @@ enum ClientCreationError {
#[derive(Debug, Error)]
enum ClienUsageError {
#[error("Redis error: {0}")]
RedisError(#[from] RedisError),
Redis(#[from] RedisError),
/// An error that stems from wrong behavior of the client.
#[error("Internal error: {0}")]
InternalError(String),
Internal(String),
/// An error that stems from wrong behavior of the user.
#[error("User error: {0}")]
User(String),
}

type ClientUsageResult<T> = Result<T, ClienUsageError>;
Expand Down
24 changes: 24 additions & 0 deletions glide-core/tests/test_socket_listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1023,4 +1023,28 @@ mod socket_listener {
ResponseType::Value,
);
}

#[rstest]
#[timeout(SHORT_CLUSTER_TEST_TIMEOUT)]
fn test_send_empty_custom_command_is_an_error(#[values(true, false)] use_cluster: bool) {
let mut test_basics = setup_test_basics(false, true, use_cluster);
const CALLBACK_INDEX: u32 = 42;

let mut buffer = Vec::with_capacity(APPROX_RESP_HEADER_LEN);
write_command_request(
&mut buffer,
CALLBACK_INDEX,
vec![],
RequestType::CustomCommand.into(),
false,
);
test_basics.socket.write_all(&buffer).unwrap();

let _size = read_from_socket(&mut buffer, &mut test_basics.socket);
assert_error_response(
buffer.as_slice(),
CALLBACK_INDEX,
ResponseType::RequestError,
);
}
}

0 comments on commit 69c77a5

Please sign in to comment.