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

Fix invalid query string for array params #1907

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading