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

Should clients reject responses with sets with duplicate elements? #1266

Closed
david-perez opened this issue Jun 10, 2022 · 5 comments
Closed

Comments

@david-perez
Copy link
Contributor

If a client receives a response, that, upon deserialization, results in data for a set shape that contains duplicate values, should a client reject it?

Servers do reject requests:

https://github.com/awslabs/smithy/blob/c8ce65a2bcc0264f3aeeb9db096564e0491a8786/smithy-aws-protocol-tests/model/restJson1/malformedRequests/malformed-set.smithy#L15-L36

@adamthom-amzn
Copy link
Contributor

No. Clients rejecting responses is lossy (it prevents you from knowing about state changes that actually happened).

@david-perez
Copy link
Contributor Author

david-perez commented Jun 10, 2022

Under what circumstances should clients reject responses? Only upon deserialization errors (like malformed JSON)? I'm thinking if having httpMalformedResponseTests would be a useful feature.

@adamthom-amzn
Copy link
Contributor

Yes, in general a client should do everything in its power to render a response from the server. Malformed JSON (or a non-JSON response) are the only cases I can think of where you should reject a response.

@mtdowling
Copy link
Member

We deprecated sets and treating them exactly like @uniqueItems. They'll be removed in IDL v2. In that sense, it's a constraint trait, clients shouldn't evaluate constraint traits (for the most part), so clients shouldn't care if there are duplicates in responses.

@david-perez
Copy link
Contributor Author

Malformed JSON (or a non-JSON response) are the only cases I can think of where you should reject a response.

I think there's another category of warranted response rejections by the client: number conversions. Technically not malformed JSON. But it's best for the client to reject a number that doesn't fit in the Smithy modeled type than to attempt an e.g. double to single-precision float conversion, or a floating point into an integral type conversion, when the receiver type cannot fit the value. Not rejecting could lead to very hard to identify logic bugs.

I think an httpMalformedResponseTests suite would thus be useful just for this case.

I identified this category of errors when trying to pass httpMalformedRequestTests in the server, but also made the change for clients in smithy-rs in smithy-lang/smithy-rs#1274.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants