Skip to content

Commit b583a2f

Browse files
david-perezdavid-perez
authored
Fix request Content-Type header checking in servers (#3690)
This fixes two bugs: 1. `Content-Type` header checking was succeeding when no `Content-Type` header was present but one was expected. 2. When a shape was `@httpPayload`-bound, `Content-Type` header checking occurred even when no payload was being sent. In this case it is not necessary to check the header, since there is no content. Code has been refactored and cleaned up. The crux of the logic is now easier to understand, and contained in `content_type_header_classifier`. ## Checklist <!--- If a checkbox below is not applicable, then please DELETE it rather than leaving it unchecked --> - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
1 parent a415cfe commit b583a2f

File tree

15 files changed

+420
-177
lines changed

15 files changed

+420
-177
lines changed

‎CHANGELOG.next.toml

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,17 @@
99
# message = "Fix typos in module documentation for generated crates"
1010
# references = ["smithy-rs#920"]
1111
# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
12-
# author = "rcoh"
12+
# author = "rcoh"
13+
[[smithy-rs]]
14+
message = """Fix request `Content-Type` header checking
15+
16+
Two bugs related to how servers were checking the `Content-Type` header in incoming requests have been fixed:
17+
18+
1. `Content-Type` header checking was incorrectly succeeding when no `Content-Type` header was present but one was expected.
19+
2. When a shape was @httpPayload`-bound, `Content-Type` header checking occurred even when no payload was being sent. In this case it is not necessary to check the header, since there is no content.
20+
21+
This is a breaking change in that servers are now stricter at enforcing the expected `Content-Type` header is being sent by the client in general, and laxer when the shape is bound with `@httpPayload`.
22+
"""
23+
references = ["smithy-rs#3690"]
24+
meta = { "breaking" = true, "tada" = false, "bug" = true, "target" = "server"}
25+
author = "david-perez"

‎codegen-client-test/build.gradle.kts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,16 @@ val allCodegenTests = listOf(
6767
ClientTest(
6868
"aws.protocoltests.restjson#RestJsonExtras",
6969
"rest_json_extras",
70-
dependsOn = listOf("rest-json-extras.smithy"),
70+
dependsOn = listOf(
71+
"rest-json-extras.smithy",
72+
// TODO(https://github.com/smithy-lang/smithy/pull/2310): Can be deleted when consumed in next Smithy version.
73+
"rest-json-extras-2310.smithy",
74+
// TODO(https://github.com/smithy-lang/smithy/pull/2314): Can be deleted when consumed in next Smithy version.
75+
"rest-json-extras-2314.smithy",
76+
// TODO(https://github.com/smithy-lang/smithy/pull/2315): Can be deleted when consumed in next Smithy version.
77+
// TODO(https://github.com/smithy-lang/smithy/pull/2331): Can be deleted when consumed in next Smithy version.
78+
"rest-json-extras-2315.smithy",
79+
),
7180
),
7281
ClientTest("aws.protocoltests.misc#MiscService", "misc", dependsOn = listOf("misc.smithy")),
7382
ClientTest("aws.protocoltests.restxml#RestXml", "rest_xml", addMessageToErrors = false),

‎codegen-core/common-test-models/rest-json-extras-2310.smithy

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
$version: "1.0"
2+
3+
namespace aws.protocoltests.restjson
4+
5+
use aws.protocols#restJson1
6+
use smithy.test#httpMalformedRequestTests
7+
8+
@http(method: "POST", uri: "/MalformedContentTypeWithBody")
9+
operation MalformedContentTypeWithBody2 {
10+
input: GreetingStruct
11+
}
12+
13+
structure GreetingStruct {
14+
salutation: String,
15+
}
16+
17+
apply MalformedContentTypeWithBody2 @httpMalformedRequestTests([
18+
{
19+
id: "RestJsonWithBodyExpectsApplicationJsonContentTypeNoHeaders",
20+
documentation: "When there is modeled input, the content type must be application/json",
21+
protocol: restJson1,
22+
request: {
23+
method: "POST",
24+
uri: "/MalformedContentTypeWithBody",
25+
body: "{}",
26+
},
27+
response: {
28+
code: 415,
29+
headers: {
30+
"x-amzn-errortype": "UnsupportedMediaTypeException"
31+
}
32+
},
33+
tags: [ "content-type" ]
34+
}
35+
])

‎codegen-core/common-test-models/rest-json-extras-2314.smithy

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
$version: "1.0"
2+
3+
namespace aws.protocoltests.restjson
4+
5+
use aws.protocols#restJson1
6+
use smithy.test#httpRequestTests
7+
8+
/// This example serializes a blob shape in the payload.
9+
///
10+
/// In this example, no JSON document is synthesized because the payload is
11+
/// not a structure or a union type.
12+
@http(uri: "/HttpPayloadTraits", method: "POST")
13+
operation HttpPayloadTraits2 {
14+
input: HttpPayloadTraitsInputOutput,
15+
output: HttpPayloadTraitsInputOutput
16+
}
17+
18+
apply HttpPayloadTraits2 @httpRequestTests([
19+
{
20+
id: "RestJsonHttpPayloadTraitsWithBlobAcceptsNoContentType",
21+
documentation: """
22+
Servers must accept no content type for blob inputs
23+
without the media type trait.""",
24+
protocol: restJson1,
25+
method: "POST",
26+
uri: "/HttpPayloadTraits",
27+
body: "This is definitely a jpeg",
28+
bodyMediaType: "application/octet-stream",
29+
headers: {
30+
"X-Foo": "Foo",
31+
},
32+
params: {
33+
foo: "Foo",
34+
blob: "This is definitely a jpeg"
35+
},
36+
appliesTo: "server",
37+
tags: [ "content-type" ]
38+
}
39+
])

‎codegen-core/common-test-models/rest-json-extras-2315.smithy

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
$version: "2.0"
2+
3+
namespace aws.protocoltests.restjson
4+
5+
use smithy.test#httpRequestTests
6+
use smithy.test#httpResponseTests
7+
use smithy.test#httpMalformedRequestTests
8+
use smithy.framework#ValidationException
9+
10+
@http(uri: "/EnumPayload2", method: "POST")
11+
@httpRequestTests([
12+
{
13+
id: "RestJsonEnumPayloadRequest2",
14+
uri: "/EnumPayload2",
15+
headers: { "Content-Type": "text/plain" },
16+
body: "enumvalue",
17+
params: { payload: "enumvalue" },
18+
method: "POST",
19+
protocol: "aws.protocols#restJson1"
20+
}
21+
])
22+
@httpResponseTests([
23+
{
24+
id: "RestJsonEnumPayloadResponse2",
25+
headers: { "Content-Type": "text/plain" },
26+
body: "enumvalue",
27+
params: { payload: "enumvalue" },
28+
protocol: "aws.protocols#restJson1",
29+
code: 200
30+
}
31+
])
32+
operation HttpEnumPayload2 {
33+
input: EnumPayloadInput,
34+
output: EnumPayloadInput
35+
errors: [ValidationException]
36+
}
37+
38+
@http(uri: "/StringPayload2", method: "POST")
39+
@httpRequestTests([
40+
{
41+
id: "RestJsonStringPayloadRequest2",
42+
uri: "/StringPayload2",
43+
body: "rawstring",
44+
bodyMediaType: "text/plain",
45+
headers: {
46+
"Content-Type": "text/plain",
47+
},
48+
requireHeaders: [
49+
"Content-Length"
50+
],
51+
params: { payload: "rawstring" },
52+
method: "POST",
53+
protocol: "aws.protocols#restJson1"
54+
}
55+
])
56+
@httpResponseTests([
57+
{
58+
id: "RestJsonStringPayloadResponse2",
59+
headers: { "Content-Type": "text/plain" },
60+
body: "rawstring",
61+
bodyMediaType: "text/plain",
62+
params: { payload: "rawstring" },
63+
protocol: "aws.protocols#restJson1",
64+
code: 200
65+
}
66+
])
67+
@httpMalformedRequestTests([
68+
{
69+
id: "RestJsonStringPayloadNoContentType2",
70+
documentation: "Serializes a string in the HTTP payload without a content-type header",
71+
protocol: "aws.protocols#restJson1",
72+
request: {
73+
method: "POST",
74+
uri: "/StringPayload2",
75+
body: "rawstring",
76+
// We expect a `Content-Type` header but none was provided.
77+
},
78+
response: {
79+
code: 415,
80+
headers: {
81+
"x-amzn-errortype": "UnsupportedMediaTypeException"
82+
}
83+
},
84+
tags: [ "content-type" ]
85+
},
86+
{
87+
id: "RestJsonStringPayloadWrongContentType2",
88+
documentation: "Serializes a string in the HTTP payload without the expected content-type header",
89+
protocol: "aws.protocols#restJson1",
90+
request: {
91+
method: "POST",
92+
uri: "/StringPayload2",
93+
body: "rawstring",
94+
headers: {
95+
// We expect `text/plain`.
96+
"Content-Type": "application/json",
97+
},
98+
},
99+
response: {
100+
code: 415,
101+
headers: {
102+
"x-amzn-errortype": "UnsupportedMediaTypeException"
103+
}
104+
},
105+
tags: [ "content-type" ]
106+
},
107+
{
108+
id: "RestJsonStringPayloadUnsatisfiableAccept2",
109+
documentation: "Serializes a string in the HTTP payload with an unstatisfiable accept header",
110+
protocol: "aws.protocols#restJson1",
111+
request: {
112+
method: "POST",
113+
uri: "/StringPayload2",
114+
body: "rawstring",
115+
headers: {
116+
"Content-Type": "text/plain",
117+
// We can't satisfy this requirement; the server will return `text/plain`.
118+
"Accept": "application/json",
119+
},
120+
},
121+
response: {
122+
code: 406,
123+
headers: {
124+
"x-amzn-errortype": "NotAcceptableException"
125+
}
126+
},
127+
tags: [ "accept" ]
128+
},
129+
])
130+
operation HttpStringPayload2 {
131+
input: StringPayloadInput,
132+
output: StringPayloadInput
133+
}

‎codegen-core/common-test-models/rest-json-extras.smithy

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,13 @@ service RestJsonExtras {
6666
CaseInsensitiveErrorOperation,
6767
EmptyStructWithContentOnWireOp,
6868
QueryPrecedence,
69+
// TODO(https://github.com/smithy-lang/smithy/pull/2314)
70+
HttpPayloadTraits2,
71+
// TODO(https://github.com/smithy-lang/smithy/pull/2310)
72+
MalformedContentTypeWithBody2,
73+
// TODO(https://github.com/smithy-lang/smithy/pull/2315)
74+
HttpEnumPayload2,
75+
HttpStringPayload2,
6976
],
7077
errors: [ExtraError]
7178
}
@@ -101,6 +108,7 @@ structure ExtraError {}
101108
id: "StringPayload",
102109
uri: "/StringPayload",
103110
body: "rawstring",
111+
headers: { "Content-Type": "text/plain" },
104112
params: { payload: "rawstring" },
105113
method: "POST",
106114
protocol: "aws.protocols#restJson1"

‎codegen-server-test/build.gradle.kts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,16 @@ val allCodegenTests = "../codegen-core/common-test-models".let { commonModels ->
6464
CodegenTest(
6565
"aws.protocoltests.restjson#RestJsonExtras",
6666
"rest_json_extras",
67-
imports = listOf("$commonModels/rest-json-extras.smithy"),
67+
imports = listOf(
68+
"$commonModels/rest-json-extras.smithy",
69+
// TODO(https://github.com/smithy-lang/smithy/pull/2310): Can be deleted when consumed in next Smithy version.
70+
"$commonModels/rest-json-extras-2310.smithy",
71+
// TODO(https://github.com/smithy-lang/smithy/pull/2314): Can be deleted when consumed in next Smithy version.
72+
"$commonModels/rest-json-extras-2314.smithy",
73+
// TODO(https://github.com/smithy-lang/smithy/pull/2315): Can be deleted when consumed in next Smithy version.
74+
// TODO(https://github.com/smithy-lang/smithy/pull/2331): Can be deleted when consumed in next Smithy version.
75+
"$commonModels/rest-json-extras-2315.smithy",
76+
),
6877
),
6978
CodegenTest(
7079
"aws.protocoltests.restjson.validation#RestJsonValidation",

‎codegen-server-test/python/build.gradle.kts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,15 @@ val allCodegenTests = "../../codegen-core/common-test-models".let { commonModels
6161
CodegenTest(
6262
"aws.protocoltests.restjson#RestJsonExtras",
6363
"rest_json_extras",
64-
imports = listOf("$commonModels/rest-json-extras.smithy"),
64+
imports = listOf(
65+
"$commonModels/rest-json-extras.smithy",
66+
// TODO(https://github.com/smithy-lang/smithy/pull/2310): Can be deleted when consumed in next Smithy version.
67+
"$commonModels/rest-json-extras-2310.smithy",
68+
// TODO(https://github.com/smithy-lang/smithy/pull/2314): Can be deleted when consumed in next Smithy version.
69+
"$commonModels/rest-json-extras-2314.smithy",
70+
// TODO(https://github.com/smithy-lang/smithy/pull/2315): Can be deleted when consumed in next Smithy version.
71+
"$commonModels/rest-json-extras-2315.smithy",
72+
),
6573
),
6674
// TODO(https://github.com/smithy-lang/smithy-rs/issues/2477)
6775
// CodegenTest(

‎codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerTypesTest.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ internal class PythonServerTypesTest {
130130
let req = Request::builder()
131131
.method("POST")
132132
.uri("/echo")
133+
.header("content-type", "application/json")
133134
.body(Body::from(${payload.dq()}))
134135
.unwrap();
135136
@@ -222,6 +223,7 @@ internal class PythonServerTypesTest {
222223
let req = Request::builder()
223224
.method("POST")
224225
.uri("/echo")
226+
.header("content-type", "application/json")
225227
.body(Body::from("{\"value\":1676298520}"))
226228
.unwrap();
227229
let res = service.call(req).await.unwrap();

‎codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,9 @@ class ServerProtocolTestGenerator(
812812
FailingTest(REST_JSON, "RestJsonEndpointTrait", TestType.Request),
813813
FailingTest(REST_JSON, "RestJsonEndpointTraitWithHostLabel", TestType.Request),
814814
FailingTest(REST_JSON, "RestJsonOmitsEmptyListQueryValues", TestType.Request),
815+
// TODO(https://github.com/smithy-lang/smithy/pull/2315): Can be deleted when fixed tests are consumed in next Smithy version
816+
FailingTest(REST_JSON, "RestJsonEnumPayloadRequest", TestType.Request),
817+
FailingTest(REST_JSON, "RestJsonStringPayloadRequest", TestType.Request),
815818
// Tests involving `@range` on floats.
816819
// Pending resolution from the Smithy team, see https://github.com/smithy-lang/smithy-rs/issues/2007.
817820
FailingTest(REST_JSON_VALIDATION, "RestJsonMalformedRangeFloat_case0", TestType.MalformedRequest),

0 commit comments

Comments
 (0)