smithy-lang / smithy

Smithy is a protocol-agnostic interface definition language and set of tools for generating clients, servers, and documentation for any programming language.
https://smithy.io
Apache License 2.0
1.71k stars 201 forks source link

`@required` in `document` shapes violated over the wire #2343

Closed david-perez closed 5 days ago

david-perez commented 5 days ago

This bug affects both clients and servers. Consider:

$version: "2.0"

namespace com.amazonaws.simple

use aws.protocols#restJson1
use smithy.test#httpRequestTests
use smithy.test#httpResponseTests

@restJson1
service SimpleService {
    operations: [
        Operation
    ]
}

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

structure OperationInputOutput {
    @required
    document: Document
}

Since the document member shape is @required, these two JSON payloads should not be valid over the wire:

{ document: null }
{ }

In both, a value is not provided for the required member shape. Hence:

  1. a client should reject both responses
  2. a client should not serialize any of the two when sending a request
  3. a server should reject both requests
  4. a server should not serialize any of the two when sending a response

However, all 4 are currently possible with smithy-rs. This is because, regardless if we generate aws_smithy_types::Document or Option<aws_smithy_types::Document> as the Rust type for the member shape (as per the structure member optionality rules), the aws_smithy_types::Document::Null variant always exists (it's a runtime type crate), so a user can always set it for the member shape, e.g.:

let op_input = OperationInput::builder()
    .document(aws_smithy_types::Document::Null)
    .build();

Indeed, this protocol test passes just fine:

apply Operation @httpRequestTests([
    {
        id: "SendNullDocument",
        protocol: restJson1,
        method: "POST",
        uri: "/operation",
        body: "{ \"document\": null }",
        bodyMediaType: "application/json",
        params: {
            document: null,
        },
    }
])

Likewise, our deserializers deserialize both JSON payloads by setting aws_smithy_types::Document::Null.

apply Operation @httpResponseTests([
    {
        id: "ReceiveNullDocument1",
        protocol: restJson1,
        code: 200,
        body: "{ \"document\": null}",
        params: {
            document: null,
        },
    }
])

apply Operation @httpResponseTests([
    {
        id: "ReceiveNullDocument2",
        protocol: restJson1,
        code: 200,
        body: "{ }",
        params: {
            document: null,
        },
    }
])
david-perez commented 5 days ago

Wrong repo sorry, I meant to open this in https://github.com/smithy-lang/smithy-rs/.