Skip to content

Commit

Permalink
Fix crash on invalid part size (#649)
Browse files Browse the repository at this point in the history
* Return CRT client initialization error instead of panicking

Signed-off-by: Alessandro Passaro <[email protected]>

* Simplify handling of initialization errors

Signed-off-by: Alessandro Passaro <[email protected]>

---------

Signed-off-by: Alessandro Passaro <[email protected]>
  • Loading branch information
passaro authored Nov 29, 2023
1 parent 3f25f32 commit 099638e
Showing 1 changed file with 17 additions and 2 deletions.
19 changes: 17 additions & 2 deletions mountpoint-s3-client/src/s3_crt_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ impl S3CrtClientInner {
let user_agent = config.user_agent.unwrap_or_else(|| UserAgent::new(None));
let user_agent_header = user_agent.build();

let s3_client = Client::new(&allocator, client_config).unwrap();
let s3_client = Client::new(&allocator, client_config).map_err(NewClientError::CrtError)?;

Ok(Self {
allocator,
Expand Down Expand Up @@ -769,10 +769,13 @@ pub enum NewClientError {
InvalidEndpoint(#[from] EndpointError),
/// Invalid AWS credentials
#[error("invalid AWS credentials")]
ProviderFailure(#[from] mountpoint_s3_crt::common::error::Error),
ProviderFailure(#[source] mountpoint_s3_crt::common::error::Error),
/// Invalid configuration
#[error("invalid configuration: {0}")]
InvalidConfiguration(String),
/// An internal error from within the AWS Common Runtime
#[error("Unknown CRT error")]
CrtError(#[source] mountpoint_s3_crt::common::error::Error),
}

/// Errors returned by the CRT-based S3 client
Expand Down Expand Up @@ -1031,6 +1034,18 @@ mod tests {
use super::*;
use test_case::test_case;

/// Test both explicit validation in [Client::new] and implicit limits in the CRT
#[test_case(4 * 1024 * 1024; "less than 5MiB")] // validated in Client::new
#[test_case(10_000_000; "not a multiple of 1024")] // CRT constraint
#[test_case(6 * 1024 * 1024 * 1024; "greater than 5GiB")] // validated in Client::new
fn client_new_fails_with_invalid_part_size(part_size: usize) {
let config = S3ClientConfig {
part_size,
..Default::default()
};
S3CrtClient::new(config).expect_err("creating a new client should fail");
}

/// Test if the prefix is added correctly to the User-Agent header
#[test]
fn test_user_agent_with_prefix() {
Expand Down

0 comments on commit 099638e

Please sign in to comment.