Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

About matrix parameter and URL decoding #440

Closed
slowvincy opened this issue Apr 11, 2019 · 10 comments
Closed

About matrix parameter and URL decoding #440

slowvincy opened this issue Apr 11, 2019 · 10 comments
Labels
feature-request A feature should be added or improved. service-api This issue is due to a problem in a service API, not the SDK implementation.

Comments

@slowvincy
Copy link

slowvincy commented Apr 11, 2019

Hi there, we have a case where we accept matrix parameter in our REST API like /v1/gangs;item={itemUrl}. But because this library does not recognize matrix parameter and also decodes the URL path when marshaling the API Gateway request (https://github.com/aws/aws-lambda-dotnet/blob/master/Libraries/src/Amazon.Lambda.AspNetCoreServer/APIGatewayProxyFunction.cs#L187), it results in the Request did not match any routes error in ASP.NET Core. The path must be breaking like /v1/gangs;item=https:/{restOfItemUrl}. I have verified either changing to query parameter or NOT decoding the URL path works. Would you please advise your idea on this issue and possible support for this case? Thanks.

@klaytaybai klaytaybai added feature-request A feature should be added or improved. service-api This issue is due to a problem in a service API, not the SDK implementation. labels Apr 12, 2019
@klaytaybai
Copy link

Hi @slowvincy, it is my understanding that this is not yet supported by API Gateway, so there is little we can do until they support it. I can help voice your feedback to the API Gateway team, but I would suggest contributing to the AWS Forums for API Gateway. I think the best thing would be to try to use query parameters instead.

@andymac4182
Copy link

@slowvincy Does ASP.NET Core support Matrix parameters?

@slowvincy
Copy link
Author

@andymac4182 When you do

[HttpGet("v1/[controller];item={itemUrl}")]
public Task<ActionResult> GetGangByItemAsync([FromRoute, Required] Uri itemUrl)

this ASP.NET Core 2.1 controller works properly and Swagger understands itemUrl is a path parameter.

@klaytaybai

  1. When you are using API Gateway Lambda Proxy Integration, API Gateway blindly passes on the request to Lambda so I don't think "API Gateway & Matrix parameter" is a relevant argument here.

  2. We would also like to understand why this decoding step is absolutely necessary https://github.com/aws/aws-lambda-dotnet/blob/master/Libraries/src/Amazon.Lambda.AspNetCoreServer/APIGatewayProxyFunction.cs#L187.

The bottom line is, if a request can be handled by ASP.NET Core controller it should be able to be handled through this library as well, which I believe is the intention of this library.

Please let me know if there is further detail I can provide. Thanks.

@andymac4182
Copy link

Ahh that makes sense.

Do you have the raw path request that was received from API Gateway before its decoded by the library? and some examples of the URLs with the values filled in.

@slowvincy
Copy link
Author

Yes, from API Gateway CloudWatch: HTTP Method: GET, Resource Path: /v1/gangs;item=https%3A%2F%2Fint-item-service.commerce.cimpress.io%2Fv1%2Fitems%2F8931cd91-cee0-4d54-9085-6a705cd77daa. Let me know if anything else would help.

@andymac4182
Copy link

Sorry I just got to read this. Do you have an example of the path that doesn't work. If I read the above correctly you said with the https breaking part it works?

@slowvincy
Copy link
Author

I'm not sure what you mean. The above URL doesn't work through the library because it got decoded to /v1/gangs;item=https://int-item-service.commerce.cimpress.io/v1/items/8931cd91-cee0-4d54-9085-6a705cd77daa. Please refer to explanation in my original post and feel free to reproduce it yourself. I believe I have provided all the information that is needed for you to support this issue. Thanks.

@slowvincy
Copy link
Author

Please kindly advise how I can escalate priority of this issue. Thanks.

@chrisoverzero
Copy link
Contributor

chrisoverzero commented Apr 25, 2019

This bug doesn't appear to be related to matrix parameters on API Gateway in any specific way -- it's only what surfaced it.

In Utilities.DecodeResourcePath, the Path property of IHttpRequestFeature is unescaped via WebUtility.UrlDecode. This method is a little too aggressive. We can see in Microsoft's implementation of Kestrel's URL decoder that the character sequence '%2F' skips unescaping when it's present in the path.

I intend to make a PR today/tomorrow which will, uh... Hm, double-escape '%2F' to '%252F' in Utilities.DecodeResourcePath before calling WebUtility.UrlDecode? I guess? 🤷‍♂️ I know it'll work, but if someone has a much smarter idea, please do tell me.

@kgaska
Copy link

kgaska commented Dec 20, 2019

Hi,

I came across similar issue but with query string encoding. I need to pass '&' as part of the query parameter value so I'm encoding it to '%26'. However it looks like it's automatically decoded during marshaling the request, so ASP.NET Core cannot properly read value of that query parameter. When I use double encoding it works correctly.

I'm considering that as a bug. Idea of Lambda Proxy Integration is to pass entire request as it is and let ASP.NET CORE pipeline to do the rest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. service-api This issue is due to a problem in a service API, not the SDK implementation.
Projects
None yet
Development

No branches or pull requests

5 participants