rails-lambda / lamby

🐑🛤 Simple Rails & AWS Lambda Integration
https://lamby.cloud
MIT License
602 stars 29 forks source link

Safely Pass Percent Symbols in Paths #170

Closed metaskills closed 1 year ago

metaskills commented 1 year ago

Here is an example of running local Rails under Puma with something like /test/dwef782jkif%3d in the path:

Started GET "/test/dwef782jkif%3d" for 172.19.0.1 at 2023-07-07 19:19:30 +0000
Processing by ApplicationController#test as HTML
  Parameters: {"path"=>"dwef782jkif="}
Completed 200 OK in 2ms (ActiveRecord: 0.0ms | Allocations: 113)

Prior to this change, here is what it looked like in Lambda with Lamby:

Started GET "/test/dwef782jkif=" for 98.166.4.233 at 2023-07-07 19:11:39 +0000
Processing by ApplicationController#test as HTML
  Parameters: {"path"=>"dwef782jkif="}
Completed 200 OK in 2ms (ActiveRecord: 0.0ms | Allocations: 114)

We learned the same lessons with query params, always trust Raw. Thankfully, HTTP v2 allows that to happen.

ℹ️ There are some folks that use AWS::ApiGatewayV2::Integration with API Gateway to proxy requests to AWS Services like Lambda, but the mapping request parameter tooling (https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-aws-services.html) does not provide a way to get to the raw value. RIP 🪦

metaskills commented 1 year ago

using lamby with path overwrites today

This feels real far off the beaten path. Was that pun bad? lol

On a serious note, what does that mean? Most folks use Lamby like we do, simple SAM/HTTPv2 proxy and when issues happen we get reports. I'm not sure how or why folks would write their own integration outside of SAM but assuming that is what you're talking about? If so, I'd be more worried about folks that wrote custom code in their Rails app to work around this vs filing an issue. Does that make sense?

drinks commented 1 year ago

This feels real far off the beaten path

Sure, totally valid.

On a serious note, what does that mean?

Just pointing out that I could deploy a lamby app as a standalone with no http layer in front of it, and use a lambda proxy integration to invoke it from a decoupled gateway. If I were applying path overwrites in such a context, they would have worked in the previous release, but would break in this one.

I think it's probably ok to simply note that in the Changelog, just pointing out the possibility.

metaskills commented 1 year ago

Thanks, I'll call out those in the changelog.