Unverified Commit f2abe7c6 authored by david-perez's avatar david-perez Committed by GitHub
Browse files

Improve invalid HTTP status code responses failure mode handling (#2522) 

If the model only uses the `@http` trait to define the response code,
Smithy already checks that this is a valid status code, so the
conversion cannot fail, and we can avoid rejecting with
`ResponseRejection::InvalidHttpStatusCode`.

This commit also removes this variant from RPC protocols, where HTTP
binding traits are not allowed, and so this failure mode can never
happen.
parent c901c0b1

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt

+24 −27
Original line number Diff line number Diff line
@@ -439,17 +439,18 @@ class ServerHttpBoundProtocolTraitImplGenerator( 
        Attribute.AllowUnusedMut.render(this)

        rustTemplate("let mut builder = #{http}::Response::builder();", *codegenScope)

        serverRenderResponseHeaders(operationShape)

        // Fallback to the default code of `@http`, 200.

        // Fallback to the default code of `@http`, which should be 200.

        val httpTraitDefaultStatusCode = HttpTrait

            .builder().method("GET").uri(UriPattern.parse("/")) /* Required to build */

            .build()

            .code

        check(httpTraitDefaultStatusCode == 200)

        val httpTraitStatusCode = operationShape.getTrait<HttpTrait>()?.code ?: httpTraitDefaultStatusCode

        bindings.find { it.location == HttpLocation.RESPONSE_CODE }

            ?.let {

                serverRenderResponseCodeBinding(it, httpTraitStatusCode)(this)

            }

            // no binding, use http's

            // No binding, use `@http`.

            ?: serverRenderHttpResponseCode(httpTraitStatusCode)(this)



        operationShape.outputShape(model).findStreamingMember(model)?.let {
@@ -555,46 +556,42 @@ class ServerHttpBoundProtocolTraitImplGenerator( 
        )

    }



    private fun serverRenderHttpResponseCode(

        defaultCode: Int,

    ): Writable {

        return writable {

    private fun serverRenderHttpResponseCode(defaultCode: Int) = writable {

        check(defaultCode in 100..999) {

            """

           Smithy library lied to us. According to https://smithy.io/2.0/spec/http-bindings.html#http-trait,

           "The provided value SHOULD be between 100 and 599, and it MUST be between 100 and 999".

           """.replace("\n", "").trimIndent()

        }

        rustTemplate(

            """

                let status = $defaultCode;

                let http_status: u16 = status.try_into()

                    .map_err(|_| #{ResponseRejection}::InvalidHttpStatusCode)?;

            let http_status: u16 = $defaultCode;

            builder = builder.status(http_status);

                """.trimIndent(),

            """,

            *codegenScope,

        )

    }

    }



    private fun serverRenderResponseCodeBinding(

        binding: HttpBindingDescriptor,

        /** This is the status code to fall back on if the member shape bound with `@httpResponseCode` is not

         * `@required` and the user did not provide a value for it at runtime. **/

        fallbackStatusCode: Int,

    ): Writable {

        check(binding.location == HttpLocation.RESPONSE_CODE)



        return writable {

            val memberName = symbolProvider.toMemberName(binding.member)

            rust("let status = output.$memberName")

            withBlock("let status = output.$memberName", ";") {

                if (symbolProvider.toSymbol(binding.member).isOptional()) {

                rustTemplate(

                    """

                    .unwrap_or($fallbackStatusCode)

                    """.trimIndent(),

                    *codegenScope,

                )

                    rust(".unwrap_or($fallbackStatusCode)")

                }

            }

            rustTemplate(

                """

                ;

                let http_status: u16 = status.try_into()

                    .map_err(|_| #{ResponseRejection}::InvalidHttpStatusCode)?;

                let http_status: u16 = status.try_into().map_err(#{ResponseRejection}::InvalidHttpStatusCode)?;

                builder = builder.status(http_status);

                """.trimIndent(),

                """,

                *codegenScope,

            )

        }

rust-runtime/aws-smithy-http-server/src/proto/aws_json/rejection.rs

+0 −1
Original line number Diff line number Diff line
@@ -9,7 +9,6 @@ use crate::rejection::MissingContentTypeReason; 


#[derive(Debug, Display)]

pub enum ResponseRejection {

    InvalidHttpStatusCode,

    Serialization(crate::Error),

    Http(crate::Error),

}

rust-runtime/aws-smithy-http-server/src/proto/rest_json_1/rejection.rs

+3 −1
Original line number Diff line number Diff line
@@ -47,6 +47,8 @@ 
//!

//! Consult `crate::proto::$protocolName::rejection` for rejection types for other protocols.



use std::num::TryFromIntError;



use strum_macros::Display;



use crate::rejection::MissingContentTypeReason;
@@ -58,7 +60,7 @@ pub enum ResponseRejection { 
    /// Used when the service implementer provides an integer outside the 100-999 range for a

    /// member targeted by `httpResponseCode`.

    /// See <https://github.com/awslabs/smithy/issues/1116>.

    InvalidHttpStatusCode,

    InvalidHttpStatusCode(TryFromIntError),



    /// Used when an invalid HTTP header value (a value that cannot be parsed as an

    /// `[http::header::HeaderValue]`) is provided for a shape member bound to an HTTP header with

rust-runtime/aws-smithy-http-server/src/proto/rest_xml/rejection.rs

+3 −1
Original line number Diff line number Diff line
@@ -7,11 +7,13 @@ 
//! [`crate::proto::rest_json_1::rejection::RequestRejection::JsonDeserialize`] is swapped for

//! [`RequestRejection::XmlDeserialize`].



use std::num::TryFromIntError;



use strum_macros::Display;



#[derive(Debug, Display)]

pub enum ResponseRejection {

    InvalidHttpStatusCode,

    InvalidHttpStatusCode(TryFromIntError),

    Build(crate::Error),

    Serialization(crate::Error),

    Http(crate::Error),