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

Issue:
Querystrings that have percent encodings get double encoded for Lambda ALB Events in lambda-http
This was an issue originally raised here: https://github.com/awslabs/aws-lambda-web-adapter/issues/99. Since the root issue is not in that project but this one I'm moving discussion here.

**Problem**: Whenever a url that contains a querystring with percent encoded values is passed through lambda-http, it gets double encoded in the newly generated http request if the original event source was an ALB. This is because the Lambda Event for ALBs does not automatically decode querystrings, whereas the same event for API Gateways do:

https://serverless-training.com/articles/api-gateway-vs-application-load-balancer-technical-details/ (about half way down the page)

> Note that the keys and values of evt.queryStringParameters will be:
> *  already URL decoded for you on API Gateway. e.g. ?foo%5Ba%5D=foo%5B will be { 'foo[a]': [ 'foo[' ] } 
> *  but not URL decoded on ALB. e.g. ?foo%5Ba%5D=foo%5B will be { 'foo%5Ba%5D': [ 'foo%5B' ] }

Discussion on the original issue lead to the thought that this difference should be handled here (but currently is not): https://github.com/awslabs/aws-lambda-rust-runtime/blob/fd2ea235f4adb166422ca5b612c4a1a42a626daa/lambda-http/src/request.rs#L209

Here is an example of a url that triggers this issue:

`https://test.com/?originalQueryString=%3FshowAll%3Dtrue`

When passed from an API Gateway the url passes through correctly:

`https://test.com/?originalQueryString=%3FshowAll%3Dtrue`

But if the original source was an ALB you get this instead:

`https://test.com/?originalQueryString=%253FshowAll%253Dtrue`

Thanks in advance.


Discussion / hints:

Fix patch:
diff --git a/lambda-http/Cargo.toml b/lambda-http/Cargo.toml
index d9a85484..d4b6167a 100644
--- a/lambda-http/Cargo.toml
+++ b/lambda-http/Cargo.toml
@@ -36,6 +36,7 @@ query_map = { version = "0.5", features = ["url-query"] }
 mime = "0.3.16"
 encoding_rs = "0.8.31"
 url = "2.2.2"
+percent-encoding = "2.2.0"
 
 [dependencies.aws_lambda_events]
 version = "^0.6.3"
diff --git a/lambda-http/src/request.rs b/lambda-http/src/request.rs
index 27d54cf2..978c3b0c 100644
--- a/lambda-http/src/request.rs
+++ b/lambda-http/src/request.rs
@@ -20,6 +20,7 @@ use serde::Deserialize;
 use serde_json::error::Error as JsonError;
 use std::future::Future;
 use std::pin::Pin;
+use std::str::FromStr;
 use std::{io::Read, mem};
 use url::Url;
 
@@ -201,22 +202,25 @@ fn into_alb_request(alb: AlbTargetGroupRequest) -> http::Request<Body> {
     let host = alb.headers.get(http::header::HOST).and_then(|s| s.to_str().ok());
     let raw_path = alb.path.unwrap_or_default();
 
+    let query_string_parameters = decode_query_map(alb.query_string_parameters);
+    let multi_value_query_string_parameters = decode_query_map(alb.multi_value_query_string_parameters);
+
     let builder = http::Request::builder()
         .uri(build_request_uri(
             &raw_path,
             &alb.headers,
             host,
-            Some((&alb.multi_value_query_string_parameters, &alb.query_string_parameters)),
+            Some((&multi_value_query_string_parameters, &query_string_parameters)),
         ))
         .extension(RawHttpPath(raw_path))
         // 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
         .extension(QueryStringParameters(
-            if alb.multi_value_query_string_parameters.is_empty() {
-                alb.query_string_parameters
+            if multi_value_query_string_parameters.is_empty() {
+                query_string_parameters
             } else {
-                alb.multi_value_query_string_parameters
+                multi_value_query_string_parameters
             },
         ))
         .extension(RequestContext::Alb(alb.request_context));
@@ -243,6 +247,12 @@ fn into_alb_request(alb: AlbTargetGroupRequest) -> http::Request<Body> {
     req
 }
 
+fn decode_query_map(query_map: QueryMap) -> QueryMap {
+    let query_string = query_map.to_query_string();
+    let decoded = percent_encoding::percent_decode(query_string.as_bytes()).decode_utf8_lossy();
+    QueryMap::from_str(&decoded).unwrap_or_default()
+}
+
 #[cfg(feature = "apigw_websockets")]
 fn into_websocket_request(ag: ApiGatewayWebsocketProxyRequest) -> http::Request<Body> {
     let http_method = ag.http_method;
@@ -548,6 +558,34 @@ mod tests {
         );
     }
 
+    #[test]
+    fn deserializes_alb_request_encoded_query_parameters_events() {
+        // from the docs
+        // https://docs.aws.amazon.com/elasticloadbalancing/latest/application/lambda-functions.html#multi-value-headers
+        let input = include_str!("../tests/data/alb_request_encoded_query_parameters.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.method(), "GET");
+        assert_eq!(
+            req.uri(),
+            "https://lambda-846800462-us-east-2.elb.amazonaws.com/?myKey=%3FshowAll%3Dtrue"
+        );
+
+        // Ensure this is an ALB request
+        let req_context = req.request_context();
+        assert!(
+            matches!(req_context, RequestContext::Alb(_)),
+            "expected Alb context, got {:?}",
+            req_context
+        );
+    }
+
     #[test]
     fn deserializes_apigw_multi_value_request_events() {
         // from docs
@@ -593,6 +631,28 @@ mod tests {
         );
     }
 
+    #[test]
+    fn deserializes_alb_multi_value_request_encoded_query_parameters_events() {
+        // from docs
+        // https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-lambda-proxy-integrations.html#api-gateway-simple-proxy-for-lambda-input-format
+        let input = include_str!("../tests/data/alb_multi_value_request_encoded_query_parameters.json");
+        let result = from_str(input);
+        assert!(
+            result.is_ok(),
+            "event is was not parsed as expected {:?} given {}",
+            result,
+            input
+        );
+        let request = result.expect("failed to parse request");
+        assert!(!request.query_string_parameters().is_empty());
+
+        // test RequestExt#query_string_parameters does the right thing
+        assert_eq!(
+            request.query_string_parameters().all("myKey"),
+            Some(vec!["?showAll=true", "?showAll=false"])
+        );
+    }
+
     #[test]
     fn deserialize_apigw_http_sam_local() {
         // manually generated from AWS SAM CLI
diff --git a/lambda-runtime-api-client/README.md b/lambda-runtime-api-client/README.md
index 530fefdd..2251cbc7 100644
--- a/lambda-runtime-api-client/README.md
+++ b/lambda-runtime-api-client/README.md
@@ -32,4 +32,4 @@ async fn main() -> Result<(), Error> {
 
     client.call(request).await
 }
-```
+```
\ No newline at end of file


Test patch:
diff --git a/lambda-http/tests/data/alb_multi_value_request_encoded_query_parameters.json b/lambda-http/tests/data/alb_multi_value_request_encoded_query_parameters.json
new file mode 100644
index 00000000..246e1de8
--- /dev/null
+++ b/lambda-http/tests/data/alb_multi_value_request_encoded_query_parameters.json
@@ -0,0 +1,37 @@
+{
+  "requestContext": {
+      "elb": {
+          "targetGroupArn": "arn:aws:elasticloadbalancing:region:123456789012:targetgroup/my-target-group/6d0ecf831eec9f09"
+      }
+  },
+  "httpMethod": "GET",
+  "path": "/",
+  "queryStringParameters": { "myKey": "%3FshowAll%3Dtrue" },
+  "multiValueQueryStringParameters": { "myKey": ["%3FshowAll%3Dtrue", "%3FshowAll%3Dfalse"] },
+  "headers": {
+      "accept": "text/html,application/xhtml+xml",
+      "accept-language": "en-US,en;q=0.8",
+      "content-type": "text/plain",
+      "cookie": "name1=value1",
+      "host": "lambda-846800462-us-east-2.elb.amazonaws.com",
+      "user-agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6)",
+      "x-amzn-trace-id": "Root=1-5bdb40ca-556d8b0c50dc66f0511bf520",
+      "x-forwarded-for": "72.21.198.66",
+      "x-forwarded-port": "443",
+      "x-forwarded-proto": "https"
+  },
+  "multiValueHeaders": {
+      "accept": ["text/html,application/xhtml+xml"],
+      "accept-language": ["en-US,en;q=0.8"],
+      "content-type": ["text/plain"],
+      "cookie":  ["name1=value1", "name2=value2"],
+      "host": ["lambda-846800462-us-east-2.elb.amazonaws.com"],
+      "user-agent": ["Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6)"],
+      "x-amzn-trace-id": ["Root=1-5bdb40ca-556d8b0c50dc66f0511bf520"],
+      "x-forwarded-for": ["72.21.198.66"],
+      "x-forwarded-port": ["443"],
+      "x-forwarded-proto": ["https"]
+  },
+  "isBase64Encoded": false,
+  "body": "request_body"
+}
\ No newline at end of file
diff --git a/lambda-http/tests/data/alb_request_encoded_query_parameters.json b/lambda-http/tests/data/alb_request_encoded_query_parameters.json
new file mode 100644
index 00000000..d8e1b452
--- /dev/null
+++ b/lambda-http/tests/data/alb_request_encoded_query_parameters.json
@@ -0,0 +1,24 @@
+{
+  "requestContext": {
+      "elb": {
+          "targetGroupArn": "arn:aws:elasticloadbalancing:region:123456789012:targetgroup/my-target-group/6d0ecf831eec9f09"
+      }
+  },
+  "httpMethod": "GET",
+  "path": "/",
+  "queryStringParameters": { "myKey": "%3FshowAll%3Dtrue"},
+  "headers": {
+      "accept": "text/html,application/xhtml+xml",
+      "accept-language": "en-US,en;q=0.8",
+      "content-type": "text/plain",
+      "cookie": "cookies",
+      "host": "lambda-846800462-us-east-2.elb.amazonaws.com",
+      "user-agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6)",
+      "x-amzn-trace-id": "Root=1-5bdb40ca-556d8b0c50dc66f0511bf520",
+      "x-forwarded-for": "72.21.198.66",
+      "x-forwarded-port": "443",
+      "x-forwarded-proto": "https"
+  },
+  "isBase64Encoded": false,
+  "body": "request_body"
+}
\ No newline at end of file