-
Notifications
You must be signed in to change notification settings - Fork 195
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
Fix request Content-Type
header checking in servers
#3690
Changes from all commits
a6f39fb
e912cf5
9f05a93
95a58b6
3d108fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
$version: "1.0" | ||
|
||
namespace aws.protocoltests.restjson | ||
|
||
use aws.protocols#restJson1 | ||
use smithy.test#httpMalformedRequestTests | ||
|
||
@http(method: "POST", uri: "/MalformedContentTypeWithBody") | ||
operation MalformedContentTypeWithBody2 { | ||
input: GreetingStruct | ||
} | ||
|
||
structure GreetingStruct { | ||
salutation: String, | ||
} | ||
|
||
apply MalformedContentTypeWithBody2 @httpMalformedRequestTests([ | ||
{ | ||
id: "RestJsonWithBodyExpectsApplicationJsonContentTypeNoHeaders", | ||
documentation: "When there is modeled input, the content type must be application/json", | ||
protocol: restJson1, | ||
request: { | ||
method: "POST", | ||
uri: "/MalformedContentTypeWithBody", | ||
body: "{}", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need an additional test to verify that it's acceptable to omit the Content-Type header when the body is empty? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
}, | ||
response: { | ||
code: 415, | ||
headers: { | ||
"x-amzn-errortype": "UnsupportedMediaTypeException" | ||
} | ||
}, | ||
tags: [ "content-type" ] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, |
||
} | ||
]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
$version: "1.0" | ||
|
||
namespace aws.protocoltests.restjson | ||
|
||
use aws.protocols#restJson1 | ||
use smithy.test#httpRequestTests | ||
|
||
/// This example serializes a blob shape in the payload. | ||
/// | ||
/// In this example, no JSON document is synthesized because the payload is | ||
/// not a structure or a union type. | ||
@http(uri: "/HttpPayloadTraits", method: "POST") | ||
operation HttpPayloadTraits2 { | ||
input: HttpPayloadTraitsInputOutput, | ||
output: HttpPayloadTraitsInputOutput | ||
} | ||
|
||
apply HttpPayloadTraits2 @httpRequestTests([ | ||
{ | ||
id: "RestJsonHttpPayloadTraitsWithBlobAcceptsNoContentType", | ||
documentation: """ | ||
Servers must accept no content type for blob inputs | ||
without the media type trait.""", | ||
protocol: restJson1, | ||
method: "POST", | ||
uri: "/HttpPayloadTraits", | ||
body: "This is definitely a jpeg", | ||
bodyMediaType: "application/octet-stream", | ||
headers: { | ||
"X-Foo": "Foo", | ||
}, | ||
params: { | ||
foo: "Foo", | ||
blob: "This is definitely a jpeg" | ||
}, | ||
appliesTo: "server", | ||
tags: [ "content-type" ] | ||
} | ||
]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
$version: "2.0" | ||
|
||
namespace aws.protocoltests.restjson | ||
|
||
use smithy.test#httpRequestTests | ||
use smithy.test#httpResponseTests | ||
use smithy.test#httpMalformedRequestTests | ||
use smithy.framework#ValidationException | ||
|
||
@http(uri: "/EnumPayload2", method: "POST") | ||
@httpRequestTests([ | ||
{ | ||
id: "RestJsonEnumPayloadRequest2", | ||
uri: "/EnumPayload2", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we please add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These tests come from upstream, we should make the changes there. |
||
headers: { "Content-Type": "text/plain" }, | ||
body: "enumvalue", | ||
params: { payload: "enumvalue" }, | ||
method: "POST", | ||
protocol: "aws.protocols#restJson1" | ||
} | ||
]) | ||
@httpResponseTests([ | ||
{ | ||
id: "RestJsonEnumPayloadResponse2", | ||
headers: { "Content-Type": "text/plain" }, | ||
body: "enumvalue", | ||
params: { payload: "enumvalue" }, | ||
protocol: "aws.protocols#restJson1", | ||
code: 200 | ||
} | ||
]) | ||
operation HttpEnumPayload2 { | ||
input: EnumPayloadInput, | ||
output: EnumPayloadInput | ||
errors: [ValidationException] | ||
} | ||
|
||
@http(uri: "/StringPayload2", method: "POST") | ||
@httpRequestTests([ | ||
{ | ||
id: "RestJsonStringPayloadRequest2", | ||
uri: "/StringPayload2", | ||
body: "rawstring", | ||
bodyMediaType: "text/plain", | ||
headers: { | ||
"Content-Type": "text/plain", | ||
}, | ||
requireHeaders: [ | ||
"Content-Length" | ||
], | ||
params: { payload: "rawstring" }, | ||
method: "POST", | ||
protocol: "aws.protocols#restJson1" | ||
} | ||
]) | ||
@httpResponseTests([ | ||
{ | ||
id: "RestJsonStringPayloadResponse2", | ||
headers: { "Content-Type": "text/plain" }, | ||
body: "rawstring", | ||
bodyMediaType: "text/plain", | ||
params: { payload: "rawstring" }, | ||
protocol: "aws.protocols#restJson1", | ||
code: 200 | ||
} | ||
]) | ||
@httpMalformedRequestTests([ | ||
{ | ||
id: "RestJsonStringPayloadNoContentType2", | ||
documentation: "Serializes a string in the HTTP payload without a content-type header", | ||
protocol: "aws.protocols#restJson1", | ||
request: { | ||
method: "POST", | ||
uri: "/StringPayload2", | ||
body: "rawstring", | ||
// We expect a `Content-Type` header but none was provided. | ||
}, | ||
response: { | ||
code: 415, | ||
headers: { | ||
"x-amzn-errortype": "UnsupportedMediaTypeException" | ||
} | ||
}, | ||
tags: [ "content-type" ] | ||
}, | ||
{ | ||
id: "RestJsonStringPayloadWrongContentType2", | ||
documentation: "Serializes a string in the HTTP payload without the expected content-type header", | ||
protocol: "aws.protocols#restJson1", | ||
request: { | ||
method: "POST", | ||
uri: "/StringPayload2", | ||
body: "rawstring", | ||
headers: { | ||
// We expect `text/plain`. | ||
"Content-Type": "application/json", | ||
}, | ||
}, | ||
response: { | ||
code: 415, | ||
headers: { | ||
"x-amzn-errortype": "UnsupportedMediaTypeException" | ||
} | ||
}, | ||
tags: [ "content-type" ] | ||
}, | ||
{ | ||
id: "RestJsonStringPayloadUnsatisfiableAccept2", | ||
documentation: "Serializes a string in the HTTP payload with an unstatisfiable accept header", | ||
protocol: "aws.protocols#restJson1", | ||
request: { | ||
method: "POST", | ||
uri: "/StringPayload2", | ||
body: "rawstring", | ||
headers: { | ||
"Content-Type": "text/plain", | ||
// We can't satisfy this requirement; the server will return `text/plain`. | ||
"Accept": "application/json", | ||
}, | ||
}, | ||
response: { | ||
code: 406, | ||
headers: { | ||
"x-amzn-errortype": "NotAcceptableException" | ||
} | ||
}, | ||
tags: [ "accept" ] | ||
}, | ||
]) | ||
operation HttpStringPayload2 { | ||
input: StringPayloadInput, | ||
output: StringPayloadInput | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A general question: are we sticking to version 1.0 or do we write all new tests in version 2.0 syntax?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should write all new tests in 2.0 syntax. But since the tests in this file are copied from the ones upstream in Smithy (and they'll go away in the next Smithy release), this is OK, I just made the same modifications that I did upstream: smithy-lang/smithy#2310