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

Refactor converters to numeric types for `aws_smithy_types::Number` (#1274) 

Currently, conversions from `aws_smithy_types::Number` into numeric Rust
types (`{i,u}{8, 16, 32, 64}` and `f{32, 64}`) are always lossy, because
they use the `as` Rust keyword to cast into the target type. This means
that clients and servers are accepting lossy data: for example, if an
operation is modeled to take in a 32-bit integer as input, and a client
incorrectly sends an integer number that does not fit in 32 bits, the
server will silently accept the truncated input. There are malformed
request protocol tests that verify that servers must reject these
requests.

This commit removes the lossy `to_*` methods on `Number` and instead
implements `TryFrom<$typ> for Number` for the target numeric type
`$typ`. These converters will attempt their best to perform the
conversion safely, and fail if it is lossy.

The code-generated JSON parsers will now fail with
`aws_smithy_json::deserialize::ErrorReason::InvalidNumber` if the number
in the JSON document cannot be converted into the modeled integer type
without losing precision. For floating point target types, lossy
conversions are still performed, via `Number::to_f32_lossy` and
`Number::to_f64_lossy`.
parent 9ee45bfd

CHANGELOG.next.toml

+60 −0
Original line number Diff line number Diff line
@@ -117,3 +117,63 @@ There is a canonical and easier way to run smithy-rs on Lambda [see example]. 
references = ["smithy-rs#1551"]

meta = { "breaking" = false, "tada" = true, "bug" = false, "target" = "server" }

author = "hugobast"



[[smithy-rs]]

message = """

Lossy converters into integer types for `aws_smithy_types::Number` have been

removed. Lossy converters into floating point types for

`aws_smithy_types::Number` have been suffixed with `_lossy`. If you were

directly using the integer lossy converters, we recommend you use the safe

converters.

_Before:_

```rust

fn f1(n: aws_smithy_types::Number) {

    let foo: f32 = n.to_f32(); // Lossy conversion!

    let bar: u32 = n.to_u32(); // Lossy conversion!

}

```

_After:_

```rust

fn f1(n: aws_smithy_types::Number) {

    use std::convert::TryInto; // Unnecessary import if you're using Rust 2021 edition.

    let foo: f32 = n.try_into().expect("lossy conversion detected"); // Or handle the error instead of panicking.

    // You can still do lossy conversions, but only into floating point types.

    let foo: f32 = n.to_f32_lossy();

    // To lossily convert into integer types, use an `as` cast directly.

    let bar: u32 = n as u32; // Lossy conversion!

}

```

"""

references = ["smithy-rs#1274"]

meta = { "breaking" = true, "tada" = false, "bug" = true, "target" = "all" }

author = "david-perez"



[[aws-sdk-rust]]

message = """

Lossy converters into integer types for `aws_smithy_types::Number` have been

removed. Lossy converters into floating point types for

`aws_smithy_types::Number` have been suffixed with `_lossy`. If you were

directly using the integer lossy converters, we recommend you use the safe

converters.

_Before:_

```rust

fn f1(n: aws_smithy_types::Number) {

    let foo: f32 = n.to_f32(); // Lossy conversion!

    let bar: u32 = n.to_u32(); // Lossy conversion!

}

```

_After:_

```rust

fn f1(n: aws_smithy_types::Number) {

    use std::convert::TryInto; // Unnecessary import if you're using Rust 2021 edition.

    let foo: f32 = n.try_into().expect("lossy conversion detected"); // Or handle the error instead of panicking.

    // You can still do lossy conversions, but only into floating point types.

    let foo: f32 = n.to_f32_lossy();

    // To lossily convert into integer types, use an `as` cast directly.

    let bar: u32 = n as u32; // Lossy conversion!

}

```

"""

references = ["smithy-rs#1274"]

meta = { "breaking" = true, "tada" = false, "bug" = true }

author = "david-perez"

aws/rust-runtime/aws-config/src/credential_process.rs

+6 −1
Original line number Diff line number Diff line
@@ -194,7 +194,12 @@ pub(crate) fn parse_credential_process_json_credentials( 
             "Expiration": "2022-05-02T18:36:00+00:00"

            */

            (key, Token::ValueNumber { value, .. }) if key.eq_ignore_ascii_case("Version") => {

                version = Some(value.to_i32())

                version = Some(i32::try_from(*value).map_err(|err| {

                    InvalidJsonCredentials::InvalidField {

                        field: "Version",

                        err: err.into(),

                    }

                })?);

            }

            (key, Token::ValueString { value, .. }) if key.eq_ignore_ascii_case("AccessKeyId") => {

                access_key_id = Some(value.to_unescaped()?)

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt

+0 −26
Original line number Diff line number Diff line
@@ -661,40 +661,14 @@ class ServerProtocolTestGenerator( 
            FailingTest(RestJson, "RestJsonWithPayloadExpectsImpliedAccept", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyMalformedBlobInvalidBase64_case1", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyMalformedBlobInvalidBase64_case2", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyByteMalformedValueRejected_case2", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyByteMalformedValueRejected_case6", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyByteMalformedValueRejected_case8", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyByteMalformedValueRejected_case10", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyByteUnderflowOverflow_case0", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyByteUnderflowOverflow_case1", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyByteUnderflowOverflow_case2", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyByteUnderflowOverflow_case3", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonWithBodyExpectsApplicationJsonContentType", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonWithPayloadExpectsImpliedContentType", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonWithPayloadExpectsModeledContentType", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonWithoutBodyExpectsEmptyContentType", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyIntegerMalformedValueRejected_case2", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyIntegerMalformedValueRejected_case6", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyIntegerMalformedValueRejected_case8", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyIntegerMalformedValueRejected_case10", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyIntegerUnderflowOverflow_case0", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyIntegerUnderflowOverflow_case1", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyMalformedListNullItem", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyMalformedMapNullValue", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyLongMalformedValueRejected_case2", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyLongMalformedValueRejected_case6", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyLongMalformedValueRejected_case8", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyLongMalformedValueRejected_case10", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonMalformedSetDuplicateItems", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonMalformedSetNullItem", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyShortMalformedValueRejected_case2", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyShortMalformedValueRejected_case6", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyShortMalformedValueRejected_case8", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyShortMalformedValueRejected_case10", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyShortUnderflowOverflow_case0", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyShortUnderflowOverflow_case1", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyShortUnderflowOverflow_case2", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyShortUnderflowOverflow_case3", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonHeaderMalformedStringInvalidBase64MediaType_case1", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyTimestampDefaultRejectsMalformedEpochSeconds_case5", TestType.MalformedRequest),

            FailingTest(RestJson, "RestJsonBodyTimestampDefaultRejectsMalformedEpochSeconds_case7", TestType.MalformedRequest),

codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/JsonParserGenerator.kt

+14 −2
Original line number Diff line number Diff line
@@ -278,8 +278,20 @@ class JsonParserGenerator( 
    }



    private fun RustWriter.deserializeNumber(target: NumberShape) {

        val symbol = symbolProvider.toSymbol(target)

        rustTemplate("#{expect_number_or_null}(tokens.next())?.map(|v| v.to_#{T}())", "T" to symbol, *codegenScope)

        if (target.isFloatShape) {

            rustTemplate("#{expect_number_or_null}(tokens.next())?.map(|v| v.to_f32_lossy())", *codegenScope)

        } else if (target.isDoubleShape) {

            rustTemplate("#{expect_number_or_null}(tokens.next())?.map(|v| v.to_f64_lossy())", *codegenScope)

        } else {

            rustTemplate(

                """

                #{expect_number_or_null}(tokens.next())?

                    .map(|v| v.try_into())

                    .transpose()?

                """,

                *codegenScope,

            )

        }

    }



    private fun RustWriter.deserializeTimestamp(shape: TimestampShape, member: MemberShape) {

rust-runtime/aws-smithy-json/src/deserialize.rs

+2 −2
Original line number Diff line number Diff line
@@ -282,7 +282,7 @@ impl<'a> JsonTokenIterator<'a> { 
    /// returns `(start_index, end_index, negative, floating)`, with `start_index`

    /// and `end_index` representing the slice of the stream that is the number,

    /// `negative` whether or not it is a negative number, and `floating` whether or not

    /// it is a floating point number.

    /// the number contains a decimal point and/or an exponent.

    fn scan_number(&mut self) -> (usize, usize, bool, bool) {

        let start_index = self.index;

        let negative = if self.peek_byte() == Some(b'-') {
@@ -338,7 +338,7 @@ impl<'a> JsonTokenIterator<'a> { 
                if negative > 0 {

                    Number::Float(-(positive as f64))

                } else {

                    Number::NegInt(negative as i64)

                    Number::NegInt(negative)

                }

            } else {

                Number::PosInt(