Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@required in document shapes violated over the wire #3735

Open
david-perez opened this issue Jul 3, 2024 · 0 comments
Open

@required in document shapes violated over the wire #3735

david-perez opened this issue Jul 3, 2024 · 0 comments
Labels
breaking-change This will require a breaking change bug Something isn't working client server Rust server SDK

Comments

@david-perez
Copy link
Contributor

david-perez commented Jul 3, 2024

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 david-perez added bug Something isn't working server Rust server SDK client labels Jul 3, 2024
@drganjoo drganjoo added the breaking-change This will require a breaking change label Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This will require a breaking change bug Something isn't working client server Rust server SDK
Projects
None yet
Development

No branches or pull requests

2 participants