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

Body media type in HTTP malformed request tests #1932

Closed
david-perez opened this issue Aug 18, 2023 · 1 comment · Fixed by #2309
Closed

Body media type in HTTP malformed request tests #1932

david-perez opened this issue Aug 18, 2023 · 1 comment · Fixed by #2309
Labels
feature-request A feature should be added or improved. protocol-test New protocol tests are needed

Comments

@david-perez
Copy link
Contributor

Both httpRequestTests and httpResponseTests have a bodyMediaType field. This field allows test generators, among other things, to know how the body field is encoded.

Because the body parameter is a string, binary data MUST be represented in body by base64 encoding the data (for example, use "Zm9vCg==" and not "foo")

However, HttpMalformedRequestDefinition does not have a bodyMediaType field. So far in smithy-rs we have sent these malformed requests by sending the body contents as-is, and this has worked well because all protocols that Smithy currently supports use text-based serialization formats. However, with the advent of Smithy RPC v2 CBOR, this is no longer case, and so we need a way to tell the test generator whether to base64-decode body or not before sending the request: the bodyMediaType field would fulfill this need.

Can we add bodyMediaType to HttpMalformedRequestDefinition?

@mtdowling
Copy link
Member

Sounds good to me.

@mtdowling mtdowling added feature-request A feature should be added or improved. protocol-test New protocol tests are needed labels Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. protocol-test New protocol tests are needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants