Unverified Commit d770c038 authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

Fix JSON parsing bug for modeled empty structs (#683)

* Fix JSON parsing bug for modeled empty structs

* Update changelog

* CR feedback
parent a61d0008
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -162,6 +162,7 @@ impl ProvideCredentials for CustomCreds {
- Update AWS SDK models (#677)
- :bug: Fix sigv4 signing when request ALPN negotiates to HTTP/2. (#674)
- :bug: Fix integer size on S3 `Size` (#679, aws-sdk-rust#209)
- :bug: Fix JSON parsing issue for modeled empty structs (#683, aws-sdk-rust#212)
- :bug: Fix acronym case disagreement between FluentClientGenerator and HttpProtocolGenerator type aliasing (#668)

**Internal Changes**
+21 −1
Original line number Diff line number Diff line
@@ -62,7 +62,8 @@ service RestJsonExtras {
        PrimitiveIntOp,
        EscapedStringValues,
        NullInNonSparse,
        CaseInsensitiveErrorOperation
        CaseInsensitiveErrorOperation,
        EmptyStructWithContentOnWireOp,
    ]
}

@@ -289,3 +290,22 @@ operation CaseInsensitiveErrorOperation {
structure CaseInsensitiveError {
    message: String
}

structure EmptyStruct {}
structure EmptyStructWithContentOnWireOpOutput {
    empty: EmptyStruct,
}

@http(uri: "/empty-struct-with-content-on-wire-op", method: "GET")
@httpResponseTests([
    {
        id: "EmptyStructWithContentOnWire",
        protocol: "aws.protocols#restJson1",
        code: 200,
        body: "{\"empty\": {\"value\":\"not actually empty\"}}",
        params: { empty: {} }
    }
])
operation EmptyStructWithContentOnWireOp {
    output: EmptyStructWithContentOnWireOpOutput,
}
+20 −18
Original line number Diff line number Diff line
@@ -72,6 +72,7 @@ class JsonParserGenerator(
        "json_token_iter" to smithyJson.member("deserialize::json_token_iter"),
        "Peekable" to RuntimeType.std.member("iter::Peekable"),
        "skip_value" to smithyJson.member("deserialize::token::skip_value"),
        "skip_to_end" to smithyJson.member("deserialize::token::skip_to_end"),
        "Token" to smithyJson.member("deserialize::Token"),
        "or_empty" to orEmptyJson()
    )
@@ -204,10 +205,7 @@ class JsonParserGenerator(
    }

    private fun RustWriter.deserializeStructInner(members: Collection<MemberShape>) {
        if (members.isEmpty()) {
            return
        }
        objectKeyLoop {
        objectKeyLoop(hasMembers = members.isNotEmpty()) {
            rustBlock("match key.to_unescaped()?.as_ref()") {
                for (member in members) {
                    rustBlock("${member.wireName().dq()} =>") {
@@ -334,7 +332,7 @@ class JsonParserGenerator(
            ) {
                startObjectOrNull {
                    rust("let mut map = #T::new();", RustType.HashMap.RuntimeType)
                    objectKeyLoop {
                    objectKeyLoop(hasMembers = true) {
                        withBlock("let key =", "?;") {
                            deserializeStringInner(keyTarget, "key")
                        }
@@ -408,7 +406,7 @@ class JsonParserGenerator(
                        """,
                        *codegenScope
                    ) {
                        objectKeyLoop {
                        objectKeyLoop(hasMembers = shape.members().isNotEmpty()) {
                            rustTemplate(
                                """
                                if variant.is_some() {
@@ -454,7 +452,10 @@ class JsonParserGenerator(
        }
    }

    private fun RustWriter.objectKeyLoop(inner: RustWriter.() -> Unit) {
    private fun RustWriter.objectKeyLoop(hasMembers: Boolean, inner: RustWriter.() -> Unit) {
        if (!hasMembers) {
            rustTemplate("#{skip_to_end}(tokens)?;", *codegenScope)
        } else {
            rustBlock("loop") {
                rustBlock("match tokens.next().transpose()?") {
                    rustBlockTemplate(
@@ -473,6 +474,7 @@ class JsonParserGenerator(
                }
            }
        }
    }

    private fun RustWriter.startArrayOrNull(inner: RustWriter.() -> Unit) = startOrNull("array", inner)
    private fun RustWriter.startObjectOrNull(inner: RustWriter.() -> Unit) = startOrNull("object", inner)
+8 −2
Original line number Diff line number Diff line
@@ -72,13 +72,17 @@ class JsonParserGeneratorTest {
            member: Choice
        }

        structure EmptyStruct {
        }

        structure Top {
            @required
            choice: Choice,
            field: String,
            extra: Integer,
            @jsonName("rec")
            recursive: TopList
            recursive: TopList,
            empty: EmptyStruct,
        }

        list TopList {
@@ -133,7 +137,8 @@ class JsonParserGeneratorTest {
                        { "extra": 45,
                          "field": "something",
                          "choice":
                              { "int": 5 }}}
                              { "int": 5 },
                          "empty": { "not_empty": true }}}
                "#;

                let output = ${writer.format(operationGenerator!!)}(json, output::op_output::Builder::default()).unwrap().build();
@@ -157,6 +162,7 @@ class JsonParserGeneratorTest {
        }
        project.withModule(RustModule.default("model", public = true)) {
            model.lookup<StructureShape>("test#Top").renderWithModelBuilder(model, symbolProvider, it)
            model.lookup<StructureShape>("test#EmptyStruct").renderWithModelBuilder(model, symbolProvider, it)
            UnionGenerator(model, symbolProvider, it, model.lookup("test#Choice")).render()
            val enum = model.lookup<StringShape>("test#FooEnum")
            EnumGenerator(model, symbolProvider, it, enum, enum.expectTrait()).render()
+28 −6
Original line number Diff line number Diff line
@@ -296,30 +296,38 @@ where
pub fn skip_value<'a>(
    tokens: &mut impl Iterator<Item = Result<Token<'a>, Error>>,
) -> Result<(), Error> {
    skip_inner(false, tokens)
    skip_inner(0, tokens)
}

/// Assumes a start object/array token has already been consumed and skips tokens until
/// until its corresponding end object/array token is found.
pub fn skip_to_end<'a>(
    tokens: &mut impl Iterator<Item = Result<Token<'a>, Error>>,
) -> Result<(), Error> {
    skip_inner(1, tokens)
}

fn skip_inner<'a>(
    inside_obj_or_array: bool,
    depth: isize,
    tokens: &mut impl Iterator<Item = Result<Token<'a>, Error>>,
) -> Result<(), Error> {
    loop {
        match tokens.next().transpose()? {
            Some(Token::StartObject { .. }) | Some(Token::StartArray { .. }) => {
                skip_inner(true, tokens)?;
                if !inside_obj_or_array {
                skip_inner(depth + 1, tokens)?;
                if depth == 0 {
                    break;
                }
            }
            Some(Token::EndObject { .. }) | Some(Token::EndArray { .. }) => {
                debug_assert!(inside_obj_or_array);
                debug_assert!(depth > 0);
                break;
            }
            Some(Token::ValueNull { .. })
            | Some(Token::ValueBool { .. })
            | Some(Token::ValueNumber { .. })
            | Some(Token::ValueString { .. }) => {
                if !inside_obj_or_array {
                if depth == 0 {
                    break;
                }
            }
@@ -424,6 +432,20 @@ pub mod test {
        ))
    }

    #[test]
    fn test_skip_to_end() {
        let tokens = json_token_iter(b"{\"one\": { \"two\": [] }, \"three\":2 }");
        let mut tokens = tokens.skip(2);
        assert!(matches!(tokens.next(), Some(Ok(Token::StartObject { .. }))));
        skip_to_end(&mut tokens).unwrap();
        match tokens.next() {
            Some(Ok(Token::ObjectKey { key, .. })) => {
                assert_eq!("three", key.as_escaped_str());
            }
            _ => panic!("expected object key three"),
        }
    }

    #[test]
    fn test_non_finite_floats() {
        let mut tokens = json_token_iter(b"inf");