Add ApiGatewayHttpApiV2ProxyRequestTranslator and ApiGatewayProxyRequestTranslator #1901

gcbeattyAWS · 2024-12-09T15:12:39Z

DOTNET-7838

Description of changes:

Add functions to translate httpcontext into APIGatewayHttpApiV2ProxyRequest and APIGatewayProxyRequest
Add unit tests for above classes

Testing details

Wrote unit tests
integration tests

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/ApiGatewayHttpApiV2ProxyRequestTranslator.cs

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/ApiGatewayProxyRequestTranslator.cs

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/ApiGatewayRequestTranslatorFactory.cs

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/IRouteConfigurationParser.cs

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/HttpContextExtensions.cs

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/HttpContextExtensions.cs

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ExceptionExtensions.cs

normj

I don't see any tests for special characters in headers, resource paths or query strings. Have we done a comparison yet with API Gateway? We should have tests for those cases. That includes the RawPath property as well.

gcbeattyAWS · 2024-12-11T01:18:24Z

I don't see any tests for special characters in headers, resource paths or query strings. Have we done a comparison yet with API Gateway? We should have tests for those cases. That includes the RawPath property as well.

so i did some tests of checking the existing functionality of api gateway in production, with special characters in the header by

deploying an api gateway
deploying a lambda
attaching the lambda to the api gatway
invoking the api gateway via command line (curl), and sending a non url encoded header value of special+++
logging the event (apigatewayrequest) object in the lambda itself, and it prints out special+++ (not encoded), so i assumed no encoding is required for that - but let me double check on this and get back to you

normj · 2024-12-11T01:20:24Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Utilities/HttpRequestUtility.cs

+        if (string.IsNullOrEmpty(contentType))
+            return false;
+
+        return contentType.StartsWith("image/", StringComparison.OrdinalIgnoreCase) ||


We have this list of content types we use for response in the ASP.NET Core bridge library. https://github.com/aws/aws-lambda-dotnet/blob/master/Libraries/src/Amazon.Lambda.AspNetCoreServer/AbstractAspNetCoreFunction.cs#L59

We should also think about how this can be configurable. Probably via environment variables that we can have users configure in Aspire.

normj · 2024-12-11T01:21:53Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/HttpContextExtensions.cs

+            Headers = headers,
+            QueryStringParameters = queryStringParameters,
+            PathParameters = pathParameters ?? new Dictionary<string, string>(),
+            Body = HttpRequestUtility.ReadRequestBody(request),


Can you take note that when we get these ToAPIGatewayXXX() methods hooked up to the main code path that is asking for the APIGatewayHttpApiV2ProxyRequest. When we convert the APIGatewayHttpApiV2ProxyRequest to the MemoryStream to Lambda we should check the size of the stream is no more then 6MB. If so throw back the same error that users would get from API Gateway.

yeah i will connect with phil on where he wants to do this logic at

normj · 2024-12-11T01:29:56Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/HttpContextExtensions.cs

+            MultiValueHeaders = multiValueHeaders,
+            QueryStringParameters = queryStringParameters,
+            MultiValueQueryStringParameters = multiValueQueryStringParameters,
+            PathParameters = pathParameters ?? new Dictionary<string, string>(),


Why are we initializing this to an empty collection if we don't have any. Is this what API Gateway does?

good catch. i just did a test
Invoke-WebRequest -Uri https://myapi-gateway-url.amazonaws.com/testfunction -Method GET
where it echos the api gateway request object and i see the value is null when there are no path parameters.

pathParameters: null, stageVariables: null, body: null, isBase64Encoded: false }

i will update the code

ill also double check the query string and header default values

ive updated the logic to handle this now

normj · 2024-12-11T01:33:06Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Utilities/HttpRequestUtility.cs

+
+        foreach (var header in headers)
+        {
+            singleValueHeaders[header.Key] = header.Value.Last() ?? "";


My memory for API Gateway is that in HTTP V2 where there is just a single headers collection the headers are comma delimited. But we need to verify that. I'm sure it isn't just get the last value.

yeah you are right on this https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-lambda.html

Format 2.0 doesn't have multiValueHeaders or multiValueQueryStringParameters fields. Duplicate headers are combined with commas and included in the headers field. Duplicate query strings are combined with commas and included in the queryStringParameters field.

i think i got confused when reading the other docs for v1 https://aws.amazon.com/blogs/compute/support-for-multi-value-parameters-in-amazon-api-gateway/

Before this change, API Gateway used to retain only the last value and drop everything else for a multi-valued parameter. You can see the original behavior in the queryStringParameters parameter in the above input, where only the “fish” value is retained.

I will update accordingly

ive updated the logic to handle this now. this logic will still just return the dicts as-is but then i have the caller perform the comma separated merging.

normj · 2024-12-11T01:34:39Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Utilities/HttpRequestUtility.cs

+
+        foreach (var param in query)
+        {
+            singleValueParams[param.Key] = param.Value.Last() ?? "";


Same comment as above for ExtractHeaders

ive updated the logic to handle this now

normj · 2024-12-11T01:57:33Z

4. invoking the api gateway via command line (curl), and sending a non url encoded header value of special+++

@gcbeattyAWS Also check more characters then + like spaces and other white space and Unicode characters. resource path and query strings can also be special. Especially when you through /, & and `?' in the values for resource path segments and query string values. In the ASP.NET Core Lambda bridge library where I have to do all of this in reverse I had to do some encoding handling.

aws-lambda-dotnet/Libraries/src/Amazon.Lambda.AspNetCoreServer/Internal/Utilities.cs

Line 148 in 3596a58

    
           internal static string CreateQueryStringParameters(IDictionary<string, string> singleValues, IDictionary<string, IList<string>> multiValues, bool urlEncodeValue)

gcbeattyAWS · 2024-12-12T17:08:22Z

invoking the api gateway via command line (curl), and sending a non url encoded header value of special+++

@gcbeattyAWS Also check more characters then + like spaces and other white space and Unicode characters. resource path and query strings can also be special. Especially when you through /, & and `?' in the values for resource path segments and query string values. In the ASP.NET Core Lambda bridge library where I have to do all of this in reverse I had to do some encoding handling.

aws-lambda-dotnet/Libraries/src/Amazon.Lambda.AspNetCoreServer/Internal/Utilities.cs

Line 148 in 3596a58

internal static string CreateQueryStringParameters(IDictionary<string, string> singleValues, IDictionary<string, IList<string>> multiValues, bool urlEncodeValue)

so i have updated the logic based on my findings:1

For query strings. QueryStringParameters should be the decoded values (this already happens automatically when accessing Request.QueryString. RawQueryString should be the encoded query string without the ?.
Headers do not seem to have any additional logic of decoding that i have seen. they are just a pass-through to lambda.
The path and path parameters should be decoded (this is also happening automatically now by accessing via request.Path)
The api gatway cannot handle %2f in the urls ("/") character

i have added test cases to check all of these scenarios

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/HttpContextExtensions.cs

...ambdaTestTool-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/HttpContextExtensionsTests.cs

gcbeattyAWS · 2024-12-19T16:54:58Z

pushing what i have so far

...ambdaTestTool-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/HttpContextExtensionsTests.cs

philasmar · 2024-12-19T17:20:06Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/HttpContextExtensions.cs

+        var (_, allHeaders) = HttpRequestUtility.ExtractHeaders(request.Headers);
+        var headers = allHeaders.ToDictionary(
+            kvp => kvp.Key,
+            kvp => string.Join(", ", kvp.Value)


nit: small inconsistency between headers and query params. Here you are joining on ", " and below on ","

this is intentional. from my testing the headers to have a space in them but query string params do not

philasmar · 2024-12-19T17:23:00Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/HttpContextExtensions.cs

+
+        if (!headers.ContainsKey("content-length"))
+        {
+            headers["content-length"] = contentLength.ToString();


Shouldn't we always update the content-length since we are updating the body?

so i checked this an apparently rest does not set this by default.

Edit: that was for the v1 actually.

i can still update this to always set it. let me double check

i checked on this. it needs to be the original content-length which is why i dont need to re-set it. my integration test fails (the real api gateway expects the original content length)

philasmar · 2024-12-19T17:25:17Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/HttpContextExtensions.cs

+                {
+                    Method = request.Method,
+                    Path = request.Path.Value, // this should be decoded value
+                    Protocol = !string.IsNullOrEmpty(request.Protocol) ? request.Protocol : "HTTP/1.1", // defaults to http 1.1 if not provided


should we make the "HTTP/1.1" assumption? This is more likely to get updated and we wouldn't know to update it

ive put this here so it matches api gateway

philasmar · 2024-12-19T17:31:43Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/HttpContextExtensions.cs

+        var (headers, multiValueHeaders) = HttpRequestUtility.ExtractHeaders(request.Headers);
+        var (queryStringParameters, multiValueQueryStringParameters) = HttpRequestUtility.ExtractQueryStringParameters(request.Query);
+
+        if (!headers.ContainsKey("content-length"))


Shouldn't we always update the content-length since we are updating the body?

yea thats a good point. it doesnt hurt to always update it

looks like rest api doesnt set this automatically by default

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Utilities/HttpRequestUtility.cs

philasmar · 2024-12-19T17:42:34Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Utilities/HttpRequestUtility.cs

+    /// X-Custom-Header: value1
+    /// 
+    /// The method will return:
+    /// singleValueHeaders: { "Accept": "application/xhtml+xml", "X-Custom-Header": "value1" }


Why are we taking the last occurrence of Accept? Is that a valid assumption?

So when api gateway receives a request with headers

Accept: a Accept: b

the single value header would be Accept: b

However, in .net headers are always sent to api gateway like

Accept: a, b

and singleValue header would be Accept: a, b.

so depending what we want to do we can either do the intended way by api gateway, or do `, '.join(header.value) for the singelvalue header and mimic what the value would be if coming from .net.

This same issue applies for multivalueheaders

when api gateway receives a request with headers

Accept: a Accept: b

multi value headers would be `accept: ['a', 'b']

but when sending from .net as

Accept: a, b

multivalueheadeders would be `accept: ['a, b'] (the string)

philasmar · 2024-12-19T17:42:51Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Utilities/HttpRequestUtility.cs

+    /// For query string: ?param1=value1&amp;param2=value2&amp;param2=value3
+    /// 
+    /// The method will return:
+    /// singleValueParams: { "param1": "value1", "param2": "value3" }


same question here

philasmar · 2024-12-19T17:47:06Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Utilities/HttpRequestUtility.cs

+    /// The generated ID is a 145character string consisting of lowercase letters and numbers, followed by an equals sign.
+    public static string GenerateRequestId()
+    {
+        return Guid.NewGuid().ToString("N").Substring(0, 8) + Guid.NewGuid().ToString("N").Substring(0, 7) + "=";


Suggested change

return Guid.NewGuid().ToString("N").Substring(0, 8) + Guid.NewGuid().ToString("N").Substring(0, 7) + "=";

return $"{Guid.NewGuid().ToString("N").Substring(0, 8)} {Guid.NewGuid().ToString("N").Substring(0, 7)}=";

philasmar · 2024-12-19T17:48:50Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Utilities/RouteTemplateUtility.cs

+    /// // parameters will contain: { {"id", "123"}, {"orderId", "456"} }
+    /// </code>
+    /// </example>
+    public static Dictionary<string, string> ExtractPathParameters(string routeTemplate, string actualPath)


This does not take into account the greedy variable {proxy+}

this should be fixed now

philasmar · 2024-12-19T17:49:02Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Utilities/RouteTemplateUtility.cs

+    /// // defaults will contain: { {"version", "v1"} }
+    /// </code>
+    /// </example>
+    public static RouteValueDictionary GetDefaults(RouteTemplate parsedTemplate)


This is not needed

gcbeattyAWS · 2024-12-20T20:20:11Z

...ambdaTestTool-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/HttpContextExtensionsTests.cs

+        }
+
+        [Fact]
+        public async Task BinaryContentHttpV1()


we still need an integration test for the rest api for binary content. it was taking too long to setup though because i think need to create a new lambda, api gateway with binary types configured and all that. i can create a backlog item to add this in next pr

gcbeattyAWS · 2024-12-20T20:37:01Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/HttpContextExtensions.cs

+        {
+            var encodedPathParameters = pathParameters.ToDictionary(
+                    kvp => kvp.Key,
+                    kvp => Uri.EscapeUriString(kvp.Value)); // intentionally using EscapeURiString over EscapeDataString since EscapeURiString correctly handles reserved characters :/?#[]@!$&'()*+,;= in this case


for example. escapedatastring encodes * as %20 which is different than how api gateway rest api does it (it keeps * as *)

gcbeattyAWS · 2024-12-20T20:45:52Z

...aTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/Extensions/HttpContextExtensionsTests.cs

+        [Theory]
+        [InlineData(ApiGatewayEmulatorMode.Rest)]
+        [InlineData(ApiGatewayEmulatorMode.HttpV1)]
+        public async Task ToApiGateway_MultiValueHeader(ApiGatewayEmulatorMode emulatorMode)


i manually tested these unit tests

gcbeattyAWS · 2024-12-20T20:48:03Z

...estTool-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/ApiGatewayIntegrationTestFixture.cs

@@ -16,17 +16,39 @@ public class ApiGatewayIntegrationTestFixture : IAsyncLifetime
        public ApiGatewayTestHelper ApiGatewayTestHelper { get; private set; }

        public string StackName { get; private set; }
-        public string RestApiId { get; private set; }


this is just renaming

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Utilities/RouteTemplateUtility.cs

gcbeattyAWS commented Dec 9, 2024

View reviewed changes

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/ApiGatewayHttpApiV2ProxyRequestTranslator.cs Outdated Show resolved Hide resolved

gcbeattyAWS requested a review from philasmar December 9, 2024 15:17

gcbeattyAWS marked this pull request as ready for review December 9, 2024 15:17

gcbeattyAWS requested a review from normj December 9, 2024 15:45

philasmar requested changes Dec 9, 2024

View reviewed changes

gcbeattyAWS force-pushed the gcbeatty/apiparse branch from 471264d to 2e94f8b Compare December 9, 2024 17:26

gcbeattyAWS commented Dec 9, 2024

View reviewed changes

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/IRouteConfigurationParser.cs Outdated Show resolved Hide resolved

gcbeattyAWS added the Release Not Needed Add this label if a PR does not need to be released. label Dec 10, 2024

gcbeattyAWS commented Dec 10, 2024

View reviewed changes

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/HttpContextExtensions.cs Outdated Show resolved Hide resolved

gcbeattyAWS marked this pull request as draft December 10, 2024 19:23

gcbeattyAWS force-pushed the gcbeatty/apiparse branch from f996e5b to 77f7679 Compare December 10, 2024 19:31

gcbeattyAWS changed the base branch from feature/lambdatesttool-v2 to asmarp/api-gateway-emulator-skeleton December 10, 2024 19:31

gcbeattyAWS marked this pull request as ready for review December 10, 2024 19:37

gcbeattyAWS commented Dec 10, 2024

View reviewed changes

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/HttpContextExtensions.cs Show resolved Hide resolved

gcbeattyAWS commented Dec 10, 2024

View reviewed changes

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/ExceptionExtensions.cs Outdated Show resolved Hide resolved

gcbeattyAWS requested a review from philasmar December 10, 2024 20:40

normj reviewed Dec 11, 2024

View reviewed changes

normj requested changes Dec 11, 2024

View reviewed changes

gcbeattyAWS requested a review from normj December 12, 2024 17:15

gcbeattyAWS commented Dec 12, 2024

View reviewed changes

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Extensions/HttpContextExtensions.cs Show resolved Hide resolved

gcbeattyAWS marked this pull request as draft December 18, 2024 00:19

gcbeattyAWS force-pushed the gcbeatty/apiparse branch from 1080c5c to 1d8f81d Compare December 19, 2024 16:08

gcbeattyAWS changed the base branch from asmarp/api-gateway-emulator-skeleton to gcbeatty/responseparsing December 19, 2024 16:09

gcbeattyAWS commented Dec 19, 2024

View reviewed changes

...ambdaTestTool-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/HttpContextExtensionsTests.cs Show resolved Hide resolved

gcbeattyAWS commented Dec 19, 2024

View reviewed changes

...ambdaTestTool-v2/tests/Amazon.Lambda.TestTool.IntegrationTests/HttpContextExtensionsTests.cs Show resolved Hide resolved

philasmar requested changes Dec 19, 2024

View reviewed changes

gcbeattyAWS force-pushed the gcbeatty/apiparse branch from 1d8f81d to 356b0cc Compare December 20, 2024 20:15

gcbeattyAWS added 2 commits December 20, 2024 15:17

Add request parsing into api gateway

9d38917

PR comments

d96af57

gcbeattyAWS force-pushed the gcbeatty/apiparse branch from 356b0cc to d96af57 Compare December 20, 2024 20:17

gcbeattyAWS changed the base branch from gcbeatty/responseparsing to feature/lambdatesttool-v2 December 20, 2024 20:17

gcbeattyAWS marked this pull request as ready for review December 20, 2024 20:17

gcbeattyAWS commented Dec 20, 2024

View reviewed changes

gcbeattyAWS requested a review from philasmar December 20, 2024 20:32

gcbeattyAWS commented Dec 20, 2024

View reviewed changes

normj reviewed Dec 20, 2024

View reviewed changes

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Utilities/RouteTemplateUtility.cs Outdated Show resolved Hide resolved

normj approved these changes Dec 21, 2024

View reviewed changes

philasmar approved these changes Dec 21, 2024

View reviewed changes

Update comment

c3bf7cd

gcbeattyAWS merged commit 68ff1db into feature/lambdatesttool-v2 Dec 23, 2024
4 checks passed

gcbeattyAWS deleted the gcbeatty/apiparse branch December 23, 2024 22:11

	return Guid.NewGuid().ToString("N").Substring(0, 8) + Guid.NewGuid().ToString("N").Substring(0, 7) + "=";
	return $"{Guid.NewGuid().ToString("N").Substring(0, 8)} {Guid.NewGuid().ToString("N").Substring(0, 7)}=";

Add ApiGatewayHttpApiV2ProxyRequestTranslator and ApiGatewayProxyRequestTranslator #1901

Add ApiGatewayHttpApiV2ProxyRequestTranslator and ApiGatewayProxyRequestTranslator #1901

Conversation

gcbeattyAWS commented Dec 9, 2024 • edited Loading

normj left a comment • edited Loading

Choose a reason for hiding this comment

gcbeattyAWS commented Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcbeattyAWS Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcbeattyAWS Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcbeattyAWS Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

normj commented Dec 11, 2024

gcbeattyAWS commented Dec 12, 2024 • edited Loading

gcbeattyAWS commented Dec 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcbeattyAWS Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

gcbeattyAWS Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcbeattyAWS Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcbeattyAWS commented Dec 9, 2024 •

edited

Loading

normj left a comment •

edited

Loading

gcbeattyAWS commented Dec 11, 2024 •

edited

Loading

gcbeattyAWS Dec 12, 2024 •

edited

Loading

gcbeattyAWS Dec 12, 2024 •

edited

Loading

gcbeattyAWS Dec 12, 2024 •

edited

Loading

gcbeattyAWS commented Dec 12, 2024 •

edited

Loading

gcbeattyAWS Dec 19, 2024 •

edited

Loading

gcbeattyAWS Dec 20, 2024 •

edited

Loading

gcbeattyAWS Dec 19, 2024 •

edited

Loading