Skip to content

Commit 4533810

Browse files
aajtoddaajtodd
authored
validate region is a valid host label when used in endpoints (#4383)
## Motivation and Context `V1988105516` ## Description * Adds validation for edge case where region is not a valid host label. * Introduces new `EndpointCustomization::endpointParamsBuilderValidator` method that customizations can use to add further validation to `crate::config::endpoint::ParamsBuilder::build()`. We could have done this in a few different places but this seemed most appropriate. ## Testing New codegen integration test. ## Checklist <!--- If a checkbox below is not applicable, then please DELETE it rather than leaving it unchecked --> - [x] For changes to the smithy-rs codegen or runtime crates, I have created a changelog entry Markdown file in the `.changelog` directory, specifying "client," "server," or both in the `applies_to` key. - [x] For changes to the AWS SDK, generated SDK code, or SDK runtime crates, I have created a changelog entry Markdown file in the `.changelog` directory, specifying "aws-sdk-rust" in the `applies_to` key. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
1 parent c71b922 commit 4533810

File tree

7 files changed

+204
-34
lines changed

7 files changed

+204
-34
lines changed

‎.changelog/1762379492.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
applies_to:
3+
- client
4+
- aws-sdk-rust
5+
authors:
6+
- aajtodd
7+
references: []
8+
breaking: false
9+
new_feature: false
10+
bug_fix: false
11+
---
12+
Validate `Region` is a valid host label when constructing endpoints.

‎aws/codegen-aws-sdk/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,16 @@ package software.amazon.smithy.rustsdk
88
import software.amazon.smithy.aws.traits.auth.SigV4Trait
99
import software.amazon.smithy.model.knowledge.ServiceIndex
1010
import software.amazon.smithy.model.node.Node
11+
import software.amazon.smithy.model.shapes.ShapeId
1112
import software.amazon.smithy.rulesengine.aws.language.functions.AwsBuiltIns
1213
import software.amazon.smithy.rulesengine.language.syntax.parameters.Parameter
1314
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
1415
import software.amazon.smithy.rust.codegen.client.smithy.configReexport
1516
import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator
1617
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.EndpointCustomization
18+
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.EndpointsLib
19+
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.generators.EndpointParamsGenerator
20+
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.memberName
1721
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ConfigCustomization
1822
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ServiceConfig
1923
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
@@ -143,6 +147,35 @@ class RegionDecorator : ClientCodegenDecorator {
143147
)
144148
}
145149
}
150+
151+
override fun endpointParamsBuilderValidator(
152+
codegenContext: ClientCodegenContext,
153+
parameter: Parameter,
154+
): Writable? {
155+
if (endpointTestsValidatesRegionAlready(codegenContext)) {
156+
return null
157+
}
158+
159+
return when (parameter.builtIn) {
160+
AwsBuiltIns.REGION.builtIn ->
161+
writable {
162+
rustTemplate(
163+
"""
164+
if let Some(region) = &self.${parameter.memberName()} {
165+
if !#{is_valid_host_label}(region.as_ref() as &str, true, &mut #{DiagnosticCollector}::new()) {
166+
return Err(#{ParamsError}::invalid_value(${parameter.memberName().dq()}, "must be a valid host label"))
167+
}
168+
}
169+
""",
170+
"is_valid_host_label" to EndpointsLib.isValidHostLabel,
171+
"ParamsError" to EndpointParamsGenerator.paramsError(),
172+
"DiagnosticCollector" to EndpointsLib.DiagnosticCollector,
173+
)
174+
}
175+
176+
else -> null
177+
}
178+
}
146179
},
147180
)
148181
}
@@ -238,3 +271,14 @@ fun usesRegion(codegenContext: ClientCodegenContext) =
238271
codegenContext.getBuiltIn(AwsBuiltIns.REGION) != null ||
239272
ServiceIndex.of(codegenContext.model)
240273
.getEffectiveAuthSchemes(codegenContext.serviceShape).containsKey(SigV4Trait.ID)
274+
275+
/**
276+
* Test if region is already validated via endpoint rules tests. Validating region when building parameters
277+
* will break endpoint tests which validate during resolution and expect specific errors.
278+
*/
279+
fun endpointTestsValidatesRegionAlready(codegenContext: ClientCodegenContext): Boolean =
280+
codegenContext.serviceShape.id in
281+
setOf(
282+
ShapeId.from("com.amazonaws.s3#AmazonS3"),
283+
ShapeId.from("com.amazonaws.s3control#AWSS3ControlServiceV20180820"),
284+
)

‎aws/codegen-aws-sdk/src/test/kotlin/software/amazon/smithy/rustsdk/RegionDecoratorTest.kt‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ package software.amazon.smithy.rustsdk
77
import org.junit.jupiter.api.Assertions.assertFalse
88
import org.junit.jupiter.api.Assertions.assertTrue
99
import org.junit.jupiter.api.Test
10+
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
11+
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
1012
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
13+
import software.amazon.smithy.rust.codegen.core.testutil.integrationTest
14+
import software.amazon.smithy.rust.codegen.core.testutil.tokioTest
1115
import kotlin.io.path.readText
1216

1317
class RegionDecoratorTest {
@@ -105,4 +109,48 @@ class RegionDecoratorTest {
105109
val configContents = path.resolve("src/config.rs").readText()
106110
assertTrue(configContents.contains("fn set_region("))
107111
}
112+
113+
// V1988105516
114+
@Test
115+
fun `models with region built-in params should validate host label`() {
116+
awsSdkIntegrationTest(modelWithRegionParam) { ctx, rustCrate ->
117+
val rc = ctx.runtimeConfig
118+
val codegenScope =
119+
arrayOf(
120+
*RuntimeType.preludeScope,
121+
"capture_request" to RuntimeType.captureRequest(rc),
122+
"Region" to AwsRuntimeType.awsTypes(rc).resolve("region::Region"),
123+
)
124+
125+
rustCrate.integrationTest("endpoint_params_validation") {
126+
tokioTest("region_must_be_valid_host_label") {
127+
val moduleName = ctx.moduleUseName()
128+
rustTemplate(
129+
"""
130+
let (http_client, _rx) = #{capture_request}(#{None});
131+
let client_config = $moduleName::Config::builder()
132+
.http_client(http_client)
133+
.region(#{Region}::new("@controlled-proxy.com##"))
134+
.build();
135+
136+
let client = $moduleName::Client::from_conf(client_config);
137+
138+
let err = client
139+
.some_operation()
140+
.send()
141+
.await
142+
.expect_err("error");
143+
144+
let err_str = format!("{}", $moduleName::error::DisplayErrorContext(&err));
145+
dbg!(&err_str);
146+
let expected = "invalid value for field: `region` - must be a valid host label";
147+
assert!(err_str.contains(expected));
148+
149+
""",
150+
*codegenScope,
151+
)
152+
}
153+
}
154+
}
155+
}
108156
}

‎aws/rust-runtime/Cargo.lock‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointsDecorator.kt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,43 @@ interface EndpointCustomization {
107107
codegenContext: ClientCodegenContext,
108108
params: String,
109109
): Writable? = null
110+
111+
/**
112+
* Allows injecting validation logic for endpoint parameters into the `ParamsBuilder::build` method.
113+
*
114+
* e.g. when generating the builder for the endpoint parameters this allows you to insert validation logic before
115+
* being finalizing the parameters.
116+
*
117+
* ```rs
118+
* impl ParamsBuilder {
119+
* pub fn build(self) -> ::std::result::Result<crate::config::endpoint::Params, crate::config::endpoint::InvalidParams> {
120+
* <validation logic>
121+
* ...
122+
* }
123+
* }
124+
*
125+
* Example:
126+
* ```kotlin
127+
*
128+
* override fun endpointParamsBuilderValidator(codegenContext: ClientCodegenContext, parameter: Parameter): Writable? {
129+
* rustTemplate("""
130+
* if let Some(region) = self.${parameter.memberName()} {
131+
* if #{is_valid_host_label}(region.as_ref() as &str, false, #{DiagnosticCollector}::new()) {
132+
* return Err(#{ParamsError}::invalid_value(${parameter.memberName().dq()}, "must be a valid host label"))
133+
* }
134+
* }
135+
* """,
136+
* "is_valid_host_label" to EndpointsLib.isValidHostLabel,
137+
* "ParamsError" to EndpointParamsGenerator.paramsError(),
138+
* "DiagnosticCollector" to EndpointsLib.DiagnosticCollector,
139+
* )
140+
* }
141+
* ```
142+
*/
143+
fun endpointParamsBuilderValidator(
144+
codegenContext: ClientCodegenContext,
145+
parameter: Parameter,
146+
): Writable? = null
110147
}
111148

112149
/**

‎codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsGenerator.kt‎

Lines changed: 58 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ val EndpointStdLib = RustModule.private("endpoint_lib")
112112
* ```
113113
*/
114114

115-
internal class EndpointParamsGenerator(
115+
class EndpointParamsGenerator(
116116
private val codegenContext: ClientCodegenContext,
117117
private val parameters: Parameters,
118118
) {
@@ -122,6 +122,52 @@ internal class EndpointParamsGenerator(
122122
fun setterName(parameterName: String) = "set_${memberName(parameterName)}"
123123

124124
fun getterName(parameterName: String) = "get_${memberName(parameterName)}"
125+
126+
fun paramsError(): RuntimeType =
127+
RuntimeType.forInlineFun("InvalidParams", ClientRustModule.Config.endpoint) {
128+
rust(
129+
"""
130+
/// An error that occurred during endpoint resolution
131+
##[derive(Debug)]
132+
pub struct InvalidParams {
133+
field: std::borrow::Cow<'static, str>,
134+
kind: InvalidParamsErrorKind,
135+
}
136+
137+
/// The kind of invalid parameter error
138+
##[derive(Debug)]
139+
enum InvalidParamsErrorKind {
140+
MissingField,
141+
InvalidValue {
142+
message: &'static str,
143+
}
144+
}
145+
146+
impl InvalidParams {
147+
##[allow(dead_code)]
148+
fn missing(field: &'static str) -> Self {
149+
Self { field: field.into(), kind: InvalidParamsErrorKind::MissingField }
150+
}
151+
152+
##[allow(dead_code)]
153+
fn invalid_value(field: &'static str, message: &'static str) -> Self {
154+
Self { field: field.into(), kind: InvalidParamsErrorKind::InvalidValue { message }}
155+
}
156+
}
157+
158+
impl std::fmt::Display for InvalidParams {
159+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
160+
match self.kind {
161+
InvalidParamsErrorKind::MissingField => write!(f, "a required field was missing: `{}`", self.field),
162+
InvalidParamsErrorKind::InvalidValue { message} => write!(f, "invalid value for field: `{}` - {}", self.field, message),
163+
}
164+
}
165+
}
166+
167+
impl std::error::Error for InvalidParams { }
168+
""",
169+
)
170+
}
125171
}
126172

127173
fun paramsStruct(): RuntimeType =
@@ -134,34 +180,6 @@ internal class EndpointParamsGenerator(
134180
generateEndpointParamsBuilder(this)
135181
}
136182

137-
private fun paramsError(): RuntimeType =
138-
RuntimeType.forInlineFun("InvalidParams", ClientRustModule.Config.endpoint) {
139-
rust(
140-
"""
141-
/// An error that occurred during endpoint resolution
142-
##[derive(Debug)]
143-
pub struct InvalidParams {
144-
field: std::borrow::Cow<'static, str>
145-
}
146-
147-
impl InvalidParams {
148-
##[allow(dead_code)]
149-
fn missing(field: &'static str) -> Self {
150-
Self { field: field.into() }
151-
}
152-
}
153-
154-
impl std::fmt::Display for InvalidParams {
155-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
156-
write!(f, "a required field was missing: `{}`", self.field)
157-
}
158-
}
159-
160-
impl std::error::Error for InvalidParams { }
161-
""",
162-
)
163-
}
164-
165183
/**
166184
* Generates an endpoints struct based on the provided endpoint rules. The struct fields are `pub(crate)`
167185
* with optionality as indicated by the required status of the parameter.
@@ -251,6 +269,17 @@ internal class EndpointParamsGenerator(
251269
"Params" to paramsStruct(),
252270
"ParamsError" to paramsError(),
253271
) {
272+
// additional validation for endpoint parameters during construction
273+
parameters.toList().forEach { parameter ->
274+
val validators =
275+
codegenContext.rootDecorator.endpointCustomizations(codegenContext)
276+
.mapNotNull { it.endpointParamsBuilderValidator(codegenContext, parameter) }
277+
278+
validators.forEach { validator ->
279+
rust("#W;", validator)
280+
}
281+
}
282+
254283
val params =
255284
writable {
256285
Attribute.AllowClippyUnnecessaryLazyEvaluations.render(this)

‎gradle.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,4 @@ allowLocalDeps=false
1717
# Avoid registering dependencies/plugins/tasks that are only used for testing purposes
1818
isTestingEnabled=true
1919
# codegen publication version
20-
codegenVersion=0.1.4
20+
codegenVersion=0.1.5

0 commit comments

Comments
 (0)