From ed4284e2093ff1f4e29a285bffb71cf993486c6f Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Tue, 25 Apr 2023 13:04:35 +0200 Subject: [PATCH 1/5] Lazy initialize mime accept header Signed-off-by: Daniele Ahmed --- .../ServerHttpBoundProtocolGenerator.kt | 18 ++++++++++++++++- .../aws-smithy-http-server/src/protocols.rs | 20 ++++++++----------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt index 85b624b567..9f8a2e917b 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt @@ -178,11 +178,13 @@ class ServerHttpBoundProtocolTraitImplGenerator( outputSymbol: Symbol, operationShape: OperationShape, ) { + val operationName = symbolProvider.toSymbol(operationShape).name + val staticContentType = "CONTENT_TYPE_${operationName.uppercase()}" val verifyAcceptHeader = writable { httpBindingResolver.responseContentType(operationShape)?.also { contentType -> rustTemplate( """ - if !#{SmithyHttpServer}::protocols::accept_header_classifier(request.headers(), ${contentType.dq()}) { + if !#{SmithyHttpServer}::protocols::accept_header_classifier(request.headers(), &$staticContentType) { return Err(#{RequestRejection}::NotAcceptable); } """, @@ -190,6 +192,18 @@ class ServerHttpBoundProtocolTraitImplGenerator( ) } } + val verifyAcceptHeaderStaticContentTypeInit = writable { + httpBindingResolver.responseContentType(operationShape)?.also { contentType -> + rustTemplate( + """ + static $staticContentType: #{OnceCell}::sync::Lazy<#{Mime}::Mime> = #{OnceCell}::sync::Lazy::new(|| { + ${contentType.dq()}.parse::<#{Mime}::Mime>().expect("BUG: MIME parsing failed, content_type is not valid") + }); + """, + *codegenScope + ) + } + } val verifyRequestContentTypeHeader = writable { operationShape .inputShape(model) @@ -215,6 +229,7 @@ class ServerHttpBoundProtocolTraitImplGenerator( // TODO(https://github.com/awslabs/smithy-rs/issues/2238): Remove the `Pin>` and replace with thin wrapper around `Collect`. rustTemplate( """ + #{verifyAcceptHeaderStaticContentTypeInit:W} #{PinProjectLite}::pin_project! { /// A [`Future`](std::future::Future) aggregating the body bytes of a [`Request`] and constructing the /// [`${inputSymbol.name}`](#{I}) using modelled bindings. @@ -267,6 +282,7 @@ class ServerHttpBoundProtocolTraitImplGenerator( "Marker" to protocol.markerStruct(), "parse_request" to serverParseRequest(operationShape), "verifyAcceptHeader" to verifyAcceptHeader, + "verifyAcceptHeaderStaticContentTypeInit" to verifyAcceptHeaderStaticContentTypeInit, "verifyRequestContentTypeHeader" to verifyRequestContentTypeHeader, ) diff --git a/rust-runtime/aws-smithy-http-server/src/protocols.rs b/rust-runtime/aws-smithy-http-server/src/protocols.rs index 2db5e7163d..4b78c01358 100644 --- a/rust-runtime/aws-smithy-http-server/src/protocols.rs +++ b/rust-runtime/aws-smithy-http-server/src/protocols.rs @@ -66,14 +66,10 @@ pub fn content_type_header_classifier( Ok(()) } -pub fn accept_header_classifier(headers: &HeaderMap, content_type: &'static str) -> bool { +pub fn accept_header_classifier(headers: &HeaderMap, content_type: &mime::Mime) -> bool { if !headers.contains_key(http::header::ACCEPT) { return true; } - // Must be of the form: type/subtype - let content_type = content_type - .parse::() - .expect("BUG: MIME parsing failed, content_type is not valid"); headers .get_all(http::header::ACCEPT) .into_iter() @@ -195,41 +191,41 @@ mod tests { #[test] fn valid_accept_header_classifier_multiple_values() { let valid_request = req_accept("text/strings, application/json, invalid"); - assert!(accept_header_classifier(&valid_request, "application/json")); + assert!(accept_header_classifier(&valid_request, &"application/json".parse().unwrap())); } #[test] fn invalid_accept_header_classifier() { let invalid_request = req_accept("text/invalid, invalid, invalid/invalid"); - assert!(!accept_header_classifier(&invalid_request, "application/json")); + assert!(!accept_header_classifier(&invalid_request, &"application/json".parse().unwrap())); } #[test] fn valid_accept_header_classifier_star() { let valid_request = req_accept("application/*"); - assert!(accept_header_classifier(&valid_request, "application/json")); + assert!(accept_header_classifier(&valid_request, &"application/json".parse().unwrap())); } #[test] fn valid_accept_header_classifier_star_star() { let valid_request = req_accept("*/*"); - assert!(accept_header_classifier(&valid_request, "application/json")); + assert!(accept_header_classifier(&valid_request, &"application/json".parse().unwrap())); } #[test] fn valid_empty_accept_header_classifier() { - assert!(accept_header_classifier(&HeaderMap::new(), "application/json")); + assert!(accept_header_classifier(&HeaderMap::new(), &"application/json".parse().unwrap())); } #[test] fn valid_accept_header_classifier_with_params() { let valid_request = req_accept("application/json; q=30, */*"); - assert!(accept_header_classifier(&valid_request, "application/json")); + assert!(accept_header_classifier(&valid_request, &"application/json".parse().unwrap())); } #[test] fn valid_accept_header_classifier() { let valid_request = req_accept("application/json"); - assert!(accept_header_classifier(&valid_request, "application/json")); + assert!(accept_header_classifier(&valid_request, &"application/json".parse().unwrap())); } } From eea7b4f1ae4a5670b71638536dc938122a437872 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Tue, 25 Apr 2023 13:35:48 +0200 Subject: [PATCH 2/5] cargo fmt Signed-off-by: Daniele Ahmed --- .../aws-smithy-http-server/src/protocols.rs | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/rust-runtime/aws-smithy-http-server/src/protocols.rs b/rust-runtime/aws-smithy-http-server/src/protocols.rs index 4b78c01358..75d723f3fc 100644 --- a/rust-runtime/aws-smithy-http-server/src/protocols.rs +++ b/rust-runtime/aws-smithy-http-server/src/protocols.rs @@ -191,41 +191,62 @@ mod tests { #[test] fn valid_accept_header_classifier_multiple_values() { let valid_request = req_accept("text/strings, application/json, invalid"); - assert!(accept_header_classifier(&valid_request, &"application/json".parse().unwrap())); + assert!(accept_header_classifier( + &valid_request, + &"application/json".parse().unwrap() + )); } #[test] fn invalid_accept_header_classifier() { let invalid_request = req_accept("text/invalid, invalid, invalid/invalid"); - assert!(!accept_header_classifier(&invalid_request, &"application/json".parse().unwrap())); + assert!(!accept_header_classifier( + &invalid_request, + &"application/json".parse().unwrap() + )); } #[test] fn valid_accept_header_classifier_star() { let valid_request = req_accept("application/*"); - assert!(accept_header_classifier(&valid_request, &"application/json".parse().unwrap())); + assert!(accept_header_classifier( + &valid_request, + &"application/json".parse().unwrap() + )); } #[test] fn valid_accept_header_classifier_star_star() { let valid_request = req_accept("*/*"); - assert!(accept_header_classifier(&valid_request, &"application/json".parse().unwrap())); + assert!(accept_header_classifier( + &valid_request, + &"application/json".parse().unwrap() + )); } #[test] fn valid_empty_accept_header_classifier() { - assert!(accept_header_classifier(&HeaderMap::new(), &"application/json".parse().unwrap())); + assert!(accept_header_classifier( + &HeaderMap::new(), + &"application/json".parse().unwrap() + )); } #[test] fn valid_accept_header_classifier_with_params() { let valid_request = req_accept("application/json; q=30, */*"); - assert!(accept_header_classifier(&valid_request, &"application/json".parse().unwrap())); + assert!(accept_header_classifier( + &valid_request, + &"application/json".parse().unwrap() + )); } #[test] fn valid_accept_header_classifier() { let valid_request = req_accept("application/json"); - assert!(accept_header_classifier(&valid_request, &"application/json".parse().unwrap())); + assert!(accept_header_classifier( + &valid_request, + &"application/json".parse().unwrap() + )); } } From 7d748d2cb9dfd2ce8fefd59b45a780697d2425c4 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Tue, 25 Apr 2023 14:24:51 +0200 Subject: [PATCH 3/5] use mime constant Signed-off-by: Daniele Ahmed --- .../ServerHttpBoundProtocolGenerator.kt | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt index 9f8a2e917b..6f620254f8 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt @@ -194,14 +194,18 @@ class ServerHttpBoundProtocolTraitImplGenerator( } val verifyAcceptHeaderStaticContentTypeInit = writable { httpBindingResolver.responseContentType(operationShape)?.also { contentType -> - rustTemplate( + val init = when (contentType) { + "application/json" -> "#{Mime}::Mime = #{Mime}::APPLICATION_JSON;" + "application/octet-stream" -> "#{Mime}::Mime = #{Mime}::APPLICATION_OCTET_STREAM;" + "application/x-www-form-urlencoded" -> "#{Mime}::Mime = #{Mime}::APPLICATION_WWW_FORM_URLENCODED;" + else -> """ - static $staticContentType: #{OnceCell}::sync::Lazy<#{Mime}::Mime> = #{OnceCell}::sync::Lazy::new(|| { - ${contentType.dq()}.parse::<#{Mime}::Mime>().expect("BUG: MIME parsing failed, content_type is not valid") - }); - """, - *codegenScope - ) + #{OnceCell}::sync::Lazy<#{Mime}::Mime> = #{OnceCell}::sync::Lazy::new(|| { + ${contentType.dq()}.parse::<#{Mime}::Mime>().expect("BUG: MIME parsing failed, content_type is not valid") + }); + """ + } + rustTemplate("static $staticContentType: $init", *codegenScope) } } val verifyRequestContentTypeHeader = writable { From 609884c380d456b598b3cc46900edf7a0666f8f8 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Tue, 25 Apr 2023 14:46:00 +0200 Subject: [PATCH 4/5] ktlint Signed-off-by: Daniele Ahmed --- .../server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt index 6f620254f8..fe93860af1 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt @@ -199,7 +199,7 @@ class ServerHttpBoundProtocolTraitImplGenerator( "application/octet-stream" -> "#{Mime}::Mime = #{Mime}::APPLICATION_OCTET_STREAM;" "application/x-www-form-urlencoded" -> "#{Mime}::Mime = #{Mime}::APPLICATION_WWW_FORM_URLENCODED;" else -> - """ + """ #{OnceCell}::sync::Lazy<#{Mime}::Mime> = #{OnceCell}::sync::Lazy::new(|| { ${contentType.dq()}.parse::<#{Mime}::Mime>().expect("BUG: MIME parsing failed, content_type is not valid") }); From e07ca98c378843372bcff990a289b8af5a466df8 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Tue, 25 Apr 2023 14:57:19 +0200 Subject: [PATCH 5/5] use const instead Signed-off-by: Daniele Ahmed --- .../protocols/ServerHttpBoundProtocolGenerator.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt index fe93860af1..c61947a397 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt @@ -195,17 +195,17 @@ class ServerHttpBoundProtocolTraitImplGenerator( val verifyAcceptHeaderStaticContentTypeInit = writable { httpBindingResolver.responseContentType(operationShape)?.also { contentType -> val init = when (contentType) { - "application/json" -> "#{Mime}::Mime = #{Mime}::APPLICATION_JSON;" - "application/octet-stream" -> "#{Mime}::Mime = #{Mime}::APPLICATION_OCTET_STREAM;" - "application/x-www-form-urlencoded" -> "#{Mime}::Mime = #{Mime}::APPLICATION_WWW_FORM_URLENCODED;" + "application/json" -> "const $staticContentType: #{Mime}::Mime = #{Mime}::APPLICATION_JSON;" + "application/octet-stream" -> "const $staticContentType: #{Mime}::Mime = #{Mime}::APPLICATION_OCTET_STREAM;" + "application/x-www-form-urlencoded" -> "const $staticContentType: #{Mime}::Mime = #{Mime}::APPLICATION_WWW_FORM_URLENCODED;" else -> """ - #{OnceCell}::sync::Lazy<#{Mime}::Mime> = #{OnceCell}::sync::Lazy::new(|| { + static $staticContentType: #{OnceCell}::sync::Lazy<#{Mime}::Mime> = #{OnceCell}::sync::Lazy::new(|| { ${contentType.dq()}.parse::<#{Mime}::Mime>().expect("BUG: MIME parsing failed, content_type is not valid") }); """ } - rustTemplate("static $staticContentType: $init", *codegenScope) + rustTemplate(init, *codegenScope) } } val verifyRequestContentTypeHeader = writable {