diff --git a/Gemfile b/Gemfile index b0918ddb..5ccd6995 100644 --- a/Gemfile +++ b/Gemfile @@ -21,7 +21,7 @@ group :test do gem 'json', '>= 1.8.1', :platforms => [:jruby, :rbx, :ruby_18, :ruby_19] gem 'mime-types', '~> 1.25', :platforms => [:jruby, :ruby_18] gem 'rspec', '>= 2.14' - gem 'rubocop', '>= 0.19', :platforms => [:ruby_19, :ruby_20, :ruby_21] + gem 'rubocop', '~> 0.19.0', :platforms => [:ruby_19, :ruby_20, :ruby_21] gem 'simplecov', :require => false gem 'yardstick' end diff --git a/lib/http.rb b/lib/http.rb index dc0c6eaf..f4c8460f 100644 --- a/lib/http.rb +++ b/lib/http.rb @@ -9,7 +9,7 @@ require 'http/response' require 'http/response/body' require 'http/response/parser' -require 'http/backports' if RUBY_VERSION < '1.9.0' +require 'http/backports' # HTTP should be easy module HTTP diff --git a/lib/http/backports.rb b/lib/http/backports.rb index d1d517ea..2f61bb67 100644 --- a/lib/http/backports.rb +++ b/lib/http/backports.rb @@ -1,2 +1,2 @@ -require 'http/backports/uri' -require 'http/backports/base64' +require 'http/backports/uri' if RUBY_VERSION < '1.9.3' +require 'http/backports/base64' if RUBY_VERSION < '1.9.0' diff --git a/lib/http/client.rb b/lib/http/client.rb index 34671339..18862c17 100644 --- a/lib/http/client.rb +++ b/lib/http/client.rb @@ -19,49 +19,27 @@ def initialize(default_options = {}) end # Make an HTTP request - def request(verb, uri, options = {}) - opts = @default_options.merge(options) + def request(verb, uri, opts = {}) + opts = @default_options.merge(opts) + uri = make_request_uri(uri, opts) headers = opts.headers - proxy = opts.proxy + proxy = opts.proxy + body = make_request_body(opts, headers) - request_body = make_request_body(opts, headers) - uri, opts = normalize_get_params(uri, opts) if verb == :get + req = HTTP::Request.new(verb, uri, headers, proxy, body) + res = perform req, opts - uri = "#{uri}?#{URI.encode_www_form(opts.params)}" if opts.params && !opts.params.empty? - - request = HTTP::Request.new(verb, uri, headers, proxy, request_body) - perform request, opts - end - - # Perform the HTTP request (following redirects if needed) - def perform(req, options) - res = perform_without_following_redirects req, options - - if options.follow - res = Redirector.new(options.follow).perform req, res do |request| - perform_without_following_redirects request, options + if opts.follow + res = Redirector.new(opts.follow).perform req, res do |request| + perform request, opts end end res end - # Read a chunk of the body - def readpartial(size = BUFFER_SIZE) - return unless @socket - - read_more size - chunk = @parser.chunk - - finish_response if @parser.finished? - - chunk - end - - private - # Perform a single (no follow) HTTP request - def perform_without_following_redirects(req, options) + def perform(req, options) # finish previous response if client was re-used # TODO: this is pretty wrong, as socket shoud be part of response # connection, so that re-use of client will not break multiple @@ -90,6 +68,20 @@ def perform_without_following_redirects(req, options) res end + # Read a chunk of the body + def readpartial(size = BUFFER_SIZE) + return unless @socket + + read_more size + chunk = @parser.chunk + + finish_response if @parser.finished? + + chunk + end + + private + # Initialize TLS connection def start_tls(socket, options) # TODO: abstract away SSLContexts so we can use other TLS libraries @@ -100,6 +92,18 @@ def start_tls(socket, options) socket end + # Merges query params if needed + def make_request_uri(uri, options) + uri = URI uri.to_s unless uri.is_a? URI + + if options.params && !options.params.empty? + params = CGI.parse(uri.query.to_s).merge(options.params || {}) + uri.query = URI.encode_www_form params + end + + uri + end + # Create the request body object to send def make_request_body(opts, headers) if opts.body @@ -128,17 +132,5 @@ def read_more(size) rescue EOFError false end - - # Moves uri get params into the opts.params hash - # @return [Array] - def normalize_get_params(uri, opts) - uri = URI(uri) unless uri.is_a?(URI) - if uri.query - extracted_params_from_uri = Hash[URI.decode_www_form(uri.query)] - opts = opts.with_params(extracted_params_from_uri.merge(opts.params || {})) - uri.query = nil - end - [uri, opts] - end end end diff --git a/spec/http/backports/uri_spec.rb b/spec/http/backports/uri_spec.rb new file mode 100644 index 00000000..a295cf27 --- /dev/null +++ b/spec/http/backports/uri_spec.rb @@ -0,0 +1,9 @@ +require 'spec_helper' + +describe URI do + describe '.encode_www_form' do + it 'properly encodes arrays' do + expect(URI.encode_www_form :a => [:b, :c]).to eq 'a=b&a=c' + end + end +end diff --git a/spec/http/client_spec.rb b/spec/http/client_spec.rb index 9d4abcc1..027b0882 100644 --- a/spec/http/client_spec.rb +++ b/spec/http/client_spec.rb @@ -2,7 +2,7 @@ describe HTTP::Client do StubbedClient = Class.new(HTTP::Client) do - def perform_without_following_redirects(request, options) + def perform(request, options) stubs.fetch(request.uri.to_s) { super(request, options) } end @@ -70,27 +70,48 @@ def simple_response(body, status = 200) end describe 'parsing params' do + let(:client) { HTTP::Client.new } + before { allow(client).to receive :perform } + it 'accepts params within the provided URL' do - client = HTTP::Client.new - allow(client).to receive(:perform) expect(HTTP::Request).to receive(:new) do |_, uri| - params = CGI.parse(URI(uri).query) - expect(params).to eq('foo' => ['bar']) + expect(CGI.parse uri.query).to eq('foo' => %w[bar]) end client.get('http://example.com/?foo=bar') end it 'combines GET params from the URI with the passed in params' do - client = HTTP::Client.new - allow(client).to receive(:perform) expect(HTTP::Request).to receive(:new) do |_, uri| - params = CGI.parse(URI(uri).query) - expect(params).to eq('foo' => ['bar'], 'baz' => ['quux']) + expect(CGI.parse uri.query).to eq('foo' => %w[bar], 'baz' => %w[quux]) end client.get('http://example.com/?foo=bar', :params => {:baz => 'quux'}) end + + it 'merges duplicate values' do + expect(HTTP::Request).to receive(:new) do |_, uri| + expect(CGI.parse uri.query).to eq('a' => %w[1 2]) + end + + client.get('http://example.com/?a=1', :params => {:a => 2}) + end + + it 'does not modifies query part if no params were given' do + expect(HTTP::Request).to receive(:new) do |_, uri| + expect(uri.query).to eq 'deadbeef' + end + + client.get('http://example.com/?deadbeef') + end + + it 'does not corrupts index-less arrays' do + expect(HTTP::Request).to receive(:new) do |_, uri| + expect(CGI.parse uri.query).to eq 'a[]' => %w[b c], 'd' => %w[e] + end + + client.get('http://example.com/?a[]=b&a[]=c', :params => {:d => 'e'}) + end end describe 'passing json' do