Skip to content

Commit

Permalink
Allow expected content-type to ignore parameters (#3724)
Browse files Browse the repository at this point in the history
Fixes: #3471

## Motivation and Context
An issue was raised about a mobile client that appends "; charset=utf-8"
to the Content-Type when using restJson1. The [latest
RFC](https://www.rfc-editor.org/rfc/rfc8259) for "application/json" does
not register a charset parameter, but indicates it is reasonable to
accept it.

## Description
This change loosens the validation of the expected content type to allow
all parameters.

## Testing
* Tests for each protocol were added to
[smithy](smithy-lang/smithy#2296)
* ran the runtime and codegen tests
* Added test for rest-xml, as smithy-rs does not currently run the
smithy tests.

## 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
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK 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._
  • Loading branch information
djedward authored Jun 28, 2024
1 parent 5498a18 commit 4beac5f
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 4 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,11 @@ message = "A feature, `aws-lambda`, has been added to generated SDKs to re-expor
references = ["smithy-rs#3643"]
meta = { "breaking" = false, "bug" = true, "tada" = false, "target" = "server" }
author = "drganjoo"

[[smithy-rs]]
message = """
Content-Type header validation now ignores parameter portion of media types.
"""
references = ["smithy-rs#3471","smithy-rs#3724"]
meta = { "breaking" = false, "tada" = false, "bug" = true, target = "server" }
authors = ["djedward"]
53 changes: 53 additions & 0 deletions codegen-core/common-test-models/rest-xml-extras.smithy
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
$version: "2.0"

namespace aws.protocoltests.restxml

use aws.api#service
use aws.protocols#restXml
use smithy.test#httpRequestTests

/// A REST XML service that sends XML requests and responses.
@service(sdkId: "Rest Xml Protocol")
@restXml
@title("Sample Rest Xml Protocol Service")
service RestXmlExtras {
version: "2024-04-15",
operations: [
ContentTypeParameters
]
}

/// The example tests how servers must support requests
/// containing a `Content-Type` header with parameters.
@http(uri: "/ContentTypeParameters", method: "PUT")
operation ContentTypeParameters {
input: ContentTypeParametersInput,
output: ContentTypeParametersOutput
}

apply ContentTypeParameters @httpRequestTests([
{
id: "RestXmlMustSupportParametersInContentType",
documentation: "A server should ignore parameters added to the content type",
protocol: restXml,
method: "PUT",
headers: {
"Content-Type": "application/xml; charset=utf-8"
},
uri: "/ContentTypeParameters",
body: "<ContentTypeParametersInput><value>5</value></ContentTypeParametersInput>",
bodyMediaType: "application/xml",
params: {
value: 5,
},
appliesTo: "server"
}
])

@input
structure ContentTypeParametersInput {
value: Integer,
}

@output
structure ContentTypeParametersOutput {}
5 changes: 5 additions & 0 deletions codegen-server-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ val allCodegenTests = "../codegen-core/common-test-models".let { commonModels ->
"pokemon-service-awsjson-server-sdk",
imports = listOf("$commonModels/pokemon-awsjson.smithy", "$commonModels/pokemon-common.smithy"),
),
CodegenTest(
"aws.protocoltests.restxml#RestXmlExtras",
"rest_xml_extras",
imports = listOf("$commonModels/rest-xml-extras.smithy"),
),
)
}

Expand Down
2 changes: 1 addition & 1 deletion rust-runtime/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions rust-runtime/aws-smithy-http-server/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "aws-smithy-http-server"
version = "0.62.0"
version = "0.62.1"
authors = ["Smithy Rust Server <[email protected]>"]
edition = "2021"
license = "Apache-2.0"
Expand Down Expand Up @@ -29,7 +29,7 @@ http = "0.2"
http-body = "0.4"
hyper = { version = "0.14.26", features = ["server", "http1", "http2", "tcp", "stream"] }
lambda_http = { version = "0.8.0", optional = true }
mime = "0.3.4"
mime = "0.3.17"
nom = "7"
once_cell = "1.13"
pin-project-lite = "0.2"
Expand Down
9 changes: 8 additions & 1 deletion rust-runtime/aws-smithy-http-server/src/protocol/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ fn content_type_header_classifier(
(Some(actual_content_type), Some(expected_content_type)) => {
let expected_mime = parse_expected_mime(expected_content_type);
let found_mime = parse_mime(actual_content_type)?;
if expected_mime != found_mime {
if expected_mime != found_mime.essence_str() {
Err(MissingContentTypeReason::UnexpectedMimeType {
expected_mime: Some(expected_mime),
found_mime: Some(found_mime),
Expand Down Expand Up @@ -241,6 +241,13 @@ mod tests {
));
}

#[test]
fn valid_content_type_header_classifier_http_params() {
let request = req_content_type_smithy("application/json; charset=utf-8");
let result = content_type_header_classifier_smithy(&request, APPLICATION_JSON);
assert!(result.is_ok());
}

#[test]
fn valid_accept_header_classifier_multiple_values() {
let valid_request = req_accept("text/strings, application/json, invalid");
Expand Down

0 comments on commit 4beac5f

Please sign in to comment.