From 25a21a95f92ea3f9764f7f3e996bdbad1af3c5be Mon Sep 17 00:00:00 2001 From: David Calavera Date: Sat, 27 Aug 2022 12:11:00 -0700 Subject: [PATCH 1/2] Fix paths with spaces in HTTP requests. Encode the characters not allowed in URLs before creating the request. Signed-off-by: David Calavera --- lambda-http/Cargo.toml | 1 + lambda-http/src/request.rs | 214 ++++++++++-------- .../data/apigw_request_path_with_space.json | 57 +++++ 3 files changed, 172 insertions(+), 100 deletions(-) create mode 100644 lambda-http/tests/data/apigw_request_path_with_space.json diff --git a/lambda-http/Cargo.toml b/lambda-http/Cargo.toml index 092a29d3..f919a277 100644 --- a/lambda-http/Cargo.toml +++ b/lambda-http/Cargo.toml @@ -32,6 +32,7 @@ serde_urlencoded = "0.7.0" query_map = { version = "0.5", features = ["url-query"] } mime = "0.3.16" encoding_rs = "0.8.31" +url = "2.2.2" [dependencies.aws_lambda_events] version = "^0.6.3" diff --git a/lambda-http/src/request.rs b/lambda-http/src/request.rs index c7ba1e20..7d37603d 100644 --- a/lambda-http/src/request.rs +++ b/lambda-http/src/request.rs @@ -14,12 +14,14 @@ use aws_lambda_events::apigw::{ApiGatewayV2httpRequest, ApiGatewayV2httpRequestC use aws_lambda_events::apigw::{ApiGatewayWebsocketProxyRequest, ApiGatewayWebsocketProxyRequestContext}; use aws_lambda_events::encodings::Body; use http::header::HeaderName; +use http::HeaderMap; use query_map::QueryMap; use serde::Deserialize; use serde_json::error::Error as JsonError; use std::future::Future; use std::pin::Pin; use std::{io::Read, mem}; +use url::Url; /// Internal representation of an Lambda http event from /// ALB, API Gateway REST and HTTP API proxy event perspectives @@ -82,7 +84,13 @@ pub enum RequestOrigin { #[cfg(feature = "apigw_http")] fn into_api_gateway_v2_request(ag: ApiGatewayV2httpRequest) -> http::Request { let http_method = ag.request_context.http.method.clone(); + let host = ag + .headers + .get(http::header::HOST) + .and_then(|s| s.to_str().ok()) + .or(ag.request_context.domain_name.as_deref()); let raw_path = ag.raw_path.unwrap_or_default(); + let path = apigw_path_with_stage(&ag.request_context.stage, &raw_path); // don't use the query_string_parameters from API GW v2 to // populate the QueryStringParameters extension because @@ -95,32 +103,14 @@ fn into_api_gateway_v2_request(ag: ApiGatewayV2httpRequest) -> http::Request path, - Some(host) => { - let scheme = ag - .headers - .get(x_forwarded_proto()) - .and_then(|s| s.to_str().ok()) - .unwrap_or("https"); - format!("{}://{}{}", scheme, host, path) - } - }; - if let Some(query) = ag.raw_query_string { - url.push('?'); - url.push_str(&query); - } - url - }) + .uri(uri) .extension(RawHttpPath(raw_path)) .extension(QueryStringParameters(query_string_parameters)) .extension(PathParameters(QueryMap::from(ag.path_parameters))) @@ -154,34 +144,22 @@ fn into_api_gateway_v2_request(ag: ApiGatewayV2httpRequest) -> http::Request http::Request { let http_method = ag.http_method; + let host = ag + .headers + .get(http::header::HOST) + .and_then(|s| s.to_str().ok()) + .or(ag.request_context.domain_name.as_deref()); let raw_path = ag.path.unwrap_or_default(); + let path = apigw_path_with_stage(&ag.request_context.stage, &raw_path); let builder = http::Request::builder() - .uri({ - let host = ag.headers.get(http::header::HOST).and_then(|s| s.to_str().ok()); - let path = apigw_path_with_stage(&ag.request_context.stage, &raw_path); - - let mut url = match host { - None => path, - Some(host) => { - let scheme = ag - .headers - .get(x_forwarded_proto()) - .and_then(|s| s.to_str().ok()) - .unwrap_or("https"); - format!("{}://{}{}", scheme, host, path) - } - }; - - if !ag.multi_value_query_string_parameters.is_empty() { - url.push('?'); - url.push_str(&ag.multi_value_query_string_parameters.to_query_string()); - } else if !ag.query_string_parameters.is_empty() { - url.push('?'); - url.push_str(&ag.query_string_parameters.to_query_string()); - } - url - }) + .uri(build_request_uri( + &path, + &ag.headers, + host, + Some(&ag.multi_value_query_string_parameters), + Some(&ag.query_string_parameters), + )) .extension(RawHttpPath(raw_path)) // multi-valued query string parameters are always a super // set of singly valued query string parameters, @@ -221,34 +199,17 @@ fn into_proxy_request(ag: ApiGatewayProxyRequest) -> http::Request { #[cfg(feature = "alb")] fn into_alb_request(alb: AlbTargetGroupRequest) -> http::Request { let http_method = alb.http_method; + let host = alb.headers.get(http::header::HOST).and_then(|s| s.to_str().ok()); let raw_path = alb.path.unwrap_or_default(); let builder = http::Request::builder() - .uri({ - let host = alb.headers.get(http::header::HOST).and_then(|s| s.to_str().ok()); - - let mut url = match host { - None => raw_path.clone(), - Some(host) => { - let scheme = alb - .headers - .get(x_forwarded_proto()) - .and_then(|s| s.to_str().ok()) - .unwrap_or("https"); - format!("{}://{}{}", scheme, host, &raw_path) - } - }; - - if !alb.multi_value_query_string_parameters.is_empty() { - url.push('?'); - url.push_str(&alb.multi_value_query_string_parameters.to_query_string()); - } else if !alb.query_string_parameters.is_empty() { - url.push('?'); - url.push_str(&alb.query_string_parameters.to_query_string()); - } - - url - }) + .uri(build_request_uri( + &raw_path, + &alb.headers, + host, + Some(&alb.multi_value_query_string_parameters), + Some(&alb.query_string_parameters), + )) .extension(RawHttpPath(raw_path)) // multi valued query string parameters are always a super // set of singly valued query string parameters, @@ -287,32 +248,21 @@ fn into_alb_request(alb: AlbTargetGroupRequest) -> http::Request { #[cfg(feature = "apigw_websockets")] fn into_websocket_request(ag: ApiGatewayWebsocketProxyRequest) -> http::Request { let http_method = ag.http_method; + let host = ag + .headers + .get(http::header::HOST) + .and_then(|s| s.to_str().ok()) + .or(ag.request_context.domain_name.as_deref()); + let path = apigw_path_with_stage(&ag.request_context.stage, &ag.path.unwrap_or_default()); + let builder = http::Request::builder() - .uri({ - let host = ag.headers.get(http::header::HOST).and_then(|s| s.to_str().ok()); - let path = apigw_path_with_stage(&ag.request_context.stage, &ag.path.unwrap_or_default()); - - let mut url = match host { - None => path, - Some(host) => { - let scheme = ag - .headers - .get(x_forwarded_proto()) - .and_then(|s| s.to_str().ok()) - .unwrap_or("https"); - format!("{}://{}{}", scheme, host, path) - } - }; - - if !ag.multi_value_query_string_parameters.is_empty() { - url.push('?'); - url.push_str(&ag.multi_value_query_string_parameters.to_query_string()); - } else if !ag.query_string_parameters.is_empty() { - url.push('?'); - url.push_str(&ag.query_string_parameters.to_query_string()); - } - url - }) + .uri(build_request_uri( + &path, + &ag.headers, + host, + Some(&ag.multi_value_query_string_parameters), + Some(&ag.query_string_parameters), + )) // multi-valued query string parameters are always a super // set of singly valued query string parameters, // when present, multi-valued query string parameters are preferred @@ -438,6 +388,43 @@ fn x_forwarded_proto() -> HeaderName { HeaderName::from_static("x-forwarded-proto") } +fn build_request_uri( + path: &str, + headers: &HeaderMap, + host: Option<&str>, + multi_value_query: Option<&QueryMap>, + single_value_query: Option<&QueryMap>, +) -> String { + // let host = headers.get(http::header::HOST).and_then(|s| s.to_str().ok()); + + let mut url = match host { + None => { + let rel_url = Url::parse(&format!("http://localhost{}", path)).unwrap(); + rel_url.path().to_string() + } + Some(host) => { + let scheme = headers + .get(x_forwarded_proto()) + .and_then(|s| s.to_str().ok()) + .unwrap_or("https"); + let url = format!("{}://{}{}", scheme, host, path); + Url::parse(&url).unwrap().to_string() + } + }; + + if let (Some(mv), Some(sv)) = (multi_value_query, single_value_query) { + if !mv.is_empty() { + url.push('?'); + url.push_str(&mv.to_query_string()); + } else if !sv.is_empty() { + url.push('?'); + url.push_str(&sv.to_query_string()); + } + } + + url +} + #[cfg(test)] mod tests { use super::*; @@ -666,4 +653,31 @@ mod tests { assert_eq!(req.method(), "GET"); assert_eq!(req.uri(), "/v1/health/"); } + + #[test] + fn deserialize_apigw_path_with_space() { + // generated from ALB health checks + let input = include_str!("../tests/data/apigw_request_path_with_space.json"); + let result = from_str(input); + assert!( + result.is_ok(), + "event was not parsed as expected {:?} given {}", + result, + input + ); + let req = result.expect("failed to parse request"); + assert_eq!(req.uri(), "https://id.execute-api.us-east-1.amazonaws.com/my/path-with%20space?parameter1=value1¶meter1=value2¶meter2=value"); + } + + #[test] + fn parse_paths_with_spaces() { + let url = build_request_uri( + "/path with spaces/and multiple segments", + &HeaderMap::new(), + None, + None, + None, + ); + assert_eq!("/path%20with%20spaces/and%20multiple%20segments", url); + } } diff --git a/lambda-http/tests/data/apigw_request_path_with_space.json b/lambda-http/tests/data/apigw_request_path_with_space.json new file mode 100644 index 00000000..53f82382 --- /dev/null +++ b/lambda-http/tests/data/apigw_request_path_with_space.json @@ -0,0 +1,57 @@ +{ + "version": "2.0", + "routeKey": "$default", + "rawPath": "/my/path-with space", + "rawQueryString": "parameter1=value1¶meter1=value2¶meter2=value", + "cookies": [ + "cookie1=value1", + "cookie2=value2" + ], + "headers": { + "Header1": "value1", + "Header2": "value2" + }, + "queryStringParameters": { + "parameter1": "value1,value2", + "parameter2": "value" + }, + "requestContext": { + "accountId": "123456789012", + "apiId": "api-id", + "authorizer": { + "jwt": { + "claims": { + "claim1": "value1", + "claim2": "value2" + }, + "scopes": [ + "scope1", + "scope2" + ] + } + }, + "domainName": "id.execute-api.us-east-1.amazonaws.com", + "domainPrefix": "id", + "http": { + "method": "POST", + "path": "/my/path-with space", + "protocol": "HTTP/1.1", + "sourceIp": "IP", + "userAgent": "agent" + }, + "requestId": "id", + "routeKey": "$default", + "stage": "$default", + "time": "12/Mar/2020:19:03:58 +0000", + "timeEpoch": 1583348638390 + }, + "body": "Hello from Lambda", + "pathParameters": { + "parameter1": "value1" + }, + "isBase64Encoded": false, + "stageVariables": { + "stageVariable1": "value1", + "stageVariable2": "value2" + } +} \ No newline at end of file From 5d0e88153a003bb8b9b293a324277aec0944c8b4 Mon Sep 17 00:00:00 2001 From: David Calavera Date: Sun, 28 Aug 2022 09:40:00 -0700 Subject: [PATCH 2/2] Cleanup parameters and comments. Signed-off-by: David Calavera --- lambda-http/src/request.rs | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/lambda-http/src/request.rs b/lambda-http/src/request.rs index 7d37603d..27d54cf2 100644 --- a/lambda-http/src/request.rs +++ b/lambda-http/src/request.rs @@ -103,7 +103,7 @@ fn into_api_gateway_v2_request(ag: ApiGatewayV2httpRequest) -> http::Request http::Request { &path, &ag.headers, host, - Some(&ag.multi_value_query_string_parameters), - Some(&ag.query_string_parameters), + Some((&ag.multi_value_query_string_parameters, &ag.query_string_parameters)), )) .extension(RawHttpPath(raw_path)) // multi-valued query string parameters are always a super @@ -207,8 +206,7 @@ fn into_alb_request(alb: AlbTargetGroupRequest) -> http::Request { &raw_path, &alb.headers, host, - Some(&alb.multi_value_query_string_parameters), - Some(&alb.query_string_parameters), + Some((&alb.multi_value_query_string_parameters, &alb.query_string_parameters)), )) .extension(RawHttpPath(raw_path)) // multi valued query string parameters are always a super @@ -260,8 +258,7 @@ fn into_websocket_request(ag: ApiGatewayWebsocketProxyRequest) -> http::Request< &path, &ag.headers, host, - Some(&ag.multi_value_query_string_parameters), - Some(&ag.query_string_parameters), + Some((&ag.multi_value_query_string_parameters, &ag.query_string_parameters)), )) // multi-valued query string parameters are always a super // set of singly valued query string parameters, @@ -392,11 +389,8 @@ fn build_request_uri( path: &str, headers: &HeaderMap, host: Option<&str>, - multi_value_query: Option<&QueryMap>, - single_value_query: Option<&QueryMap>, + queries: Option<(&QueryMap, &QueryMap)>, ) -> String { - // let host = headers.get(http::header::HOST).and_then(|s| s.to_str().ok()); - let mut url = match host { None => { let rel_url = Url::parse(&format!("http://localhost{}", path)).unwrap(); @@ -412,7 +406,7 @@ fn build_request_uri( } }; - if let (Some(mv), Some(sv)) = (multi_value_query, single_value_query) { + if let Some((mv, sv)) = queries { if !mv.is_empty() { url.push('?'); url.push_str(&mv.to_query_string()); @@ -671,13 +665,7 @@ mod tests { #[test] fn parse_paths_with_spaces() { - let url = build_request_uri( - "/path with spaces/and multiple segments", - &HeaderMap::new(), - None, - None, - None, - ); + let url = build_request_uri("/path with spaces/and multiple segments", &HeaderMap::new(), None, None); assert_eq!("/path%20with%20spaces/and%20multiple%20segments", url); } }