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

Server behavior in AWS JSON 1.x protocol with empty input #2327

Open david-perez opened 2 weeks ago

david-perez commented 2 weeks ago

Consider:

operation NoInputAndNoOutput { }

operation EmptyInputAndEmptyOutput {
    input: EmptyInputOutput
    output: EmptyInputOutput
}

structure EmptyInputOutput { }

In the AWS JSON 1.x protocols, there's tests that assert that services must accept both an empty HTTP request body and a request body with an empty JSON document {} when there is no input.

However, what should the behavior be when there is empty input? Note that:

  1. In AWS JSON 1.0 and AWS JSON 1.1, clients must always send an empty JSON document {}. If this is really a MUST, it would make sense to have services reject an empty HTTP request body for an empty input operation.
  2. However, restJson1 and rpcv2Cbor are more lenient and allow both empty request bodies and empty JSON/CBOR objects.

It's important to pin down this behavior because the protocol tests currently don't prescribe one; (1) would indicate that servers should be strict, but (2), and in general Postel's law, would lead us to not make the AWS JSON 1.x protocols less lenient than the other ones.

As a data point, currently AWS JSON 1.x smithy-rs servers are lenient.

JordonPhillips commented 1 week ago

The service behavior in each of these cases is consistent, they should be lenient in accepting either an empty body or an empty-object body.

The client behavior is different because there are actual implementation differences in the protocols. In the AWS JSON protocols, there is a difference in behavior when input is not modeled (i.e. unit in smithy) and when input is modeled, but has no required members. In your example, NoInputAndNoOutput is behaviorally different in those protocols than EmptyInputAndEmptyOutput. If you send an empty body to an aws json service using the standard implementation for EmptyInputAndEmptyOutput, the request will fail. If, however, you send an empty object to NoInputAndNoOutput, that will be happily accepted. Thus, clients must send an empty-object body in both cases to ensure forwards compatibility.

This is behavior of actual services. It doesn't respect Postel's law. It's bad and I don't like it. But it is. There's not really anything we can do but make sure our clients behave in the most compatible way and make sure that any new service frameworks are lenient as we would expect. That's why the protocol tests are the way they are.

Now for restJson1, there actually can be a meaningful difference between empty body and empty-object body since the body is only a part of the request rather than the entire request. It could theoretically be some nullable shape (in some cases, as you've pointed out before, it's difficult to tell).

In the cbor case, we want to be restrictive in what we're providing so we might as well do the same thing as we're doing with the other rpc protocols. I will admit though that I have far less experience with that protocol.

david-perez commented 1 week ago

If you send an empty body to an aws json service using the standard implementation for EmptyInputAndEmptyOutput, the request will fail.

So then the answer to this issue is (1), services MUST reject an empty HTTP request body for an empty input operation. Can you confirm? We need protocol tests to pin this behavior.

Thus, clients must send an empty-object body in both cases to ensure forwards compatibility.

I don't understand this argument. Transitioning from empty input to no input or viceversa is a breaking change, so why is there a forwards-compatibility concern?

JordonPhillips commented 1 week ago

So then the answer to this issue is (1), services MUST reject an empty HTTP request body for an empty input operation. Can you confirm?

No, this is not behavior we want to carry forward in new implementations. There are a very small number of cases like this elsewhere iirc.

I don't understand this argument. Transitioning from empty input to no input or viceversa is a breaking change, so why is there a forwards-compatibility concern?

Services do make breaking changes. Always sending an empty body minimizes the impact of this particular case significantly, at essentially no cost.