From fa54d8b6b5ab58b71625a4c2ec12a0dd3cf6d89f Mon Sep 17 00:00:00 2001 From: david-perez Date: Tue, 8 Aug 2023 15:52:13 +0200 Subject: [PATCH] Check `Content-Type` header in all server protocols (#2531) ## Checklist - [ ] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .../ServerHttpBoundProtocolGenerator.kt | 45 ++++++++++--------- .../src/protocol/rest_json_1/rejection.rs | 2 + .../src/protocol/rest_json_1/runtime_error.rs | 3 ++ 3 files changed, 29 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 6e22828a1c..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 @@ -234,6 +233,11 @@ class ServerHttpBoundProtocolTraitImplGenerator( rustTemplate(init, *codegenScope) } } + // 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) @@ -242,11 +246,15 @@ 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( """ - #{SmithyHttpServer}::protocol::content_type_header_classifier(request.headers(), $expectedRequestContentType)?; + #{SmithyHttpServer}::protocol::content_type_header_classifier( + request.headers(), + Some("$expectedRequestContentType"), + )?; """, *codegenScope, ) @@ -689,30 +697,25 @@ 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}::protocol::content_type_header_classifier(&parts.headers, Some("application/json"))?; + #{SmithyHttpServer}::protocol::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/protocol/rest_json_1/rejection.rs b/rust-runtime/aws-smithy-http-server/src/protocol/rest_json_1/rejection.rs index e931f31052..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 @@ -115,6 +115,8 @@ pub enum RequestRejection { NotAcceptable, /// Used when checking the `Content-Type` header. + /// This is bubbled up in the generated SDK when calling + /// [`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 5faf3ea8cf..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 @@ -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::protocol::accept_header_classifier`] in + /// `from_request`. NotAcceptable, /// The request does not contain the expected `Content-Type` header value. UnsupportedMediaType,