Skip to content

Commit

Permalink
Accept only true and false for env disabling S3 Express
Browse files Browse the repository at this point in the history
This commit addresses #3433 (comment)
  • Loading branch information
ysaito1001 committed Feb 23, 2024
1 parent f1cc4e8 commit 3cdd811
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 7 deletions.
1 change: 1 addition & 0 deletions aws/rust-runtime/aws-inlineable/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ tracing = "0.1"
aws-smithy-async = { path = "../../../rust-runtime/aws-smithy-async", features = ["test-util"] }
aws-smithy-http = { path = "../../../rust-runtime/aws-smithy-http", features = ["rt-tokio"] }
aws-smithy-runtime-api = { path = "../../../rust-runtime/aws-smithy-runtime-api", features = ["test-util"] }
proptest = "1"
tempfile = "3.6.0"
tokio = { version = "1.23.1", features = ["macros", "rt", "io-util"] }

Expand Down
35 changes: 28 additions & 7 deletions aws/rust-runtime/aws-inlineable/src/s3_express.rs
Original file line number Diff line number Diff line change
Expand Up @@ -690,18 +690,18 @@ pub(crate) mod runtime_plugin {
.is_none()
{
match env.get(env::S3_DISABLE_EXPRESS_SESSION_AUTH) {
Ok(value)
if value.eq_ignore_ascii_case("true")
|| value.eq_ignore_ascii_case("false") =>
{
Ok(value) if valid_environment_variable_value(&value) => {
let value = value
.to_lowercase()
.parse::<bool>()
.expect("just checked to be a bool-valued string");
.expect("just checked to be a valid bool-valued string");
layer.store_or_unset(Some(crate::config::DisableS3ExpressSessionAuth(
value,
)));
}
Ok(value) => panic!(
"Must provide either `true` or `false` for an environment variable `{}`, but got {}",
env::S3_DISABLE_EXPRESS_SESSION_AUTH,
value),
_ => {
// TODO(aws-sdk-rust#1073): Transfer a value of
// `s3_disable_express_session_auth` from a profile file to `layer`
Expand All @@ -715,6 +715,10 @@ pub(crate) mod runtime_plugin {
}
}

fn valid_environment_variable_value(value: &str) -> bool {
value == "true" || value == "false"
}

impl RuntimePlugin for S3ExpressRuntimePlugin {
fn config(&self) -> Option<FrozenLayer> {
Some(self.config.clone())
Expand All @@ -724,6 +728,8 @@ pub(crate) mod runtime_plugin {
#[cfg(test)]
mod tests {
use super::*;
use proptest::prelude::*;
use proptest::proptest;

#[test]
fn disable_option_set_from_service_client_should_take_the_highest_precedence() {
Expand Down Expand Up @@ -764,8 +770,8 @@ pub(crate) mod runtime_plugin {
);
}

#[should_panic]
#[test]
#[should_panic]
fn disable_option_set_from_profile_file_should_take_the_lowest_precedence() {
// TODO(aws-sdk-rust#1073): Implement a test that mimics only setting
// `s3_disable_express_session_auth` in a profile file
Expand All @@ -785,5 +791,20 @@ pub(crate) mod runtime_plugin {
.load::<crate::config::DisableS3ExpressSessionAuth>()
.is_none());
}

proptest! {
#[test]
#[should_panic]
fn env_value_other_than_case_sensitive_true_or_false_should_result_in_panic(
invalid_env_var_value in any::<String>()
.prop_filter("Use only invalid values for panic test",
|s| !valid_environment_variable_value(&s))
) {
let _ = S3ExpressRuntimePlugin::new_with(
Layer::new("test").freeze(),
Env::from_slice(&[(super::env::S3_DISABLE_EXPRESS_SESSION_AUTH, &invalid_env_var_value)]),
);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ private fun s3ExpressDependencies(runtimeConfig: RuntimeConfig) =
CargoDependency.Hex,
CargoDependency.Hmac,
CargoDependency.Lru,
CargoDependency.Proptest,
CargoDependency.Sha2,
CargoDependency.smithyAsync(runtimeConfig),
CargoDependency.smithyRuntimeApiClient(runtimeConfig),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ class FluentClientGenerator(
/// - Retries or timeouts are enabled without a `sleep_impl` configured.
/// - Identity caching is enabled without a `sleep_impl` and `time_source` configured.
/// - No `behavior_version` is provided.
/// - Base client-level runtime plugins could not be created.
///
/// The panic message for each of these will have instructions on how to resolve them.
##[track_caller]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,8 @@ data class CargoDependency(
val Hound: CargoDependency = CargoDependency("hound", CratesIo("3.4.0"), DependencyScope.Dev)
val PrettyAssertions: CargoDependency =
CargoDependency("pretty_assertions", CratesIo("1.3.0"), DependencyScope.Dev)
val Proptest: CargoDependency =
CargoDependency("proptest", CratesIo("1"), DependencyScope.Dev)
val SerdeJson: CargoDependency = CargoDependency("serde_json", CratesIo("1.0.0"), DependencyScope.Dev)
val Smol: CargoDependency = CargoDependency("smol", CratesIo("1.2.0"), DependencyScope.Dev)
val TempFile: CargoDependency = CargoDependency("tempfile", CratesIo("3.2.0"), DependencyScope.Dev)
Expand Down

0 comments on commit 3cdd811

Please sign in to comment.