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

convert query values to strings before comparison #517

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/webmock/matchers/hash_including_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ def initialize(expected)
end

def ==(actual)
@expected.all? {|k,v| actual.has_key?(k) && v === actual[k]}
@expected.all? do |k,v|
actual_value = actual.respond_to?(:has_key?) ? actual[k] : actual
actual_value = WebMock::Util::QueryValueStringifier.stringify(actual_value)
actual.has_key?(k) && WebMock::Util::QueryValueStringifier.stringify(v) === actual_value
end

rescue NoMethodError
false
end
Expand Down
74 changes: 44 additions & 30 deletions lib/webmock/util/query_mapper.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require "webmock/util/query_value_stringifier"

module WebMock::Util
class QueryMapper
class << self
Expand Down Expand Up @@ -174,39 +176,15 @@ def fill_accumulator_for_subscript(accumulator, key, value)
#
# @param [Hash, #to_hash, Array] new_query_values The new query values.
def values_to_query(new_query_values, options = {})
options[:notation] ||= :subscript
return if new_query_values.nil?
options[:notation] ||= :subscript

unless new_query_values.is_a?(Array)
unless new_query_values.respond_to?(:to_hash)
raise TypeError,
"Can't convert #{new_query_values.class} into Hash."
end
new_query_values = new_query_values.to_hash
new_query_values = new_query_values.inject([]) do |object, (key, value)|
key = key.to_s if key.is_a?(::Symbol) || key.nil?
if value.is_a?(Array)
value.each { |v| object << [key.to_s + '[]', v] }
elsif value.is_a?(Hash)
value.each { |k, v| object << ["#{key.to_s}[#{k}]", v]}
else
object << [key.to_s, value]
end
object
end
# Useful default for OAuth and caching.
# Only to be used for non-Array inputs. Arrays should preserve order.
begin
new_query_values.sort! # may raise for non-comparable values
rescue NoMethodError, ArgumentError
# ignore
end
end
query_array = convert_query_hash_to_array(new_query_values)

buffer = ''
new_query_values.each do |parent, value|
query_array.each do |parent, value|
encoded_parent = ::Addressable::URI.encode_component(
parent.dup, ::Addressable::URI::CharacterClasses::UNRESERVED
parent.dup, ::Addressable::URI::CharacterClasses::UNRESERVED
)
buffer << "#{to_query(encoded_parent, value, options)}&"
end
Expand Down Expand Up @@ -245,7 +223,10 @@ def to_query(parent, value, options = {})
when ::Hash
value = value.map do |key, val|
[
::Addressable::URI.encode_component(key.to_s.dup, ::Addressable::URI::CharacterClasses::UNRESERVED),
::Addressable::URI.encode_component(
key.to_s.dup,
::Addressable::URI::CharacterClasses::UNRESERVED
),
val
]
end
Expand All @@ -270,7 +251,40 @@ def to_query(parent, value, options = {})
"#{parent}=#{encoded_value}"
end
end
end

private

def convert_query_hash_to_array(new_query_values)
return new_query_values if new_query_values.is_a?(Array)

unless new_query_values.respond_to?(:to_hash)
raise TypeError,
"Can't convert #{new_query_values.class} into Hash."
end

new_query_values = new_query_values.to_hash.inject([]) do |object, (key, value)|
key = key.to_s if key.is_a?(::Symbol) || key.nil?

case value = QueryValueStringifier.stringify(value)
when Array
value.each { |v| object << [key.to_s + '[]', v] }
when Hash
value.each { |k, v| object << ["#{key.to_s}[#{k}]", v]}
else
object << [key.to_s, value]
end

object
end

# Useful default for OAuth and caching.
# Only to be used for non-Array inputs. Arrays should preserve order.
new_query_values.sort! # may raise for non-comparable values
new_query_values
rescue NoMethodError, ArgumentError
new_query_values
end

end
end
end
22 changes: 22 additions & 0 deletions lib/webmock/util/query_value_stringifier.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module WebMock::Util
class QueryValueStringifier
class << self
def stringify(value)
case value
when String, NilClass
value
when Array
value.map { |v| stringify(v) }
when Hash
Hash[value.map { |k, v| [k, stringify(v)] }]
when Integer, TrueClass, FalseClass, Symbol
value.to_s
when WebMock::Matchers::AnyArgMatcher, RSpec::Mocks::ArgumentMatchers::AnyArgMatcher
value
else
value
end
end
end
end
end
16 changes: 16 additions & 0 deletions spec/acceptance/shared/stubbing_requests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,22 @@
end

describe "based on query params" do
it "converts hash keys in the query body into strings" do
stub_request(:get, "www.example.com").with(:query => {:a => [5, "c d"]}).to_return(:body => "abc")
expect(http_request(:get, "http://www.example.com/?a[]=5&a[]=c%20d").body).to eq("abc")
end

it "converts hash values in the query body into strings" do
stub_request(:get, "www.example.com").with(:query => {"a" => [5, "c d"]}).to_return(:body => "abc")
expect(http_request(:get, "http://www.example.com/?a[]=5&a[]=c%20d").body).to eq("abc")

stub_request(:get, "www.example.com").with(:query => {"a" => [:j, "c d"]}).to_return(:body => "abc")
expect(http_request(:get, "http://www.example.com/?a[]=j&a[]=c%20d").body).to eq("abc")

stub_request(:get, "www.example.com").with(:query => {"a" => [true, "c d"]}).to_return(:body => "abc")
expect(http_request(:get, "http://www.example.com/?a[]=true&a[]=c%20d").body).to eq("abc")
end

it "should return stubbed response when stub declares query params as a hash" do
stub_request(:get, "www.example.com").with(:query => {"a" => ["b x", "c d"]}).to_return(:body => "abc")
expect(http_request(:get, "http://www.example.com/?a[]=b+x&a[]=c%20d").body).to eq("abc")
Expand Down
27 changes: 27 additions & 0 deletions spec/hash_including_bug_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
require "spec_helper"
begin
require "httparty"
rescue LoadError
@skip_specs = true
end

unless @skip_specs
RSpec.describe "Hash including Bug" do
let!(:dummy_url) { 'http://dummyurl.com' }

it "receive a request" do
stub_request(:get, dummy_url).
with(:query => hash_including({ :param1 => 5 })).
to_return(:body => 'body 1')

expect(
HTTParty.get(dummy_url, {
:query => {
:param1 => 5,
:param2 => 'random1'
}
}).body
).to eq 'body 1'
end
end
end
35 changes: 35 additions & 0 deletions spec/unit/util/query_value_stringifier_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
require "spec_helper"

RSpec.describe WebMock::Util::QueryValueStringifier do
describe ".stringify" do
it "handles strings" do
expect(described_class.stringify("a")).to eq("a")
end

it "handles integers" do
expect(described_class.stringify(1)).to eq("1")
end

it "handles booleans" do
expect(described_class.stringify(true)).to eq("true")
end

it "handles symbols" do
expect(described_class.stringify(:hello)).to eq("hello")
end

it "handles hashes" do
input = { :a => :a, :b => :b, :c => 1, :d => [true, false] }
expect(described_class.stringify(input)).to eq(
{ :a => "a", :b => "b", :c => "1", :d => ["true", "false"] }
)
end

it "handles arrays" do
input = [ :a, 1, [true, false], {"a" => :b} ]
expect(described_class.stringify(input)).to eq(
[ "a", "1", ["true", "false"], {"a" => "b"} ]
)
end
end
end
1 change: 1 addition & 0 deletions webmock.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Gem::Specification.new do |s|
s.add_development_dependency 'excon', '>= 0.27.5'
s.add_development_dependency 'minitest', '~> 5.0.0'
s.add_development_dependency 'rdoc', ((RUBY_VERSION == '1.8.6') ? '<= 3.5.0' : '>3.5.0')
s.add_development_dependency 'httparty' if RUBY_VERSION >= "1.9.3"

s.files = `git ls-files`.split("\n")
s.test_files = `git ls-files -- {test,spec,features}/*`.split("\n")
Expand Down