rails-lambda / lamby

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

Don't set percent encoded queries into Rack Environment's `QUERY_STRING` #157

Open ma2gedev opened 1 year ago

ma2gedev commented 1 year ago

Hi, thanks for making a nice gem!

I noticed Lamby does not set percent encoded queries into Rack environment's QUERY_STRING at this here. My application receives percent decoded queries set in ::Rack::QUERY_STRING. As a result, my app handles double percent decoded query parameters.

For example, inputting example.com/path?xxx=testquery%2B%3D into browser, then ::Rack::QUERY_STRING on Lamby is percent decode like the following:

"QUERY_STRING": "xxx=testquery+="

On the other hand, QUERY_STRING is like the following on Puma:

"QUERY_STRING": "xxx=testquery%2B%3D"

In my opinion, Lamby should sets percent encoded queries into ::Rack::QUERY_STRING like other rack web servers(Puma or Passenger, etc). Rack specification does not say details about QUERY_STRING. However it expects to set percent encoded queries because PATH_INFO may be percent-encoded.

https://github.com/rack/rack/blob/main/SPEC.rdoc

Though this may cause breaking changes, so should also consider to provide a config to change behavior.

Environment

My porject uses Lamby with combination of AWS Lambda integrataion and API Gateway REST API.

Expected Behavior

::Rack::QUERY_STRING application received is percent-encoded.

Actual Behavior

::Rack::QUERY_STRING application received is not percent-encoded.

Steps to Reproduce

References

metaskills commented 1 year ago

Thanks for the thoughtful issue report. I would be interested in gathering data on how all proxies work. Mostly focusing on REST, HTTP, and Function URLs. Sometimes it is possible they get it wrong on the raw data we have access too. I think we have a test for &[]color=blue&[]color=red too. Using that info, we can write tests, configs, changes etc.