smithy-rs icon indicating copy to clipboard operation
smithy-rs copied to clipboard

Servers incorrectly accept any request body when the modeled operation input is empty or non-existent

Open david-perez opened this issue 7 months ago • 0 comments

When there is operation input:

$version: "2.0"

namespace com.amazonaws.simple

use aws.protocols#restJson1
use smithy.test#httpMalformedRequestTests

@restJson1
service SimpleService {
    operations: [
        Operation
    ]
}

@http(uri: "/operation", method: "POST")
operation Operation {
    input: OperationInputOutput
    output: OperationInputOutput
}

structure OperationInputOutput {
    message: String
}

servers correctly reject a request that contains additional tokens past where we expect the end of the body:

apply Operation @httpMalformedRequestTests([
    {
        id: "AdditionalTokensEmptyStruct"
        documentation: """
        When additional tokens are found past where we expect the end of the body,
        the request should be rejected with a serialization exception."""
        protocol: restJson1
        request: {
            method: "POST"
            uri: "/operation"
            headers: {
                "Content-Type": "application/json"
            }
            body: "{}{}"
        },
        response: {
            headers: {
                "x-amzn-errortype": "SerializationException"
            }
            code: 400,
        }
    }
])

That is because JsonParserGenerator.kt asserts that there are no leftover tokens after parsing the operation input:

https://github.com/smithy-lang/smithy-rs/blob/b74887861280ec475db984f9a7603eebb4d959b9/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/parse/JsonParserGenerator.kt#L156-L156

https://github.com/smithy-lang/smithy-rs/blob/b74887861280ec475db984f9a7603eebb4d959b9/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/parse/JsonParserGenerator.kt#L243-L250

However, when the modeled operation input is empty:

structure OperationInputOutput { }

Or when there is no operation input (removing the Content-Type header in this case):

@http(uri: "/operation", method: "POST")
operation Operation { }

We don't generate a deserializer for the operation, and so we don't even attempt to inspect the body, accepting anything:

thread 'operation::operation_test::additional_tokens_empty_struct_malformed_request' panicked at simple/rust-server-codegen/src/operation.rs:112:52:
request should have been rejected, but we accepted it; we parsed operation input `OperationInput`

This bug affects all protocols. Note that in some protocols, like restJson1 and rpcv2Cbor (and perhaps all?), servers must accept both no request body and empty object in request body when the modeled operation input is empty. However, they shouldn't accept anything.

david-perez avatar Jun 27 '24 10:06 david-perez