Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Dates passed through body in a_request method aren't correctly compared #216

Closed
wants to merge 25 commits into from

3 participants

@fltiago

When dates are passed as parameters in body of the method a_request and are compared in method matching_hashes? they are in different formats, even being generated in the same way. This happen because when the JSON.parse do the conversion of a date the format is modified and the comparison in method matching_hashes? doesn't consider it, comparing peer-to-peer.

PS: Sorry about my mistake, submitting this pull request in portuguese. :P

myronmarston and others added some commits
@myronmarston myronmarston Add failing spec demonstrating em-http-request bug.
When a response modifying middleware is used with em-http-request, and a real request is made, the middleware can modify the response before the after_request is invoked. This prevents VCR from being able to record the response accurately.  The after_request hook should be invoked before the em-http-request middleware.

Related to myronmarston/vcr#169.
3fb913d
@myronmarston myronmarston Add a failing spec demonstrating a bug in the em-http-request adapter.
When a request is made to a URL that returns a 3xx response and the
:redirects option is set, the globally_stub_request/after_request
hooks are not paired properly.  Both hooks should receive the original
request and the redirect-following request.

This spec should probably be re-written to use the local webmock
server, but I couldn't figure out how to get it to conditionally
send a redirect response since it writes directly to the socket
and doesn't (as far as I can tell) have the request info available
in that scope...so there's not easy way to have it send a different
response for different requests :(.

See myronmarston/vcr#171 for the original VCR issue that caused
me to investigate this bug.
a42d463
@bblimke Merge branch 'master' into em_http_middleware_after_request_bug f40be82
@bblimke Merge branch 'master' into vcr-171 dc0e226
@bblimke Fixed em-http-adapter bug. When a request is made to a URL that retur…
…ns a 3xx response and the :redirects option is set, the globally_stub_request/after_request

hooks are now fired for the original request and the redirect-following request.
806818f
@bblimke Fixed problem with Net::HTTP::DigestAuth. All constants are set back …
…on original Net::HTTP, after WebMock is disabled, if any constants were added to Net::HTTP after WebMock was enabled.
1acfa58
Pawel Pierzchala Fixes problem with failing em-http-request with queries 2c92af6
@bblimke Merge pull request #210 from wrozka/master
em-http-request problems with queries
b7ebbab
@bblimke Ensured that all em-http-request specs are executed for em-http-reque…
…st 0.3.
f38d04f
@bblimke Made spec compatible with em-http-request 0.x adapter f86bdde
@bblimke Redirects support is only working for em-http-request 1.0.x 4036430
@bblimke Version 1.8.10 623cf85
@bblimke Fixed Net::HTTP adapter spec to be compatible with ruby 1.0 123974b
@bblimke Use blocking requests in Excon specs to pass on Jruby 86a0158
@myronmarston myronmarston Fix excon adapter to handle :body => some_file_object. f1a3293
@bblimke Merge pull request #213 from bblimke/fix_excon_file_uploads
Fix excon adapter to handle :body => some_file_object.
9915698
@bblimke Version 1.8.11 5662ca4
@fltiago fltiago Adding method to compare two dates using json format. 8f01af0
@fltiago fltiago Adding condition in matching_hashes to change the way that two dates …
…are compared
19670eb
@bblimke Merge pull request #212 from i0rek/typhoeus_0.5.0
Typhoeus 0.5.0
720f151
@bblimke Dependency on Typhoeus > 0.5 d52d0d4
@bblimke Upgrade to version 1.9.0 e2c2e27
@fltiago fltiago Adding method to compare two dates using json format. 93e73d9
@fltiago fltiago Adding condition in matching_hashes to change the way that two dates …
…are compared
153c57a
@fltiago fltiago Merge branch 'master' of github.com:fltiago/webmock 7e34b7b
@fltiago fltiago closed this
@bblimke
Owner

Is that not an issue anymore?

@fltiago

It is. But I merged from master (bblimke/webmock) to my branch master (fltiago/webmock) to take the new commits, because I'm using my branch in my project, and a lot of commit that already had here were added to the pull. So, I will redo it with an branch that have the same name of my issue.

Do you have any other suggestions?

@bblimke
Owner

Ok. Looking forward to the new pull request :)

I was very busy recently, sorry for the delay with merging it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 15, 2012
  1. @myronmarston

    Add failing spec demonstrating em-http-request bug.

    myronmarston authored
    When a response modifying middleware is used with em-http-request, and a real request is made, the middleware can modify the response before the after_request is invoked. This prevents VCR from being able to record the response accurately.  The after_request hook should be invoked before the em-http-request middleware.
    
    Related to myronmarston/vcr#169.
Commits on May 25, 2012
  1. @myronmarston

    Add a failing spec demonstrating a bug in the em-http-request adapter.

    myronmarston authored
    When a request is made to a URL that returns a 3xx response and the
    :redirects option is set, the globally_stub_request/after_request
    hooks are not paired properly.  Both hooks should receive the original
    request and the redirect-following request.
    
    This spec should probably be re-written to use the local webmock
    server, but I couldn't figure out how to get it to conditionally
    send a redirect response since it writes directly to the socket
    and doesn't (as far as I can tell) have the request info available
    in that scope...so there's not easy way to have it send a different
    response for different requests :(.
    
    See myronmarston/vcr#171 for the original VCR issue that caused
    me to investigate this bug.
Commits on Aug 26, 2012
  1. Fixed em-http-adapter bug. When a request is made to a URL that retur…

    authored
    …ns a 3xx response and the :redirects option is set, the globally_stub_request/after_request
    
    hooks are now fired for the original request and the redirect-following request.
  2. Fixed problem with Net::HTTP::DigestAuth. All constants are set back …

    authored
    …on original Net::HTTP, after WebMock is disabled, if any constants were added to Net::HTTP after WebMock was enabled.
Commits on Sep 6, 2012
  1. Fixes problem with failing em-http-request with queries

    Pawel Pierzchala authored
Commits on Sep 9, 2012
  1. Merge pull request #210 from wrozka/master

    authored
    em-http-request problems with queries
  2. Version 1.8.10

    authored
Commits on Sep 29, 2012
  1. @myronmarston
  2. Merge pull request #213 from bblimke/fix_excon_file_uploads

    authored
    Fix excon adapter to handle :body => some_file_object.
  3. Version 1.8.11

    authored
Commits on Oct 11, 2012
  1. @fltiago
  2. @fltiago
Commits on Nov 7, 2012
  1. Merge pull request #212 from i0rek/typhoeus_0.5.0

    authored
    Typhoeus 0.5.0
  2. Dependency on Typhoeus > 0.5

    authored
  3. Upgrade to version 1.9.0

    authored
Commits on Nov 19, 2012
  1. @fltiago
  2. @fltiago
  3. @fltiago
This page is out of date. Refresh to see the latest.
View
27 CHANGELOG.md
@@ -1,5 +1,32 @@
# Changelog
+## 1.9.0
+
+* Added support for Typhoeus >= 0.5.0 and removed support for Typhoeus < 0.5.0.
+
+ Thanks to [Hans Hasselberg](https://github.com/i0rek)
+
+## 1.8.11
+
+* Fix excon adapter to handle `:body => some_file_object`
+
+ Thanks to [Myron Marston](https://github.com/myronmarston)
+
+## 1.8.10
+
+* em-http-request fix. After request callbacks are correctly invoked for 3xx responses,
+ when :redirects option is set.
+
+ Thanks to [Myron Marston](https://github.com/myronmarston) for reporting that issue.
+
+* Fixed compatibility with Net::HTTP::DigestAuth
+
+ Thanks to [Jonathan Hyman](https://github.com/jonhyman) for reporting that issue.
+
+* Fixed problem in em-http-request 0.x appending the query to the client URI twice.
+
+ Thanks to [Paweł Pierzchała](https://github.com/wrozka)
+
## 1.8.9
* Fixed problem with caching nil responses when the same HTTPClient instance is used.
View
1  Gemfile
@@ -10,7 +10,6 @@ group :development do
gem 'rake'
gem 'guard-rspec'
gem 'rb-fsevent'
- gem "typhoeus", git: "git@github.com:typhoeus/typhoeus.git"
end
group :test do
View
1  README.md
@@ -713,6 +713,7 @@ People who submitted patches and new features or suggested improvements. Many th
* Kevin Glowacz
* Hans Hasselberg
* Andrew France
+* Jonathan Hyman
For a full list of contributors you can visit the
[contributors](https://github.com/bblimke/webmock/contributors) page.
View
2  Rakefile
@@ -27,7 +27,7 @@ end
task :em_http_request_0_x_spec do
- sh "EM_HTTP_REQUEST_0_X=true bundle install && bundle exec rspec spec/acceptance/em_http_request/em_http_request_spec.rb" if RUBY_VERSION <= "1.8.7"
+ sh "EM_HTTP_REQUEST_0_X=true bundle install && EM_HTTP_REQUEST_0_X=true bundle exec rspec spec/acceptance/em_http_request/em_http_request_spec.rb" if RUBY_VERSION <= "1.8.7"
end
require 'rake/testtask'
View
1  lib/webmock.rb
@@ -15,6 +15,7 @@
require 'webmock/util/hash_keys_stringifier'
require 'webmock/util/json'
require 'webmock/util/version_checker'
+require 'webmock/util/date_comparator'
require 'webmock/matchers/hash_including_matcher'
View
4 lib/webmock/http_lib_adapters/em_http_request/em_http_request_0_x.rb
@@ -90,11 +90,11 @@ def build_request_signature
if @req
options = @req.options
method = @req.method
- uri = @req.uri
+ uri = @req.uri.dup
else
options = @options
method = @method
- uri = @uri
+ uri = @uri.dup
end
if options[:authorization] || options['authorization']
View
6 lib/webmock/http_lib_adapters/em_http_request/em_http_request_1_x.rb
@@ -108,14 +108,16 @@ def send_request(head, body)
end
end
- def set_deferred_status(status, *args)
- if status == :succeeded && !stubbed_webmock_response && WebMock::CallbackRegistry.any_callbacks?
+ def unbind(reason = nil)
+ if !stubbed_webmock_response && WebMock::CallbackRegistry.any_callbacks?
webmock_response = build_webmock_response
WebMock::CallbackRegistry.invoke_callbacks(
{:lib => :em_http_request, :real_request => true},
request_signature,
webmock_response)
end
+ @request_signature = nil
+ remove_instance_variable(:@stubbed_webmock_response)
super
end
View
11 lib/webmock/http_lib_adapters/excon_adapter.rb
@@ -43,7 +43,16 @@ def self.build_request(params)
method = (params.delete(:method) || :get).to_s.downcase.to_sym
params[:query] = to_query(params[:query]) if params[:query].is_a?(Hash)
uri = Addressable::URI.new(params).to_s
- WebMock::RequestSignature.new method, uri, :body => params[:body], :headers => params[:headers]
+ WebMock::RequestSignature.new method, uri, :body => body_from(params), :headers => params[:headers]
+ end
+
+ def self.body_from(params)
+ body = params[:body]
+ return body unless body.respond_to?(:read)
+
+ contents = body.read
+ body.rewind if body.respond_to?(:rewind)
+ contents
end
def self.real_response(mock)
View
12 lib/webmock/http_lib_adapters/net_http.rb
@@ -28,6 +28,16 @@ def self.disable!
Net.send(:const_set, :HTTP, OriginalNetHTTP)
Net.send(:const_set, :HTTPSession, OriginalNetHTTP)
Net.send(:const_set, :BufferedIO, OriginalNetBufferedIO)
+
+ #copy all constants from @webMockNetHTTP to original Net::HTTP
+ #in case any constants were added to @webMockNetHTTP instead of Net::HTTP
+ #after WebMock was enabled.
+ #i.e Net::HTTP::DigestAuth
+ @webMockNetHTTP.constants.each do |constant|
+ if !OriginalNetHTTP.constants.map(&:to_s).include?(constant.to_s)
+ OriginalNetHTTP.send(:const_set, constant, @webMockNetHTTP.const_get(constant))
+ end
+ end
end
@webMockNetHTTP = Class.new(Net::HTTP) do
@@ -56,7 +66,7 @@ def const_get(name, inherit=true)
if Module.method(:constants).arity != 0
def constants(inherit=true)
- super + self.superclass.constants(inherit)
+ (super + self.superclass.constants(inherit)).uniq
end
end
end
View
10 lib/webmock/request_pattern.rb
@@ -237,7 +237,11 @@ def matching_hashes?(query_parameters, pattern)
if actual.is_a?(Hash) && expected.is_a?(Hash)
return false unless matching_hashes?(actual, expected)
else
- return false unless expected === actual
+ if is_kind_of_date?(expected)
+ return false unless WebMock::Util::DateComparator.compare(actual, expected)
+ else
+ return false unless expected === actual
+ end
end
end
true
@@ -251,6 +255,10 @@ def normalize_hash(hash)
Hash[WebMock::Util::HashKeysStringifier.stringify_keys!(hash).sort]
end
+ def is_kind_of_date?(date)
+ date.is_a?(Date) || date.is_a?(DateTime) || date.is_a?(Time)
+ end
+
end
class HeadersPattern
View
9 lib/webmock/util/date_comparator.rb
@@ -0,0 +1,9 @@
+module WebMock
+ module Util
+ class Util::DateComparator
+ def self.compare(date1, date2)
+ date1.to_json === date2.to_json
+ end
+ end
+ end
+end
View
2  lib/webmock/version.rb
@@ -1,3 +1,3 @@
module WebMock
- VERSION = '1.8.9' unless defined?(::WebMock::VERSION)
+ VERSION = '1.9.0' unless defined?(::WebMock::VERSION)
end
View
78 spec/acceptance/em_http_request/em_http_request_spec.rb
@@ -13,6 +13,39 @@
#functionality only supported for em-http-request 1.x
if defined?(EventMachine::HttpConnection)
+ context 'when a real request is made and redirects are followed' do
+ before { WebMock.allow_net_connect! }
+
+ # This url redirects to the https URL.
+ let(:http_url) { "http://raw.github.com:80/gist/fb555cb593f3349d53af/6921dd638337d3f6a51b0e02e7f30e3c414f70d6/vcr_gist" }
+ let(:https_url) { http_url.gsub('http', 'https').gsub('80', '443') }
+
+ def make_request
+ EM.run do
+ request = EM::HttpRequest.new(http_url).get(:redirects => 1)
+ request.callback { EM.stop }
+ end
+ end
+
+ it "invokes the globally_stub_request hook with both requests" do
+ urls = []
+ WebMock.globally_stub_request { |r| urls << r.uri.to_s; nil }
+
+ make_request
+
+ urls.should eq([http_url, https_url])
+ end
+
+ it 'invokes the after_request hook with both requests' do
+ urls = []
+ WebMock.after_request { |req, res| urls << req.uri.to_s }
+
+ make_request
+
+ urls.should eq([http_url, https_url])
+ end
+ end
+
describe "with middleware" do
it "should work with request middleware" do
@@ -38,19 +71,21 @@ def request(client, head, body)
end
end
- it "should work with response middleware" do
- stub_request(:get, "www.example.com").to_return(:body => 'foo')
-
- middleware = Class.new do
+ let(:response_middleware) do
+ Class.new do
def response(resp)
resp.response = 'bar'
end
end
+ end
+
+ it "should work with response middleware" do
+ stub_request(:get, "www.example.com").to_return(:body => 'foo')
EM.run do
conn = EventMachine::HttpRequest.new('http://www.example.com/')
- conn.use middleware
+ conn.use response_middleware
http = conn.get
@@ -60,6 +95,36 @@ def response(resp)
end
end
end
+
+ let(:webmock_server_url) { "http://#{WebMockServer.instance.host_with_port}/" }
+
+ shared_examples_for "em-http-request middleware/after_request hook integration" do
+ it 'yields the original raw body to the after_request hook even if a response middleware modifies the body' do
+ yielded_response_body = nil
+ ::WebMock.after_request do |request, response|
+ yielded_response_body = response.body
+ end
+
+ EM::HttpRequest.use response_middleware
+
+ EM.run do
+ http = EventMachine::HttpRequest.new(webmock_server_url).get
+ http.callback { EM.stop }
+ end
+
+ yielded_response_body.should eq("hello world")
+ end
+ end
+
+ context 'making a real request' do
+ before { WebMock.allow_net_connect! }
+ include_examples "em-http-request middleware/after_request hook integration"
+ end
+
+ context 'when the request is stubbed' do
+ before { stub_request(:get, webmock_server_url).to_return(:body => 'hello world') }
+ include_examples "em-http-request middleware/after_request hook integration"
+ end
end
# not pretty, but it works
@@ -174,7 +239,8 @@ def client(uri, options = {})
it "#request_signature doesn't mutate the original uri" do
subject.uri.should == Addressable::URI.parse("http://www.example.com/?a=1")
- subject.request_signature.uri.should == Addressable::URI.parse(uri)
+ signature = WebMock::RequestRegistry.instance.requested_signatures.hash.keys.first
+ signature.uri.should == Addressable::URI.parse(uri)
end
end
end
View
15 spec/acceptance/excon/excon_spec.rb
@@ -11,5 +11,20 @@
Excon.get('http://example.com', :path => "resource/", :query => {:a => 1, :b => 2}).body.should == "abc"
end
+ let(:file) { File.new(__FILE__) }
+ let(:file_contents) { File.new(__FILE__).read }
+
+ it 'handles file uploads correctly' do
+ stub_request(:put, "http://example.com/upload").with(:body => file_contents)
+
+ yielded_request_body = nil
+ WebMock.after_request do |req, res|
+ yielded_request_body = req.body
+ end
+
+ Excon.put("http://example.com", :path => "upload", :body => file)
+
+ yielded_request_body.should eq(file_contents)
+ end
end
View
2  spec/acceptance/excon/excon_spec_helper.rb
@@ -7,7 +7,7 @@ def http_request(method, uri, options = {}, &block)
uri = Addressable::URI.heuristic_parse(uri)
uri = uri.omit(:userinfo).to_s.gsub(' ', '+')
- options = options.merge(:method => method) # Dup and merge
+ options = options.merge(:method => method, :nonblock => false) # Dup and merge
response = Excon.new(uri).request(options, &block)
headers = WebMock::Util::Headers.normalize_headers(response.headers)
View
13 spec/acceptance/net_http/net_http_spec.rb
@@ -86,6 +86,19 @@ class TestMarshalingInWebMockNetHTTP
Object.const_get("Net").const_get("HTTP").constants(false).map(&:to_s).should include("Get")
end
end
+
+ describe "after WebMock is disabled" do
+ after(:each) do
+ WebMock.enable!
+ end
+ it "Net::HTTP should have the same constants" do
+ orig_consts_number = WebMock::HttpLibAdapters::NetHttpAdapter::OriginalNetHTTP.constants.size
+ Net::HTTP.send(:const_set, "TEST_CONST", 10)
+ Net::HTTP.constants.size.should == orig_consts_number + 1
+ WebMock.disable!
+ Net::HTTP.constants.size.should == orig_consts_number + 1
+ end
+ end
end
it "should work with block provided" do
View
11 spec/acceptance/shared/request_expectations.rb
@@ -1,3 +1,5 @@
+require 'json'
+
shared_context "request expectations" do |*adapter_info|
describe "when request expectations are set" do
describe "when net connect is not allowed" do
@@ -258,6 +260,15 @@
}.should_not raise_error
end
+ it "should satisfy expectation even if json had date in the content" do
+ body_hash['date'] = Date.today
+ lambda {
+ http_request(:post, "http://www.example.com/", :headers => {'Content-Type' => 'application/json'},
+ :body => body_hash.to_json)
+ a_request(:post, "www.example.com").with(:body => body_hash).should have_been_made
+ }.should_not raise_error
+ end
+
it "should satisfy expectation even if json body contains date string" do
lambda {
http_request(:post, "http://www.example.com/", :headers => {'Content-Type' => 'application/json'},
View
2  webmock.gemspec
@@ -23,7 +23,7 @@ Gem::Specification.new do |s|
s.add_development_dependency 'em-http-request', '>= 1.0.2'
s.add_development_dependency 'em-synchrony', '>= 1.0.0' if RUBY_VERSION >= "1.9"
s.add_development_dependency 'curb', '>= 0.8.0' unless RUBY_PLATFORM =~ /java/
- s.add_development_dependency 'typhoeus', '>= 0.3.3' unless RUBY_PLATFORM =~ /java/
+ s.add_development_dependency 'typhoeus', '>= 0.5.0' unless RUBY_PLATFORM =~ /java/
s.add_development_dependency 'excon', '>= 0.11.0'
s.add_development_dependency 'minitest', '>= 2.2.2'
s.add_development_dependency 'rdoc', ((RUBY_VERSION == '1.8.6') ? '<= 3.5.0' : '>3.5.0')
Something went wrong with that request. Please try again.