Skip to content

Commit

Permalink
Don't print query params if they are empty
Browse files Browse the repository at this point in the history
I found this while investigating this issue on Lucky:
luckyframework/lucky#214

It turns out that because I parsed the query params in that handler it
set the URI#query to `""`. This caused `HTTP::Request#resource` to add
an extra `?` to the end of the path, even though the query params were
empty.

This PR fixes the issue by also checking for an empty string before
printing `?`
  • Loading branch information
paulcsmith committed Dec 1, 2017
1 parent 1978c8b commit 326aa17
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
20 changes: 14 additions & 6 deletions spec/std/uri_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,20 @@ describe "URI" do
assert_uri("/foo?q=1", path: "/foo", query: "q=1")
assert_uri("mailto:[email protected]", scheme: "mailto", path: nil, opaque: "[email protected]")

it { URI.parse("http://www.example.com/foo").full_path.should eq("/foo") }
it { URI.parse("http://www.example.com").full_path.should eq("/") }
it { URI.parse("http://www.example.com/foo?q=1").full_path.should eq("/foo?q=1") }
it { URI.parse("http://www.example.com/?q=1").full_path.should eq("/?q=1") }
it { URI.parse("http://www.example.com?q=1").full_path.should eq("/?q=1") }
it { URI.parse("http://test.dev/a%3Ab").full_path.should eq("/a%3Ab") }
describe "full_path" do
it { URI.parse("http://www.example.com/foo").full_path.should eq("/foo") }
it { URI.parse("http://www.example.com").full_path.should eq("/") }
it { URI.parse("http://www.example.com/foo?q=1").full_path.should eq("/foo?q=1") }
it { URI.parse("http://www.example.com/?q=1").full_path.should eq("/?q=1") }
it { URI.parse("http://www.example.com?q=1").full_path.should eq("/?q=1") }
it { URI.parse("http://test.dev/a%3Ab").full_path.should eq("/a%3Ab") }

it "does not add '?' to the end if the query params are empty" do
uri = URI.parse("http://www.example.com/foo")
uri.query = ""
uri.full_path.should eq("/foo")
end
end

describe "normalize" do
it "removes dot notation from path" do
Expand Down
2 changes: 1 addition & 1 deletion src/uri.cr
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class URI
def full_path
String.build do |str|
str << (@path.try { |p| !p.empty? } ? @path : "/")
str << "?" << @query if @query
str << "?" << @query if (query = @query) && !query.empty?
end
end

Expand Down

0 comments on commit 326aa17

Please sign in to comment.