Skip to content

Commit

Permalink
Fix invalid query string for array params
Browse files Browse the repository at this point in the history
  • Loading branch information
akadusei committed Aug 11, 2024
1 parent 157ec4f commit 76aadf7
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 11 deletions.
10 changes: 10 additions & 0 deletions spec/lucky/base_http_client_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ require "../spec_helper"
class HelloWorldAction < TestAction
accepted_formats [:plain_text]

param codes : Array(String)?

post "/hello" do
plain_text "world"
end
Expand Down Expand Up @@ -46,6 +48,13 @@ describe Lucky::BaseHTTPClient do
request = TestServer.last_request
request.body.to_s.should eq({foo: "bar"}.to_json)
end

it "works with array query params" do
response = MyClient.new.exec HelloWorldAction.with(codes: ["ab", "xy"])

request = TestServer.last_request
request.query.should eq("codes[]=ab&codes[]=xy")
end
end

describe "with a Lucky::RouteHelper" do
Expand Down Expand Up @@ -134,6 +143,7 @@ describe Lucky::BaseHTTPClient do
request.method.should eq("HEAD")
request.path.should eq "hello"
end

it "works without params" do
response = MyClient.new.head(path: "hello")

Expand Down
43 changes: 32 additions & 11 deletions src/lucky/routable.cr
Original file line number Diff line number Diff line change
Expand Up @@ -298,19 +298,23 @@ module Lucky::Routable
{{ param.gsub(/^\?:/, "").id }},
{% end %}
)
query_params = {} of String => String

query_params = Hash(String, String | Array(String)).new

{% for param in PARAM_DECLARATIONS %}
# add query param if given and not nil
query_params["{{ param.var }}"] = {{ param.var }}.to_s unless {{ param.var }}.nil?
_param = {{ param.var }}

unless _param.nil?
if _param.is_a?(Array)
query_params["{{ param.var }}"] = _param.map(&.to_s)
else
query_params["{{ param.var }}"] = _param.to_s
end
end
{% end %}
unless query_params.empty?
io << '?'
{% if compare_versions(Crystal::VERSION, "1.10.0") < 0 %}
{% @type.warning("[Deprecated] Please update your Crystal version #{Crystal::VERSION}. Using Lucky with a version below 1.10.0 is deprecated.") %}
io << HTTP::Params.encode(query_params)
{% else %}
HTTP::Params.encode(io, query_params)
{% end %}

query_params.each_with_index do |tuple, i|
build_query_params(io, tuple[0], tuple[1], i == 0)
end

anchor.try do |value|
Expand Down Expand Up @@ -400,6 +404,23 @@ module Lucky::Routable

path.presence || "/"
end

private def self.build_query_params(io, key, values : Array, first)
values.each_with_index do |value, i|
io << (first && i == 0 ? '?' : '&')
URI.encode_www_form(key, io)
io << "[]"
io << '='
URI.encode_www_form(value, io) if value
end
end

private def self.build_query_params(io, key, value : String, first)
io << (first ? '?' : '&')
URI.encode_www_form(key, io)
io << '='
URI.encode_www_form(value, io) if value
end
end

macro included
Expand Down

0 comments on commit 76aadf7

Please sign in to comment.