search

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.77k stars 208 forks source link

Servers must reject `{}` when deserializing a union #2300

Closed david-perez closed 4 months ago

david-perez commented 4 months ago

Once we're inside of a {} JSON object, a known union variant must be provided. Servers must not accept {} as "the union shape was not set" when the union shape is optional (which was the behavior in smithy-rs prior to https://github.com/smithy-lang/smithy-rs/pull/3481).

This commit adds a protocol test to pin this behavior.

Testing

I verified that the added test fails in smithy-rs when checking out a commit prior to https://github.com/smithy-lang/smithy-rs/pull/3481

thread 'operation::server_union_with_empty_body_operation_test::rest_json_with_empty_json_object_for_union_variant_malformed_request' panicked at 'request should have been rejected, but we accepted it; we parsed operation input `UnionWithEmptyBodyOperationInput { member: None }`', rest_json_extras/rust-server-codegen/src/operation.rs:774:52

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

david-perez commented 4 months ago

This applies to all JSON-based protocols. But it seems like the repo only has @httpMalformedRequestTests for restJson1.

david-perez commented 4 months ago

Note that clients should reject {} too when deserializing a union in a response (instead of setting an "unknown union variant"). But there's no @httpMalformedResponseTests to pin this behavior :(