From f92855f6e0011b3cb9028d07470a9fc6c102880b Mon Sep 17 00:00:00 2001 From: david-perez Date: Thu, 27 Oct 2022 16:13:37 +0200 Subject: [PATCH 01/13] Bump `wrk-api-bench` to v0.0.8 (#1908) The current version is vulnerable. See https://github.com/crisidev/wrk-api-bench-rs/pull/1. --- .../aws-smithy-http-server/examples/pokemon-service/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-runtime/aws-smithy-http-server/examples/pokemon-service/Cargo.toml b/rust-runtime/aws-smithy-http-server/examples/pokemon-service/Cargo.toml index 82162950a1e..f1b842f7e33 100644 --- a/rust-runtime/aws-smithy-http-server/examples/pokemon-service/Cargo.toml +++ b/rust-runtime/aws-smithy-http-server/examples/pokemon-service/Cargo.toml @@ -47,7 +47,7 @@ pokemon-service-server-sdk = { path = "../pokemon-service-server-sdk/" } assert_cmd = "2.0" home = "0.5" serial_test = "0.7.0" -wrk-api-bench = "0.0.7" +wrk-api-bench = "0.0.8" # This dependency is only required for testing the `pokemon-service-tls` program. hyper-rustls = { version = "0.23.0", features = ["http2"] } From fc57e13cade87f8ad6de77a6aa048d66322651c2 Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Thu, 27 Oct 2022 11:09:18 -0400 Subject: [PATCH 02/13] Update changelog and use you task::spawn helper (#1915) --- CHANGELOG.next.toml | 6 ++++++ rust-runtime/aws-smithy-async/Cargo.toml | 1 + .../aws-smithy-async/src/future/fn_stream.rs | 17 +++++------------ 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 8326beeb140..307295aefe6 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -46,3 +46,9 @@ message = "`aws_smithy_http_server::routing::Router` is exported from the crate references = ["smithy-rs#1910"] meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server" } author = "david-perez" + +[[smithy-rs]] +message = "Fix bug that can cause panics in paginators" +references = ["smithy-rs#1903", "smithy-rs#1902"] +meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client"} +author = "rcoh" diff --git a/rust-runtime/aws-smithy-async/Cargo.toml b/rust-runtime/aws-smithy-async/Cargo.toml index 20e0507b046..04d600ea62f 100644 --- a/rust-runtime/aws-smithy-async/Cargo.toml +++ b/rust-runtime/aws-smithy-async/Cargo.toml @@ -18,6 +18,7 @@ futures-util = "0.3.16" [dev-dependencies] tokio = { version = "1.8.4", features = ["rt", "macros", "test-util"] } +tokio-test = "0.4.2" [package.metadata.docs.rs] all-features = true diff --git a/rust-runtime/aws-smithy-async/src/future/fn_stream.rs b/rust-runtime/aws-smithy-async/src/future/fn_stream.rs index 46d8d3850c0..804b08f6bbf 100644 --- a/rust-runtime/aws-smithy-async/src/future/fn_stream.rs +++ b/rust-runtime/aws-smithy-async/src/future/fn_stream.rs @@ -76,6 +76,8 @@ where Poll::Pending => { if let Some(generator) = me.generator.as_mut().as_pin_mut() { if generator.poll(cx).is_ready() { + // if the generator returned ready we MUST NOT poll it again—doing so + // will cause a panic. me.generator.set(None); } } @@ -144,10 +146,7 @@ where #[cfg(test)] mod test { use crate::future::fn_stream::{FnStream, TryFlatMap}; - use futures_util::task::noop_waker_ref; - use std::future::Future; use std::sync::{Arc, Mutex}; - use std::task::Context; use std::time::Duration; use tokio_stream::StreamExt; @@ -185,15 +184,9 @@ mod test { }) }); assert_eq!(stream.next().await, Some("blah")); - let mut fut = Box::pin(stream.next()); - assert!(fut - .as_mut() - .poll(&mut Context::from_waker(noop_waker_ref())) - .is_pending()); - assert!(fut - .as_mut() - .poll(&mut Context::from_waker(noop_waker_ref())) - .is_pending()); + let mut test_stream = tokio_test::task::spawn(stream); + assert!(test_stream.poll_next().is_pending()); + assert!(test_stream.poll_next().is_pending()); } /// Tests that the generator will not advance until demand exists From ddd290553c4f351acdb46e5de383fc29d0bd8256 Mon Sep 17 00:00:00 2001 From: Luca Palmieri <20745048+LukeMathWalker@users.noreply.github.com> Date: Thu, 27 Oct 2022 17:48:21 +0200 Subject: [PATCH 03/13] Add issue templates to nudge issue submitters towards our security policy if they want to report a vulnerability. (#1913) --- .github/ISSUE_TEMPLATE/blank_issue.md | 4 ++++ .github/ISSUE_TEMPLATE/config.yml | 5 +++++ 2 files changed, 9 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/blank_issue.md create mode 100644 .github/ISSUE_TEMPLATE/config.yml diff --git a/.github/ISSUE_TEMPLATE/blank_issue.md b/.github/ISSUE_TEMPLATE/blank_issue.md new file mode 100644 index 00000000000..9aef3ebe637 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/blank_issue.md @@ -0,0 +1,4 @@ +--- +name: Blank Issue +about: Create a blank issue. +--- diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 00000000000..68ac311d011 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,5 @@ +blank_issues_enabled: true +contact_links: +- name: Report a security vulnerability + url: https://github.com/awslabs/smithy-rs/security/policy + about: Please review our security policy for more details From b499d9345ca75dd45bbd1b4c59f29a9ad6471c2e Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 27 Oct 2022 11:07:33 -0700 Subject: [PATCH 04/13] Slow down crate publish more (#1911) Co-authored-by: Zelda Hessler --- tools/publisher/src/subcommand/publish.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/publisher/src/subcommand/publish.rs b/tools/publisher/src/subcommand/publish.rs index 58cc9ea8333..bf101d33fd4 100644 --- a/tools/publisher/src/subcommand/publish.rs +++ b/tools/publisher/src/subcommand/publish.rs @@ -66,15 +66,15 @@ pub async fn subcommand_publish( if !is_published(&package.handle).await? { publish(&package.handle, &package.crate_path).await?; + // Keep things slow to avoid getting throttled by crates.io + tokio::time::sleep(Duration::from_secs(2)).await; + // Sometimes it takes a little bit of time for the new package version // to become available after publish. If we proceed too quickly, then // the next package publish can fail if it depends on this package. wait_for_eventual_consistency(&package).await?; info!("Successfully published `{}`", package.handle); any_published = true; - - // Keep things slow to avoid getting throttled by crates.io - tokio::time::sleep(Duration::from_secs(1)).await; } else { info!("`{}` was already published", package.handle); } From 87e1ed0c941a0cd384761e7dc6b3f6422a584448 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 28 Oct 2022 10:12:19 +0100 Subject: [PATCH 05/13] Fix repeated options when creating an issue (#1916) --- .github/ISSUE_TEMPLATE/config.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 68ac311d011..3ba13e0cec6 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,5 +1 @@ -blank_issues_enabled: true -contact_links: -- name: Report a security vulnerability - url: https://github.com/awslabs/smithy-rs/security/policy - about: Please review our security policy for more details +blank_issues_enabled: false From c5c87be5e684704bf8fdb31e62e5e5c1b37531a8 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 28 Oct 2022 11:13:22 +0100 Subject: [PATCH 06/13] Add links to relevant issues/PRs for failing protocol tests (#1895) * Add links to relevant issues/PRs for failing tests * Update codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt Co-authored-by: david-perez * Format comment better * Remove `RestJsonStreamingTraitsRequireLengthWithBlob` from failing tests This test actually passes fine, but is not relevant for the server. * Add comment about `RestJsonHttpResponseCodeDefaultsToModeledCode` Co-authored-by: david-perez --- .../protocol/ServerProtocolTestGenerator.kt | 35 +++++++++++++------ 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt index cf709651074..7f07dea739f 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt @@ -872,31 +872,46 @@ class ServerProtocolTestGenerator( private val AwsQuery = "aws.protocoltests.query#AwsQuery" private val Ec2Query = "aws.protocoltests.ec2#AwsEc2" private val ExpectFail = setOf( - // Headers. + // Pending resolution from the Smithy team, see https://github.com/awslabs/smithy/issues/1068. FailingTest(RestJson, "RestJsonHttpWithHeadersButNoPayload", TestType.Request), - FailingTest(RestJson, "RestJsonEndpointTrait", TestType.Request), - FailingTest(RestJson, "RestJsonEndpointTraitWithHostLabel", TestType.Request), - FailingTest(RestJson, "RestJsonStreamingTraitsRequireLengthWithBlob", TestType.Response), FailingTest(RestJson, "RestJsonHttpWithEmptyBlobPayload", TestType.Request), FailingTest(RestJson, "RestJsonHttpWithEmptyStructurePayload", TestType.Request), + + // See https://github.com/awslabs/smithy/issues/1098 for context. FailingTest(RestJson, "RestJsonHttpResponseCodeDefaultsToModeledCode", TestType.Response), + // Endpoint trait is not implemented yet, see https://github.com/awslabs/smithy-rs/issues/950. + FailingTest(RestJson, "RestJsonEndpointTrait", TestType.Request), + FailingTest(RestJson, "RestJsonEndpointTraitWithHostLabel", TestType.Request), + + // Work in progress PR, see https://github.com/awslabs/smithy-rs/pull/1294. FailingTest(RestJson, "RestJsonBodyMalformedBlobInvalidBase64_case1", TestType.MalformedRequest), FailingTest(RestJson, "RestJsonBodyMalformedBlobInvalidBase64_case2", TestType.MalformedRequest), - FailingTest(RestJson, "RestJsonWithBodyExpectsApplicationJsonContentType", TestType.MalformedRequest), - FailingTest(RestJson, "RestJsonBodyMalformedListNullItem", TestType.MalformedRequest), - FailingTest(RestJson, "RestJsonBodyMalformedMapNullValue", TestType.MalformedRequest), - FailingTest(RestJson, "RestJsonMalformedSetDuplicateItems", TestType.MalformedRequest), - FailingTest(RestJson, "RestJsonMalformedSetNullItem", TestType.MalformedRequest), FailingTest( RestJson, "RestJsonHeaderMalformedStringInvalidBase64MediaType_case1", TestType.MalformedRequest, ), - FailingTest(RestJson, "RestJsonMalformedUnionNoFieldsSet", TestType.MalformedRequest), + + FailingTest(RestJson, "RestJsonWithBodyExpectsApplicationJsonContentType", TestType.MalformedRequest), + FailingTest(RestJson, "RestJsonBodyMalformedListNullItem", TestType.MalformedRequest), + FailingTest(RestJson, "RestJsonBodyMalformedMapNullValue", TestType.MalformedRequest), + + // Deprioritized, sets don't exist in Smithy 2.0. + // They have the exact same semantics as list shapes with `@uniqueItems`, + // so we could implement them as such once we've added support for constraint traits. + // + // See https://github.com/awslabs/smithy/issues/1266#issuecomment-1169543051. + // See https://awslabs.github.io/smithy/2.0/guides/migrating-idl-1-to-2.html#convert-set-shapes-to-list-shapes. + FailingTest(RestJson, "RestJsonMalformedSetDuplicateItems", TestType.MalformedRequest), + FailingTest(RestJson, "RestJsonMalformedSetNullItem", TestType.MalformedRequest), FailingTest(RestJson, "RestJsonMalformedSetDuplicateBlobs", TestType.MalformedRequest), + FailingTest(RestJson, "RestJsonMalformedUnionNoFieldsSet", TestType.MalformedRequest), + + // Tests involving constraint traits, which are not yet implemented. + // See https://github.com/awslabs/smithy-rs/pull/1342. FailingTest(RestJsonValidation, "RestJsonMalformedEnumList_case0", TestType.MalformedRequest), FailingTest(RestJsonValidation, "RestJsonMalformedEnumList_case1", TestType.MalformedRequest), FailingTest(RestJsonValidation, "RestJsonMalformedEnumMapKey_case0", TestType.MalformedRequest), From 4c852b1d0b4456493869db13a934eecb5c81657b Mon Sep 17 00:00:00 2001 From: Zelda Hessler Date: Fri, 28 Oct 2022 15:30:41 -0500 Subject: [PATCH 07/13] Add operation metadata to property bag just before sending request through middleware (#1920) * update: add operation metadata to property bag during `make_operation` add: test ensuring metadata is added to property bag add: CHANGELOG.next.toml entry * update: use new strategy for op metadata insertion update: a new strategy requires a new test update: CHANGELOG.next.toml * format: run cargo fmt --- CHANGELOG.next.toml | 9 +++++ .../tests/middleware_e2e_test.rs | 38 +++++++++++++++++++ .../protocol/MakeOperationGenerator.kt | 9 +++-- .../src/parse_response.rs | 3 +- 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 307295aefe6..6ca2c0ee650 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -52,3 +52,12 @@ message = "Fix bug that can cause panics in paginators" references = ["smithy-rs#1903", "smithy-rs#1902"] meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client"} author = "rcoh" + +[[smithy-rs]] +message = """ +Operation metadata is now added to the property bag before sending requests allowing middlewares to behave +differently depending on the operation being sent. +""" +references = ["smithy-rs#1919"] +meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client"} +author = "Velfi" diff --git a/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs b/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs index 90fdc32373b..8dfafce802e 100644 --- a/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs +++ b/aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs @@ -19,6 +19,7 @@ use aws_http::retry::AwsResponseRetryClassifier; use aws_http::user_agent::AwsUserAgent; use aws_inlineable::middleware::DefaultMiddleware; use aws_sig_auth::signer::OperationSigningConfig; +use aws_smithy_client::erase::DynConnector; use aws_smithy_client::test_connection::TestConnection; use aws_smithy_http::body::SdkBody; @@ -112,6 +113,7 @@ fn test_operation() -> Operation() + .cloned() + .unwrap(); + + assert_eq!("test-op", metadata.name()); + assert_eq!("test-service", metadata.service()); + + req + }) + .connector(DynConnector::new(conn)) + .build(); + + let resp = client.call(test_operation()).await; + let resp = resp.expect("successful operation"); + assert_eq!(resp, "Hello!"); +} diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/protocol/MakeOperationGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/protocol/MakeOperationGenerator.kt index cda444132cc..c77ef20c4a4 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/protocol/MakeOperationGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/protocol/MakeOperationGenerator.kt @@ -71,7 +71,8 @@ open class MakeOperationGenerator( ) { val operationName = symbolProvider.toSymbol(shape).name val baseReturnType = buildOperationType(implBlockWriter, shape, customizations) - val returnType = "std::result::Result<$baseReturnType, ${implBlockWriter.format(runtimeConfig.operationBuildError())}>" + val returnType = + "std::result::Result<$baseReturnType, ${implBlockWriter.format(runtimeConfig.operationBuildError())}>" val outputSymbol = symbolProvider.toSymbol(shape) val takesOwnership = bodyGenerator.payloadMetadata(shape).takesOwnership @@ -82,8 +83,10 @@ open class MakeOperationGenerator( implBlockWriter.docs("Consumes the builder and constructs an Operation<#D>", outputSymbol) Attribute.AllowUnusedMut.render(implBlockWriter) // For codegen simplicity - Attribute.Custom("allow(clippy::let_and_return)").render(implBlockWriter) // For codegen simplicity, allow `let x = ...; x` - Attribute.Custom("allow(clippy::needless_borrow)").render(implBlockWriter) // Allows builders that don’t consume the input borrow + Attribute.Custom("allow(clippy::let_and_return)") + .render(implBlockWriter) // For codegen simplicity, allow `let x = ...; x` + Attribute.Custom("allow(clippy::needless_borrow)") + .render(implBlockWriter) // Allows builders that don’t consume the input borrow implBlockWriter.rustBlockTemplate( "$fnType $functionName($self, _config: &#{config}::Config) -> $returnType", *codegenScope, diff --git a/rust-runtime/aws-smithy-http-tower/src/parse_response.rs b/rust-runtime/aws-smithy-http-tower/src/parse_response.rs index b711fc9885d..5dfd71f9a11 100644 --- a/rust-runtime/aws-smithy-http-tower/src/parse_response.rs +++ b/rust-runtime/aws-smithy-http-tower/src/parse_response.rs @@ -88,7 +88,7 @@ where } fn call(&mut self, req: Operation) -> Self::Future { - let (req, parts) = req.into_request_response(); + let (mut req, parts) = req.into_request_response(); let handler = parts.response_handler; // send_operation records the full request-response lifecycle. // NOTE: For operations that stream output, only the setup is captured in this span. @@ -103,6 +103,7 @@ where if let Some(metadata) = parts.metadata { span.record("operation", &metadata.name()); span.record("service", &metadata.service()); + req.properties_mut().insert(metadata); } let resp = self.inner.call(req); let fut = async move { From 5eb3c8227d8b4d59d43fe27ba8b2c3780a4fd893 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 31 Oct 2022 14:07:15 +0000 Subject: [PATCH 08/13] Run HttpMalformedRequest tests through router (#1904) * Add links to relevant issues/PRs for failing tests * Update codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt Co-authored-by: david-perez * Make `build_router_and_make_request` return the response * Use `rustWriter.write` over `rustWriter.rust` for incomplete rust code * Run HttpMalformedRequest tests through router * Don't assert on operation_extension This causes HttpMalformedRequest tests to fail. * Refactor code to avoid repetition * Run ktlint * Check operation extension for regular request tests * Run ktlint * Remove unused parameter from `makeRequest` * Refactor checkRequest2 * Remove `checkRequest/2`, run new-style tests for malformed http requests - Remove `checkRequest`, `checkRequest2` in favour of using `makeRequest`/`makeRequest2` directly - Run service builder tests for MalformedHttpRequest tests * Format comment better * Remove `RestJsonStreamingTraitsRequireLengthWithBlob` from failing tests This test actually passes fine, but is not relevant for the server. * Add comment about `RestJsonHttpResponseCodeDefaultsToModeledCode` * Remove channel assertion on `makeRequest2` * Use `checkOperationExtension` for `makeRequest2` * Attempt to use a channel for both `makeRequest/2` * Go back to using channel for new api and operation extension for old api * Fix malformedHttpRequest tests * Add block around each API's tests * Remove unrelated file * Mark wrong tests as failing Marks these two tests as failing: - RestJsonWithPayloadExpectsImpliedContentType - RestJsonBodyMalformedMapNullKey These will be fixed in https://github.com/awslabs/smithy/pull/1477. Co-authored-by: david-perez --- .../protocol/ServerProtocolTestGenerator.kt | 101 ++++++++++++------ 1 file changed, 69 insertions(+), 32 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt index 7f07dea739f..ad28e4bc71b 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt @@ -37,6 +37,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.rustlang.withBlock +import software.amazon.smithy.rust.codegen.core.rustlang.withBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType @@ -200,12 +201,10 @@ class ServerProtocolTestGenerator( #{RegistryBuilderMethods:W} } - /// The operation full name is a concatenation of `.`. pub(crate) async fn build_router_and_make_request( http_request: #{Http}::request::Request<#{SmithyHttpServer}::body::Body>, - operation_full_name: &str, f: &dyn Fn(RegistryBuilder) -> RegistryBuilder, - ) { + ) -> #{Http}::response::Response<#{SmithyHttpServer}::body::BoxBody> { let mut router: #{Router} = f(create_operation_registry_builder()) .build() .expect("unable to build operation registry") @@ -214,6 +213,12 @@ class ServerProtocolTestGenerator( .call(http_request) .await .expect("unable to make an HTTP request"); + + http_response + } + + /// The operation full name is a concatenation of `.`. + pub(crate) fn check_operation_extension_was_set(http_response: #{Http}::response::Response<#{SmithyHttpServer}::body::BoxBody>, operation_full_name: &str) { let operation_extension = http_response.extensions() .get::<#{SmithyHttpServer}::extension::OperationExtension>() .expect("extension `OperationExtension` not found"); @@ -284,6 +289,7 @@ class ServerProtocolTestGenerator( is TestCase.MalformedRequestTest -> this.renderHttpMalformedRequestTestCase( it.testCase, + operationShape, operationSymbol, ) } @@ -388,7 +394,8 @@ class ServerProtocolTestGenerator( renderHttpRequest(uri, method, headers, body.orNull(), queryParams, host.orNull()) } if (protocolSupport.requestBodyDeserialization) { - checkRequest(operationShape, operationSymbol, httpRequestTestCase, this) + makeRequest(operationShape, this, checkRequestHandler(operationShape, httpRequestTestCase)) + checkHandlerWasEntered(operationShape, operationSymbol, this) } // Test against new service builder. @@ -396,7 +403,8 @@ class ServerProtocolTestGenerator( renderHttpRequest(uri, method, headers, body.orNull(), queryParams, host.orNull()) } if (protocolSupport.requestBodyDeserialization) { - checkRequest2(operationShape, operationSymbol, httpRequestTestCase, this) + makeRequest2(operationShape, operationSymbol, this, checkRequestHandler(operationShape, httpRequestTestCase)) + checkHandlerWasEntered2(this) } // Explicitly warn if the test case defined parameters that we aren't doing anything with @@ -467,24 +475,30 @@ class ServerProtocolTestGenerator( */ private fun RustWriter.renderHttpMalformedRequestTestCase( testCase: HttpMalformedRequestTestCase, + operationShape: OperationShape, operationSymbol: Symbol, ) { - with(testCase.request) { - // TODO(https://github.com/awslabs/smithy/issues/1102): `uri` should probably not be an `Optional`. - renderHttpRequest(uri.get(), method, headers, body.orNull(), queryParams, host.orNull()) + val (_, outputT) = operationInputOutputTypes[operationShape]!! + + rust("// Use the `OperationRegistryBuilder`") + rustBlock("") { + with(testCase.request) { + // TODO(https://github.com/awslabs/smithy/issues/1102): `uri` should probably not be an `Optional`. + renderHttpRequest(uri.get(), method, headers, body.orNull(), queryParams, host.orNull()) + } + makeRequest(operationShape, this, writable("todo!() as $outputT")) + checkResponse(this, testCase.response) } - val operationName = "${operationSymbol.name}${ServerHttpBoundProtocolGenerator.OPERATION_INPUT_WRAPPER_SUFFIX}" - rustTemplate( - """ - let mut http_request = #{SmithyHttpServer}::request::RequestParts::new(http_request); - let rejection = super::$operationName::from_request(&mut http_request).await.expect_err("request was accepted but we expected it to be rejected"); - let http_response = #{SmithyHttpServer}::response::IntoResponse::<#{Protocol}>::into_response(rejection); - """, - "Protocol" to protocolGenerator.protocol.markerStruct(), - *codegenScope, - ) - checkResponse(this, testCase.response) + rust("// Use new service builder") + rustBlock("") { + with(testCase.request) { + // TODO(https://github.com/awslabs/smithy/issues/1102): `uri` should probably not be an `Optional`. + renderHttpRequest(uri.get(), method, headers, body.orNull(), queryParams, host.orNull()) + } + makeRequest2(operationShape, operationSymbol, this, writable("todo!() as $outputT")) + checkResponse(this, testCase.response) + } } private fun RustWriter.renderHttpRequest( @@ -563,41 +577,53 @@ class ServerProtocolTestGenerator( } /** Checks the request using the `OperationRegistryBuilder`. */ - private fun checkRequest( + private fun makeRequest( operationShape: OperationShape, - operationSymbol: Symbol, - httpRequestTestCase: HttpRequestTestCase, rustWriter: RustWriter, + operationBody: Writable, ) { val (inputT, outputT) = operationInputOutputTypes[operationShape]!! - rustWriter.withBlock( + rustWriter.withBlockTemplate( """ - super::$PROTOCOL_TEST_HELPER_MODULE_NAME::build_router_and_make_request( + let http_response = super::$PROTOCOL_TEST_HELPER_MODULE_NAME::build_router_and_make_request( http_request, - "${operationShape.id.namespace}.${operationSymbol.name}", &|builder| { builder.${operationShape.toName()}((|input| Box::pin(async move { """, "})) as super::$PROTOCOL_TEST_HELPER_MODULE_NAME::Fun<$inputT, $outputT>)}).await;", - + *codegenScope, ) { - checkRequestHandler(operationShape, httpRequestTestCase)() + operationBody() } } + private fun checkHandlerWasEntered( + operationShape: OperationShape, + operationSymbol: Symbol, + rustWriter: RustWriter, + ) { + val operationFullName = "${operationShape.id.namespace}.${operationSymbol.name}" + rustWriter.rust( + """ + super::$PROTOCOL_TEST_HELPER_MODULE_NAME::check_operation_extension_was_set(http_response, "$operationFullName"); + """, + ) + } + /** Checks the request using the new service builder. */ - private fun checkRequest2( + private fun makeRequest2( operationShape: OperationShape, operationSymbol: Symbol, - httpRequestTestCase: HttpRequestTestCase, rustWriter: RustWriter, + body: Writable, ) { val (inputT, _) = operationInputOutputTypes[operationShape]!! val operationName = RustReservedWords.escapeIfNeeded(operationSymbol.name.toSnakeCase()) rustWriter.rustTemplate( """ + ##[allow(unused_mut)] let (sender, mut receiver) = #{Tokio}::sync::mpsc::channel(1); let service = crate::service::$serviceName::unchecked_builder() .$operationName(move |input: $inputT| { @@ -612,13 +638,20 @@ class ServerProtocolTestGenerator( let http_response = #{Tower}::ServiceExt::oneshot(service, http_request) .await .expect("unable to make an HTTP request"); - assert!(receiver.recv().await.is_some()) """, - "Body" to checkRequestHandler(operationShape, httpRequestTestCase), + "Body" to body, *codegenScope, ) } + private fun checkHandlerWasEntered2(rustWriter: RustWriter) { + rustWriter.rust( + """ + assert!(receiver.recv().await.is_some()); + """, + ) + } + private fun checkRequestParams(inputShape: StructureShape, rustWriter: RustWriter) { if (inputShape.hasStreamingMember(model)) { // A streaming shape does not implement `PartialEq`, so we have to iterate over the input shape's members @@ -842,7 +875,7 @@ class ServerProtocolTestGenerator( private fun assertOk(rustWriter: RustWriter, inner: Writable) { rustWriter.rust("#T(", RuntimeType.ProtocolTestHelper(codegenContext.runtimeConfig, "assert_ok")) inner(rustWriter) - rustWriter.rust(");") + rustWriter.write(");") } private fun strSlice(writer: RustWriter, args: List) { @@ -872,6 +905,10 @@ class ServerProtocolTestGenerator( private val AwsQuery = "aws.protocoltests.query#AwsQuery" private val Ec2Query = "aws.protocoltests.ec2#AwsEc2" private val ExpectFail = setOf( + // Pending merge from the Smithy team: see https://github.com/awslabs/smithy/pull/1477. + FailingTest(RestJson, "RestJsonWithPayloadExpectsImpliedContentType", TestType.MalformedRequest), + FailingTest(RestJson, "RestJsonBodyMalformedMapNullKey", TestType.MalformedRequest), + // Pending resolution from the Smithy team, see https://github.com/awslabs/smithy/issues/1068. FailingTest(RestJson, "RestJsonHttpWithHeadersButNoPayload", TestType.Request), From 3d8294616d7b91083c6fb163a5c413dac3f18eaa Mon Sep 17 00:00:00 2001 From: Harry Barber <106155934+hlbarber@users.noreply.github.com> Date: Mon, 31 Oct 2022 15:28:40 +0000 Subject: [PATCH 09/13] Add JSON serialization input/output extension points (#1899) --- .../serialize/JsonSerializerGenerator.kt | 18 +++++++++++++----- .../server/smithy/protocols/ServerAwsJson.kt | 1 + 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt index bd527f5caea..415a0a8de82 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt @@ -58,6 +58,12 @@ import software.amazon.smithy.rust.codegen.core.util.outputShape sealed class JsonSection(name: String) : Section(name) { /** Mutate the server error object prior to finalization. Eg: this can be used to inject `__type` to record the error type. */ data class ServerError(val structureShape: StructureShape, val jsonObject: String) : JsonSection("ServerError") + + /** Mutate the input object prior to finalization. */ + data class InputStruct(val structureShape: StructureShape, val jsonObject: String) : JsonSection("InputStruct") + + /** Mutate the output object prior to finalization. */ + data class OutputStruct(val structureShape: StructureShape, val jsonObject: String) : JsonSection("OutputStruct") } /** @@ -167,13 +173,14 @@ class JsonSerializerGenerator( /** * Reusable structure serializer implementation that can be used to generate serializing code for - * operation, error and structure shapes. + * operation outputs or errors. * This function is only used by the server, the client uses directly [serializeStructure]. */ - private fun serverStructureSerializer( + private fun serverSerializer( fnName: String, structureShape: StructureShape, includedMembers: List, + makeSection: (StructureShape, String) -> JsonSection, ): RuntimeType { return RuntimeType.forInlineFun(fnName, operationSerModule) { rustBlockTemplate( @@ -184,7 +191,7 @@ class JsonSerializerGenerator( rust("let mut out = String::new();") rustTemplate("let mut object = #{JsonObjectWriter}::new(&mut out);", *codegenScope) serializeStructure(StructContext("object", "value", structureShape), includedMembers) - customizations.forEach { it.section(JsonSection.ServerError(structureShape, "object"))(this) } + customizations.forEach { it.section(makeSection(structureShape, "object"))(this) } rust("object.finish();") rustTemplate("Ok(out)", *codegenScope) } @@ -244,6 +251,7 @@ class JsonSerializerGenerator( rust("let mut out = String::new();") rustTemplate("let mut object = #{JsonObjectWriter}::new(&mut out);", *codegenScope) serializeStructure(StructContext("object", "input", inputShape), httpDocumentMembers) + customizations.forEach { it.section(JsonSection.InputStruct(inputShape, "object"))(this) } rust("object.finish();") rustTemplate("Ok(#{SdkBody}::from(out))", *codegenScope) } @@ -285,7 +293,7 @@ class JsonSerializerGenerator( val outputShape = operationShape.outputShape(model) val fnName = symbolProvider.serializeFunctionName(outputShape) - return serverStructureSerializer(fnName, outputShape, httpDocumentMembers) + return serverSerializer(fnName, outputShape, httpDocumentMembers, JsonSection::OutputStruct) } override fun serverErrorSerializer(shape: ShapeId): RuntimeType { @@ -294,7 +302,7 @@ class JsonSerializerGenerator( httpBindingResolver.errorResponseBindings(shape).filter { it.location == HttpLocation.DOCUMENT } .map { it.member } val fnName = symbolProvider.serializeFunctionName(errorShape) - return serverStructureSerializer(fnName, errorShape, includedMembers) + return serverSerializer(fnName, errorShape, includedMembers, JsonSection::ServerError) } private fun RustWriter.serializeStructure( diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerAwsJson.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerAwsJson.kt index e82cea742a6..8215d485ba6 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerAwsJson.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerAwsJson.kt @@ -71,6 +71,7 @@ class ServerAwsJsonError(private val awsJsonVersion: AwsJsonVersion) : JsonCusto rust("""${section.jsonObject}.key("__type").string("${escape(typeId)}");""") } } + else -> emptySection } } From b8eb3419a05577ddfc80b1b78fe8b8cd22a7926f Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 31 Oct 2022 16:47:27 +0000 Subject: [PATCH 10/13] Use plain cargo to test runtime crates instead of gradle (#1924) --- README.md | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 0cf99e8d25f..32d56512660 100644 --- a/README.md +++ b/README.md @@ -60,19 +60,29 @@ Some components, such as `codegen-client-test` and `codegen-server-test`, are pu To test the `rust-runtime` crates: ```bash -# Run all Rust tests for `rust-runtime/` -./gradlew rust-runtime:cargoTest -# Run clippy for `rust-runtime/` -./gradlew rust-runtime:cargoClippy +# Run all Rust tests for `rust-runtime/` (from repo root): +cargo test --manifest-path=rust-runtime/Cargo.toml +# Run clippy for `rust-runtime/` (from repo root): +cargo clippy --manifest-path=rust-runtime/Cargo.toml + +# Or +cd rust-runtime +cargo test +cargo clippy ``` -For `aws/rust-runtime`, just prefix with `aws:`: +To test the `aws/rust-runtime` crates: ```bash -# Run all Rust tests for `rust-runtime/` -./gradlew aws:rust-runtime:cargoTest -# Run clippy for `rust-runtime/` -./gradlew aws:rust-runtime:cargoClippy +# Run all Rust tests for `aws/rust-runtime/` (from repo root): +cargo test --manifest-path=aws/rust-runtime/Cargo.toml +# Run clippy for `aws/rust-runtime/` (from repo root): +cargo clippy --manifest-path=aws/rust-runtime/Cargo.toml + +# Or +cd aws/rust-runtime +cargo test +cargo clippy ``` Some runtime crates have a `additional-ci` script that can also be run. These scripts often require From 2aafd044e26b5e3410efd357af1fb74a0c916d58 Mon Sep 17 00:00:00 2001 From: Zelda Hessler Date: Mon, 31 Oct 2022 13:39:19 -0500 Subject: [PATCH 11/13] update: to smithy v1.26 (#1929) * update: to smithy v1.26 * add: entry to CHANGELOG.next.toml --- CHANGELOG.next.toml | 6 ++++++ gradle.properties | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 6ca2c0ee650..4be2815b37f 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -61,3 +61,9 @@ differently depending on the operation being sent. references = ["smithy-rs#1919"] meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client"} author = "Velfi" + +[[smithy-rs]] +message = "Upgrade Smithy to v1.26" +references = ["smithy-rs#1929"] +meta = { "breaking" = false, "tada" = true, "bug" = false, "target" = "all"} +author = "Velfi" diff --git a/gradle.properties b/gradle.properties index cb73ffe2b60..2a123b94464 100644 --- a/gradle.properties +++ b/gradle.properties @@ -15,7 +15,7 @@ kotlin.code.style=official # codegen smithyGradlePluginVersion=0.6.0 -smithyVersion=1.25.0 +smithyVersion=1.26.0 # kotlin kotlinVersion=1.6.21 From f0b7f55bf515f85bdb72975f355e297badc8a2f4 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Mon, 31 Oct 2022 15:06:09 -0500 Subject: [PATCH 12/13] Fix cargo audit issue on chrono (#1907) * Avoid the chrono crate depending on the time crate This commit is in response to RUSTSEC-2020-0071 where there is a potential segfault in the time crate. The aws-smithy-types-convert crate now disables the default features of the chrono crate so that it will not depend on the time crate. * Depend on lambda_http without RUSTSEC-2020-0071 This commit updates the version of lambda_http used by aws-smithy-http-server and aws-smithy-http-server-python to 0.7.0. The prior version 0.6.0 of lambda_http used the chrono crate in a way that exposed a security issue described in RUSTSEC-2020-0071. By switching to 0.7.0 of lambda_http, those two crates do not exhibit vulnerabilities as reported by cargo audit. * Bump minor version of lambda_http in pokemon-service This commit updates the version of `lambda_http` used by `pokemon-service` from 0.6.0 to 0.7.0. This is in sync with the fact that both `aws-smithy-http-server` and `aws-smithy-http-server-python` now depend on 0.7.0 of `lambda_http`. Failing to do so would cause `pokemon-service` to fail to compile due to an error at `lambda_http::run(handler)` in the main function of the `pokemon-service-lambda` binary: the trait `Service>` is not implemented for `LambdaHandler` * Depend on lambda-http 0.7.1 This commit updates the version of `lambda_http` from 0.7.0 to 0.7.1 in the crates within the top-level `rust-runtime` workspace. These updates are needed to solve the issue described in awslabs/aws-lambda-rust-runtime#556 * Update CHANGELOG.next.toml * Address https://github.com/awslabs/smithy-rs/pull/1907\#pullrequestreview-1161609833 Co-authored-by: Saito Co-authored-by: Zelda Hessler --- CHANGELOG.next.toml | 6 ++++++ rust-runtime/aws-smithy-http-server-python/Cargo.toml | 2 +- rust-runtime/aws-smithy-http-server/Cargo.toml | 2 +- .../examples/pokemon-service/Cargo.toml | 2 +- rust-runtime/aws-smithy-types-convert/Cargo.toml | 2 +- 5 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 4be2815b37f..64cfb40a1eb 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -67,3 +67,9 @@ message = "Upgrade Smithy to v1.26" references = ["smithy-rs#1929"] meta = { "breaking" = false, "tada" = true, "bug" = false, "target" = "all"} author = "Velfi" + +[[smithy-rs]] +message = "Fix cargo audit issue on chrono." +references = ["smithy-rs#1907"] +meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "all" } +author = "ysaito1001" diff --git a/rust-runtime/aws-smithy-http-server-python/Cargo.toml b/rust-runtime/aws-smithy-http-server-python/Cargo.toml index 52e1e537af7..1a4e4098b0e 100644 --- a/rust-runtime/aws-smithy-http-server-python/Cargo.toml +++ b/rust-runtime/aws-smithy-http-server-python/Cargo.toml @@ -22,7 +22,7 @@ bytes = "1.2" futures = "0.3" http = "0.2" hyper = { version = "0.14.20", features = ["server", "http1", "http2", "tcp", "stream"] } -lambda_http = "0.6.0" +lambda_http = "0.7.1" num_cpus = "1.13.1" parking_lot = "0.12.1" pin-project-lite = "0.2" diff --git a/rust-runtime/aws-smithy-http-server/Cargo.toml b/rust-runtime/aws-smithy-http-server/Cargo.toml index 055b7aab8cb..10888a09e54 100644 --- a/rust-runtime/aws-smithy-http-server/Cargo.toml +++ b/rust-runtime/aws-smithy-http-server/Cargo.toml @@ -26,7 +26,7 @@ futures-util = { version = "0.3", default-features = false } http = "0.2" http-body = "0.4" hyper = { version = "0.14.12", features = ["server", "http1", "http2", "tcp", "stream"] } -lambda_http = "0.6.0" +lambda_http = "0.7.1" mime = "0.3" nom = "7" pin-project-lite = "0.2" diff --git a/rust-runtime/aws-smithy-http-server/examples/pokemon-service/Cargo.toml b/rust-runtime/aws-smithy-http-server/examples/pokemon-service/Cargo.toml index f1b842f7e33..e6a078f897d 100644 --- a/rust-runtime/aws-smithy-http-server/examples/pokemon-service/Cargo.toml +++ b/rust-runtime/aws-smithy-http-server/examples/pokemon-service/Cargo.toml @@ -37,7 +37,7 @@ rustls-pemfile = "1.0.1" futures-util = "0.3" # This dependency is only required for the `pokemon-service-lambda` program. -lambda_http = "0.6.0" +lambda_http = "0.7.1" # Local paths aws-smithy-http-server = { path = "../../" } diff --git a/rust-runtime/aws-smithy-types-convert/Cargo.toml b/rust-runtime/aws-smithy-types-convert/Cargo.toml index 552c3e106ac..c9ffbd22a01 100644 --- a/rust-runtime/aws-smithy-types-convert/Cargo.toml +++ b/rust-runtime/aws-smithy-types-convert/Cargo.toml @@ -13,7 +13,7 @@ convert-time = ["aws-smithy-types", "time"] [dependencies] aws-smithy-types = { path = "../aws-smithy-types", optional = true } -chrono = { version = "0.4.19", optional = true } +chrono = { version = "0.4.19", optional = true, default-features = false, features = ["std"] } time = { version = "0.3.4", optional = true } [package.metadata.docs.rs] From e9f876a16667475c556c6217d78df5cd6fffb441 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Mon, 31 Oct 2022 16:42:56 -0700 Subject: [PATCH 13/13] Upgrade `cargo-check-external-types` to 0.1.5 (#1882) --- aws/rust-runtime/aws-inlineable/external-types.toml | 6 ++++++ aws/sdk/sdk-external-types.toml | 5 +++++ tools/Dockerfile | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/aws/rust-runtime/aws-inlineable/external-types.toml b/aws/rust-runtime/aws-inlineable/external-types.toml index c3a3fd53268..e46c02842d9 100644 --- a/aws/rust-runtime/aws-inlineable/external-types.toml +++ b/aws/rust-runtime/aws-inlineable/external-types.toml @@ -1,6 +1,10 @@ allowed_external_types = [ + "aws_endpoint::*", + "aws_http::*", + "aws_sig_auth::*", "aws_smithy_client::*", "aws_smithy_http::*", + "aws_smithy_http_tower::*", "aws_smithy_types::*", "aws_types::*", "http::header::map::HeaderMap", @@ -12,4 +16,6 @@ allowed_external_types = [ # TODO(https://github.com/awslabs/smithy-rs/issues/1193): Decide if we want to continue exposing tower_layer "tower_layer::Layer", + "tower_layer::identity::Identity", + "tower_layer::stack::Stack", ] diff --git a/aws/sdk/sdk-external-types.toml b/aws/sdk/sdk-external-types.toml index e346c691137..3000eee7504 100644 --- a/aws/sdk/sdk-external-types.toml +++ b/aws/sdk/sdk-external-types.toml @@ -1,9 +1,12 @@ # These are the allowed external types in the `aws-sdk-*` generated crates, checked by CI. allowed_external_types = [ + "aws_endpoint::*", "aws_http::*", + "aws_sig_auth::*", "aws_smithy_async::*", "aws_smithy_client::*", "aws_smithy_http::*", + "aws_smithy_http_tower::*", "aws_smithy_types::*", "aws_types::*", "http::header::map::HeaderMap", @@ -21,4 +24,6 @@ allowed_external_types = [ # TODO(https://github.com/awslabs/smithy-rs/issues/1193): Decide if we want to continue exposing tower_layer "tower_layer::Layer", + "tower_layer::identity::Identity", + "tower_layer::stack::Stack", ] diff --git a/tools/Dockerfile b/tools/Dockerfile index b2f11d4d90d..e2445926bb4 100644 --- a/tools/Dockerfile +++ b/tools/Dockerfile @@ -51,7 +51,7 @@ ARG cargo_deny_version=0.12.2 ARG cargo_udeps_version=0.1.29 ARG cargo_hack_version=0.5.14 ARG cargo_minimal_versions_version=0.1.4 -ARG cargo_check_external_types_version=0.1.4 +ARG cargo_check_external_types_version=0.1.5 ENV RUSTUP_HOME=/opt/rustup \ CARGO_HOME=/opt/cargo \ PATH=/opt/cargo/bin/:${PATH} \