Repository: awslabs/aws-lambda-rust-runtime (language: rust)

Issue:
lambda_http fails to build request with spaces - http::Error(InvalidUri(InvalidUriChar))
Hi,

I'm running into an issue where URLs with spaces are causing my Lambda function to return server errors on API Gateway.

I'm no expert in URL encoding but I do realise that spaces are unsafe characters and should be avoided at all costs. Unfortunately the product I am building is acting as a sort of redirecting proxy and I am not in full control of the URL structure.

Here is the information needed to reproduce this error:
```bash
~ rustc --version
rustc 1.61.0 (fe5b13d68 2022-05-18)
```

Place the following files in the same directory:

```toml
# Cargo.toml
[package]
name = "http-basic-bug"
version = "0.1.0"
edition = "2021"

[[bin]]
name = "example"
path = "main.rs"

[dependencies]
lambda_http = "0.6.0"
lambda_runtime = "0.6.0"
tokio = { version = "1", features = ["macros"] }
tracing = { version = "0.1", features = ["log"] }
tracing-subscriber = { version = "0.3", default-features = false, features = ["fmt"] }
```

```rust
// main.rs
use lambda_http::{run, service_fn, Body, Error, Request, Response};

async fn function_handler(_event: Request) -> Result<Response<Body>, Error> {
    // Extract some useful information from the request

    // Return something that implements IntoResponse.
    // It will be serialized to the right response event automatically by the runtime
    let resp = Response::builder()
        .status(200)
        .header("content-type", "text/html")
        .body("Hello AWS Lambda HTTP request".into())
        .map_err(Box::new)?;
    Ok(resp)
}

#[tokio::main]
async fn main() -> Result<(), Error> {
    tracing_subscriber::fmt()
        .with_max_level(tracing::Level::INFO)
        // disabling time is handy because CloudWatch will add the ingestion time.
        .without_time()
        .init();

    run(service_fn(function_handler)).await
}

#[cfg(test)]
mod tests {
    use super::*;

    #[tokio::test]
    async fn test_no_space() {
        let input = include_str!("good-request.json");
        let request = lambda_http::request::from_str(input).expect("failed to create request");
        let response = function_handler(request)
            .await
            .expect("failed to handle request");
        assert_eq!(response.status().as_u16(), 200);
    }

    #[tokio::test]
    async fn test_with_space() {
        let input = include_str!("bad-request.json");
        let request = lambda_http::request::from_str(input).expect("failed to create request");
        let response = function_handler(request)
            .await
            .expect("failed to handle request");
        assert_eq!(response.status().as_u16(), 200);
    }
}
```

`good-request.json`
```json
{
    "version": "2.0",
    "routeKey": "$default",
    "rawPath": "/my/path",
    "rawQueryString": "parameter1=value1&parameter1=value2&parameter2=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",
        "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"}
}
```

`bad-request.json`
```json
{
    "version": "2.0",
    "routeKey": "$default",
    "rawPath": "/my/path-with space",
    "rawQueryString": "parameter1=value1&parameter1=value2&parameter2=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"}
}
```

Run the tests

```bash
~ cargo test
   Compiling http-basic-bug v0.1.0 (/private/tmp/lambda_http_bug)
    Finished test [unoptimized + debuginfo] target(s) in 0.74s
     Running unittests main.rs (target/debug/deps/example-4220d3d5bc4bf2cb)

running 2 tests
test tests::test_no_space ... ok
test tests::test_with_space ... FAILED

failures:

---- tests::test_with_space stdout ----
thread 'tests::test_with_space' panicked at 'failed to build request: http::Error(InvalidUri(InvalidUriChar))', /Users/<REDACTED>/.cargo/registry/src/github.com-1ecc6299db9ec823/lambda_http-0.6.0/src/request.rs:145:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    tests::test_with_space

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass '--bin example'
```


If this is a case of using unsafe characters or you don't feel like this is an issue of `lambda_http` then please close this issue.


Discussion / hints:
Thanks for opening this issue with such clear details. I think we probably need to encode the path before creating the URI for all the events. That should fix the issue, any other ideas are very welcome. Probably in these two places if you want to give it a try:

https://github.com/awslabs/aws-lambda-rust-runtime/blob/main/lambda-http/src/request.rs#L351
https://github.com/awslabs/aws-lambda-rust-runtime/blob/main/lambda-http/src/request.rs#L224

Fix patch:
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..27d54cf2 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<Body> {
     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<Bod
         ag.query_string_parameters
     };
 
+    let mut uri = build_request_uri(&path, &ag.headers, host, None);
+    if let Some(query) = ag.raw_query_string {
+        uri.push('?');
+        uri.push_str(&query);
+    }
+
     let builder = http::Request::builder()
-        .uri({
-            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, &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 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,21 @@ fn into_api_gateway_v2_request(ag: ApiGatewayV2httpRequest) -> http::Request<Bod
 #[cfg(feature = "apigw_rest")]
 fn into_proxy_request(ag: ApiGatewayProxyRequest) -> http::Request<Body> {
     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, &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 +198,16 @@ fn into_proxy_request(ag: ApiGatewayProxyRequest) -> http::Request<Body> {
 #[cfg(feature = "alb")]
 fn into_alb_request(alb: AlbTargetGroupRequest) -> http::Request<Body> {
     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, &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 +246,20 @@ fn into_alb_request(alb: AlbTargetGroupRequest) -> http::Request<Body> {
 #[cfg(feature = "apigw_websockets")]
 fn into_websocket_request(ag: ApiGatewayWebsocketProxyRequest) -> http::Request<Body> {
     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, &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 +385,40 @@ fn x_forwarded_proto() -> HeaderName {
     HeaderName::from_static("x-forwarded-proto")
 }
 
+fn build_request_uri(
+    path: &str,
+    headers: &HeaderMap,
+    host: Option<&str>,
+    queries: Option<(&QueryMap, &QueryMap)>,
+) -> String {
+    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, sv)) = queries {
+        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 +647,25 @@ 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&parameter1=value2&parameter2=value");
+    }
+
+    #[test]
+    fn parse_paths_with_spaces() {
+        let url = build_request_uri("/path with spaces/and multiple segments", &HeaderMap::new(), None, None);
+        assert_eq!("/path%20with%20spaces/and%20multiple%20segments", url);
+    }
 }


Test patch:
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&parameter1=value2&parameter2=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