From 83fab65af6561e2c550da6761d619c22a9adfe68 Mon Sep 17 00:00:00 2001 From: david-perez Date: Mon, 3 Apr 2023 16:54:19 +0200 Subject: [PATCH 1/3] Check `Content-Type` header in all server protocols --- .../ServerHttpBoundProtocolGenerator.kt | 40 +++++++++---------- .../src/proto/rest_json_1/rejection.rs | 2 + .../src/proto/rest_json_1/runtime_error.rs | 3 ++ 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt index eb2fd10a3c..e0e5897fd9 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt @@ -170,6 +170,11 @@ class ServerHttpBoundProtocolTraitImplGenerator( ) } } + // This checks for the expected `Content-Type` header if the `@httpPayload` trait is present, as dictated by + // the core Smithy library, which _does not_ require deserializing the payload. + // If no members have `@httpPayload`, the expected `Content-Type` header as dictated _by the protocol_ is + // checked later on for non-streaming operations, in `serverRenderShapeParser`: that check _does_ require at + // least buffering the entire payload, since the check must only be performed if the payload is empty. val verifyRequestContentTypeHeader = writable { operationShape .inputShape(model) @@ -178,12 +183,13 @@ class ServerHttpBoundProtocolTraitImplGenerator( ?.let { payload -> val target = model.expectShape(payload.target) if (!target.isBlobShape || target.hasTrait()) { - val expectedRequestContentType = httpBindingResolver.requestContentType(operationShape) - ?.let { "Some(${it.dq()})" } ?: "None" + // `null` is only returned by Smithy when there are no members, but we know there's at least + // the one with `@httpPayload`, so `!!` is safe here. + val expectedRequestContentType = httpBindingResolver.requestContentType(operationShape)!! rustTemplate( """ - if #{SmithyHttpServer}::protocols::content_type_header_classifier(request.headers(), $expectedRequestContentType).is_err() { - return Err(#{RuntimeError}::UnsupportedMediaType) + if #{SmithyHttpServer}::protocols::content_type_header_classifier(request.headers(), Some("$expectedRequestContentType")).is_err() { + return Err(#{RuntimeError}::UnsupportedMediaType); } """, *codegenScope, @@ -616,30 +622,22 @@ class ServerHttpBoundProtocolTraitImplGenerator( rust("let (parts, body) = request.into_parts();") val parser = structuredDataParser.serverInputParser(operationShape) val noInputs = model.expectShape(operationShape.inputShape).expectTrait().originalId == null + if (parser != null) { - rustTemplate( - """ - let bytes = #{Hyper}::body::to_bytes(body).await?; - if !bytes.is_empty() { - """, - *codegenScope, - ) - if (protocol is RestJson) { + // `null` is only returned by Smithy when there are no members, but we know there's at least one, since + // there's something to parse (i.e. `parser != null`), so `!!` is safe here. + val expectedRequestContentType = httpBindingResolver.requestContentType(operationShape)!! + rustTemplate("let bytes = #{Hyper}::body::to_bytes(body).await?;", *codegenScope) + rustBlock("if !bytes.is_empty()") { rustTemplate( """ - #{SmithyHttpServer}::protocols::content_type_header_classifier(&parts.headers, Some("application/json"))?; + #{SmithyHttpServer}::protocols::content_type_header_classifier(&parts.headers, Some("$expectedRequestContentType"))?; + input = #{parser}(bytes.as_ref(), input)?; """, *codegenScope, + "parser" to parser, ) } - rustTemplate( - """ - input = #{parser}(bytes.as_ref(), input)?; - } - """, - *codegenScope, - "parser" to parser, - ) } for (binding in bindings) { val member = binding.member diff --git a/rust-runtime/aws-smithy-http-server/src/proto/rest_json_1/rejection.rs b/rust-runtime/aws-smithy-http-server/src/proto/rest_json_1/rejection.rs index 9c3ffc20f7..848a8b768a 100644 --- a/rust-runtime/aws-smithy-http-server/src/proto/rest_json_1/rejection.rs +++ b/rust-runtime/aws-smithy-http-server/src/proto/rest_json_1/rejection.rs @@ -113,6 +113,8 @@ pub enum RequestRejection { HttpBody(crate::Error), /// Used when checking the `Content-Type` header. + /// This is bubbled up in the generated SDK when calling + /// [`crate::protocols::content_type_header_classifier`] in `from_request`. MissingContentType(MissingContentTypeReason), /// Used when failing to deserialize the HTTP body's bytes into a JSON document conforming to diff --git a/rust-runtime/aws-smithy-http-server/src/proto/rest_json_1/runtime_error.rs b/rust-runtime/aws-smithy-http-server/src/proto/rest_json_1/runtime_error.rs index fdf55e623a..3cf49687ca 100644 --- a/rust-runtime/aws-smithy-http-server/src/proto/rest_json_1/runtime_error.rs +++ b/rust-runtime/aws-smithy-http-server/src/proto/rest_json_1/runtime_error.rs @@ -47,6 +47,9 @@ pub enum RuntimeError { InternalFailure(crate::Error), /// Request contained an `Accept` header with a MIME type, and the server cannot return a response /// body adhering to that MIME type. + /// This is returned directly (i.e. without going through a [`RequestRejection`] first) in the + /// generated SDK when calling [`crate::protocols::accept_header_classifier`] in + /// `from_request`. NotAcceptable, /// The request does not contain the expected `Content-Type` header value. UnsupportedMediaType, From 602a51846daa96f751705d42a7d262644af05a6c Mon Sep 17 00:00:00 2001 From: david-perez Date: Mon, 7 Aug 2023 17:35:18 +0200 Subject: [PATCH 2/3] Double-quote expected content-type --- .../protocols/ServerHttpBoundProtocolGenerator.kt | 10 ++++++++-- .../src/protocol/rest_json_1/rejection.rs | 2 +- .../src/protocol/rest_json_1/runtime_error.rs | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt index af0caf6b47..fa7e9dd3cb 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt @@ -252,7 +252,10 @@ class ServerHttpBoundProtocolTraitImplGenerator( val expectedRequestContentType = httpBindingResolver.requestContentType(operationShape)!! rustTemplate( """ - #{SmithyHttpServer}::protocol::content_type_header_classifier(request.headers(), $expectedRequestContentType)?; + #{SmithyHttpServer}::protocol::content_type_header_classifier( + request.headers(), + Some("$expectedRequestContentType"), + )?; """, *codegenScope, ) @@ -704,7 +707,10 @@ class ServerHttpBoundProtocolTraitImplGenerator( rustBlock("if !bytes.is_empty()") { rustTemplate( """ - #{SmithyHttpServer}::protocol::content_type_header_classifier(&parts.headers, Some("$expectedRequestContentType"))?; + #{SmithyHttpServer}::protocol::content_type_header_classifier( + &parts.headers, + Some("$expectedRequestContentType"), + )?; input = #{parser}(bytes.as_ref(), input)?; """, *codegenScope, diff --git a/rust-runtime/aws-smithy-http-server/src/protocol/rest_json_1/rejection.rs b/rust-runtime/aws-smithy-http-server/src/protocol/rest_json_1/rejection.rs index ea0c1d6aed..8577d4a557 100644 --- a/rust-runtime/aws-smithy-http-server/src/protocol/rest_json_1/rejection.rs +++ b/rust-runtime/aws-smithy-http-server/src/protocol/rest_json_1/rejection.rs @@ -116,7 +116,7 @@ pub enum RequestRejection { /// Used when checking the `Content-Type` header. /// This is bubbled up in the generated SDK when calling - /// [`crate::protocols::content_type_header_classifier`] in `from_request`. + /// [`crate::protocol::content_type_header_classifier`] in `from_request`. #[error("expected `Content-Type` header not found: {0}")] MissingContentType(#[from] MissingContentTypeReason), diff --git a/rust-runtime/aws-smithy-http-server/src/protocol/rest_json_1/runtime_error.rs b/rust-runtime/aws-smithy-http-server/src/protocol/rest_json_1/runtime_error.rs index 1518c9eeeb..9b2a7b21e7 100644 --- a/rust-runtime/aws-smithy-http-server/src/protocol/rest_json_1/runtime_error.rs +++ b/rust-runtime/aws-smithy-http-server/src/protocol/rest_json_1/runtime_error.rs @@ -48,7 +48,7 @@ pub enum RuntimeError { /// Request contained an `Accept` header with a MIME type, and the server cannot return a response /// body adhering to that MIME type. /// This is returned directly (i.e. without going through a [`RequestRejection`] first) in the - /// generated SDK when calling [`crate::protocols::accept_header_classifier`] in + /// generated SDK when calling [`crate::protocol::accept_header_classifier`] in /// `from_request`. NotAcceptable, /// The request does not contain the expected `Content-Type` header value. From 4b4bf312efa270dac08ec972088dacd1db62f6d2 Mon Sep 17 00:00:00 2001 From: david-perez Date: Mon, 7 Aug 2023 17:36:05 +0200 Subject: [PATCH 3/3] ./gradlew ktlintFormat --- .../server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt index fa7e9dd3cb..c579d64415 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt @@ -57,7 +57,6 @@ import software.amazon.smithy.rust.codegen.core.smithy.protocols.HttpBoundProtoc import software.amazon.smithy.rust.codegen.core.smithy.protocols.HttpLocation import software.amazon.smithy.rust.codegen.core.smithy.protocols.Protocol import software.amazon.smithy.rust.codegen.core.smithy.protocols.ProtocolFunctions -import software.amazon.smithy.rust.codegen.core.smithy.protocols.RestJson import software.amazon.smithy.rust.codegen.core.smithy.protocols.parse.StructuredDataParserGenerator import software.amazon.smithy.rust.codegen.core.smithy.traits.SyntheticInputTrait import software.amazon.smithy.rust.codegen.core.smithy.transformers.operationErrors