Fix tests, and fix a few straggling bugs.

This commit is contained in:
Andy Brody 2016-09-12 01:36:07 -04:00
parent 4381e9f9cd
commit e9df60cd11
6 changed files with 54 additions and 117 deletions

View File

@ -133,6 +133,7 @@ module RestClient
def initialize(
method:,
url:,
payload: nil,
headers: {},
params: {},
cookies: nil,
@ -145,7 +146,7 @@ module RestClient
read_timeout: :notprovided, open_timeout: :notprovided,
timeout: :notprovided,
ssl_client_cert: nil, ssl_client_key: nil, ssl_ca_file: nil,
ssl_ca_path: nil, ssl_cert_store: nil, ssl_verify_callback:
ssl_ca_path: nil, ssl_cert_store: :notprovided, ssl_verify_callback:
nil, ssl_verify_callback_warnings: nil, ssl_version: nil,
ssl_ciphers: nil,
before_execution_proc: nil)
@ -172,25 +173,20 @@ module RestClient
end
@method = normalize_method(method)
@headers = headers.dup.freeze # TODO don't dup/freeze
@headers = headers.dup.freeze
@url = process_url_params(normalize_url(url), params)
@user = @password = nil
parse_url_with_auth!(url, use_netrc: use_netrc)
parse_url_with_auth!(@url, use_netrc: use_netrc)
# process cookies
if cookies
@cookie_jar = process_cookie_args(@uri, cookies)
else
@cookie_jar = nil
end
@cookie_jar = process_cookie_args(@uri, cookies)
@payload = Payload.generate(payload)
@user = user if user
@password = password if password
@read_timeout = @open_timeout = nil
if timeout != :notprovided
@read_timeout = @open_timeout = timeout
end
@ -226,23 +222,23 @@ module RestClient
@ssl_client_key = ssl_client_key
@ssl_ca_file = ssl_ca_file
@ssl_ca_path = ssl_ca_path
@ssl_cert_store = ssl_cert_store
@ssl_cert_store = ssl_cert_store if ssl_cert_store != :notprovided
@ssl_version = ssl_version
@ssl_ciphers = ssl_ciphers
@ssl_verify_callback = ssl_verify_callback
@ssl_verify_callback_warnings = ssl_verify_callback_warnings
# If there's no CA file, CA path, or cert store provided, use default
if !ssl_ca_file && !ssl_ca_path && !ssl_cert_store
if !@ssl_ca_file && !@ssl_ca_path && !defined?(@ssl_cert_store)
@ssl_cert_store = self.class.default_ssl_cert_store
end
unless ssl_ciphers
unless @ssl_ciphers
# If we're on a Ruby version that has insecure default ciphers,
# override it with our default list.
if WeakDefaultCiphers.include?(
OpenSSL::SSL::SSLContext::DEFAULT_PARAMS.fetch(:ciphers))
@ssl_opts[:ciphers] = DefaultCiphers
@ssl_ciphers = DefaultCiphers
end
end
end
@ -287,7 +283,8 @@ module RestClient
when RestClient::ParamsArray
else
raise ArgumentError.new(
'params must be Hash or RestClient::ParamsArray')
'params must be Hash or RestClient::ParamsArray, got ' +
url_params.inspect)
end
# build resulting URL with query string
@ -318,8 +315,6 @@ module RestClient
def cookies
hash = {}
return nil unless @cookie_jar
@cookie_jar.cookies(uri).each do |c|
hash[c.name] = c.value
end
@ -748,14 +743,8 @@ module RestClient
warn('Try passing :verify_ssl => false instead.')
end
net.read_timeout = @read_timeout
if defined? @open_timeout
if @open_timeout == -1
warn 'Deprecated: to disable timeouts, please use nil instead of -1'
@open_timeout = nil
end
net.open_timeout = @open_timeout
end
net.read_timeout = @read_timeout if defined?(@read_timeout)
net.open_timeout = @open_timeout if defined?(@open_timeout)
RestClient.before_execution_procs.each do |before_proc|
before_proc.call(req, self)
@ -796,7 +785,7 @@ module RestClient
end
def setup_credentials(req)
req.basic_auth(user, password) if user && !headers.has_key?("Authorization")
req.basic_auth(user, password) if user && !@processed_headers.has_key?("Authorization")
end
def fetch_body(http_response)

View File

@ -17,6 +17,6 @@ module Helpers
def request_double(url: 'http://example.com', method: 'get')
double('request', url: url, uri: URI.parse(url), method: method,
user: nil, password: nil, cookie_jar: HTTP::CookieJar.new,
redirection_history: nil, args: {url: url, method: method})
redirection_history: nil, original_opts: {url: url, method: method})
end
end

View File

@ -28,10 +28,6 @@ describe RestClient::Request do
expect { request.execute }.to_not raise_error
end
# TODO: deprecate and remove RestClient::SSLCertificateNotVerified and just
# pass through OpenSSL::SSL::SSLError directly. See note in
# lib/restclient/request.rb.
#
# On OS X, this test fails since Apple has patched OpenSSL to always fall
# back on the system CA store.
it "is unsuccessful with an incorrect ca_file", :unless => RestClient::Platform.mac_mri? do
@ -40,7 +36,7 @@ describe RestClient::Request do
:url => 'https://www.mozilla.org',
:ssl_ca_file => File.join(File.dirname(__FILE__), "certs", "verisign.crt")
)
expect { request.execute }.to raise_error(RestClient::SSLCertificateNotVerified)
expect { request.execute }.to raise_error(OpenSSL::SSL::SSLError, /certificate verify failed/)
end
# On OS X, this test fails since Apple has patched OpenSSL to always fall
@ -51,7 +47,7 @@ describe RestClient::Request do
:url => 'https://www.mozilla.org',
:ssl_ca_path => File.join(File.dirname(__FILE__), "capath_verisign")
)
expect { request.execute }.to raise_error(RestClient::SSLCertificateNotVerified)
expect { request.execute }.to raise_error(OpenSSL::SSL::SSLError, /certificate verify failed/)
end
it "is successful using the default system cert store" do
@ -86,7 +82,7 @@ describe RestClient::Request do
:verify_ssl => true,
:ssl_verify_callback => lambda { |preverify_ok, store_ctx| false },
)
expect { request.execute }.to raise_error(RestClient::SSLCertificateNotVerified)
expect { request.execute }.to raise_error(OpenSSL::SSL::SSLError, /certificate verify failed/)
end
it "succeeds verification when the callback returns true",

View File

@ -5,29 +5,26 @@ describe RestClient::Request do
context 'params for GET requests' do
it "manage params for get requests" do
stub_request(:get, 'http://some/resource?a=b&c=d').with(:headers => {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate', 'Foo'=>'bar'}).to_return(:body => 'foo', :status => 200)
expect(RestClient::Request.execute(:url => 'http://some/resource', :method => :get, :headers => {:foo => :bar, :params => {:a => :b, 'c' => 'd'}}).body).to eq 'foo'
stub_request(:get, 'http://some/resource').with(:headers => {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate', 'Foo'=>'bar'}).to_return(:body => 'foo', :status => 200)
expect(RestClient::Request.execute(:url => 'http://some/resource', :method => :get, :headers => {:foo => :bar, :params => :a}).body).to eq 'foo'
expect(RestClient::Request.execute(:url => 'http://some/resource', :method => :get, :headers => {:foo => :bar}, :params => {:a => :b, 'c' => 'd'}).body).to eq 'foo'
end
it 'adds GET params when params are present in URL' do
stub_request(:get, 'http://some/resource?a=b&c=d').with(:headers => {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate', 'Foo'=>'bar'}).to_return(:body => 'foo', :status => 200)
expect(RestClient::Request.execute(:url => 'http://some/resource?a=b', :method => :get, :headers => {:foo => :bar, :params => {:c => 'd'}}).body).to eq 'foo'
expect(RestClient::Request.execute(:url => 'http://some/resource?a=b', :method => :get, :headers => {:foo => :bar}, :params => {:c => 'd'}).body).to eq 'foo'
end
it 'encodes nested GET params' do
stub_request(:get, 'http://some/resource?a[foo][]=1&a[foo][]=2&a[bar]&b=foo+bar&math=2+%2B+2+%3D%3D+4').with(:headers => {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate'}).to_return(:body => 'foo', :status => 200)
expect(RestClient::Request.execute(url: 'http://some/resource', method: :get, headers: {
params: {
a: {
foo: [1,2],
bar: nil,
},
b: 'foo bar',
math: '2 + 2 == 4',
}
}).body).to eq 'foo'
expect(RestClient::Request.execute(url: 'http://some/resource', method: :get,
params: {
a: {
foo: [1,2],
bar: nil,
},
b: 'foo bar',
math: '2 + 2 == 4',
}
).body).to eq 'foo'
end
end
@ -38,7 +35,7 @@ describe RestClient::Request do
response_value = http_response.body
end
stub_request(:get, 'http://some/resource?a=b&c=d').with(:headers => {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate', 'Foo'=>'bar'}).to_return(:body => 'foo', :status => 200)
RestClient::Request.execute(:url => 'http://some/resource', :method => :get, :headers => {:foo => :bar, :params => {:a => :b, 'c' => 'd'}}, :block_response => block)
RestClient::Request.execute(:url => 'http://some/resource', :method => :get, :headers => {:foo => :bar}, :params => {:a => :b, 'c' => 'd'}, :block_response => block)
expect(response_value).to eq "foo"
end

View File

@ -148,21 +148,13 @@ describe RestClient::Request, :include_helpers do
jar,
cookies_arr
].each do |cookies|
[true, false].each do |in_headers|
if in_headers
opts = {headers: {cookies: cookies}}
else
opts = {cookies: cookies}
end
request = RestClient::Request.new(method: :get, url: 'example.com', **opts)
expect(request).to receive(:default_headers).and_return({'Foo' => 'bar'})
expect(request.make_headers({})).to eq({'Foo' => 'bar', 'Cookie' => 'session_id=1; user_id=someone'})
expect(request.make_cookie_header).to eq 'session_id=1; user_id=someone'
expect(request.cookies).to eq({'session_id' => '1', 'user_id' => 'someone'})
expect(request.cookie_jar.cookies.length).to eq 2
expect(request.cookie_jar.object_id).not_to eq jar.object_id # make sure we dup it
end
request = RestClient::Request.new(method: :get, url: 'example.com', cookies: cookies)
expect(request).to receive(:default_headers).and_return({'Foo' => 'bar'})
expect(request.make_headers({})).to eq({'Foo' => 'bar', 'Cookie' => 'session_id=1; user_id=someone'})
expect(request.make_cookie_header).to eq 'session_id=1; user_id=someone'
expect(request.cookies).to eq({'session_id' => '1', 'user_id' => 'someone'})
expect(request.cookie_jar.cookies.length).to eq 2
expect(request.cookie_jar.object_id).not_to eq jar.object_id # make sure we dup it
end
# test with no cookies
@ -199,19 +191,10 @@ describe RestClient::Request, :include_helpers do
end
it 'rejects or warns with contradictory cookie options' do
# same opt in two different places
expect {
RestClient::Request.new(method: :get, url: 'example.com',
cookies: {bar: '456'},
headers: {cookies: {foo: '123'}})
}.to raise_error(ArgumentError, /Cannot pass :cookies in Request.*headers/)
# :cookies opt and Cookie header
[
{cookies: {foo: '123'}, headers: {cookie: 'foo'}},
{cookies: {foo: '123'}, headers: {'Cookie' => 'foo'}},
{headers: {cookies: {foo: '123'}, cookie: 'foo'}},
{headers: {cookies: {foo: '123'}, 'Cookie' => 'foo'}},
].each do |opts|
expect(fake_stderr {
RestClient::Request.new(method: :get, url: 'example.com', **opts)
@ -437,9 +420,9 @@ describe RestClient::Request, :include_helpers do
end
it "does not attempt to send credentials if Authorization header is set" do
@request.headers['Authorization'] = 'Token abc123'
allow(@request).to receive(:user).and_return('joe')
allow(@request).to receive(:password).and_return('mypass')
@request = RestClient::Request.new(method: :get, url: 'http://some/resource',
headers: {authorization: 'Token abc123'},
user: 'joe', password: 'mypass')
req = double("request")
expect(req).not_to receive(:basic_auth)
@request.send(:setup_credentials, req)
@ -740,34 +723,6 @@ describe RestClient::Request, :include_helpers do
@request.send(:transmit, @uri, 'req', nil)
end
it 'deprecated: warns when disabling timeout by setting it to -1' do
@request = RestClient::Request.new(:method => :put, :url => 'http://some/resource', :payload => 'payload', :read_timeout => -1)
allow(@http).to receive(:request)
allow(@request).to receive(:process_result)
allow(@request).to receive(:response_log)
expect(@net).to receive(:read_timeout=).with(nil)
expect(fake_stderr {
@request.send(:transmit, @uri, 'req', nil)
}).to match(/^Deprecated: .*timeout.* nil instead of -1$/)
end
it "deprecated: disable timeout by setting it to -1" do
@request = RestClient::Request.new(:method => :put, :url => 'http://some/resource', :payload => 'payload', :read_timeout => -1, :open_timeout => -1)
allow(@http).to receive(:request)
allow(@request).to receive(:process_result)
allow(@request).to receive(:response_log)
expect(@request).to receive(:warn)
expect(@net).to receive(:read_timeout=).with(nil)
expect(@request).to receive(:warn)
expect(@net).to receive(:open_timeout=).with(nil)
@request.send(:transmit, @uri, 'req', nil)
end
end
describe "ssl" do
@ -1228,24 +1183,24 @@ describe RestClient::Request, :include_helpers do
describe 'process_url_params' do
it 'should handle basic URL params' do
expect(@request.process_url_params('https://example.com/foo', params: {key1: 123, key2: 'abc'})).
expect(@request.process_url_params('https://example.com/foo', {key1: 123, key2: 'abc'})).
to eq 'https://example.com/foo?key1=123&key2=abc'
expect(@request.process_url_params('https://example.com/foo', params: {'key1' => 123})).
expect(@request.process_url_params('https://example.com/foo', {'key1' => 123})).
to eq 'https://example.com/foo?key1=123'
expect(@request.process_url_params('https://example.com/path',
params: {foo: 'one two', bar: 'three + four == seven'})).
{foo: 'one two', bar: 'three + four == seven'})).
to eq 'https://example.com/path?foo=one+two&bar=three+%2B+four+%3D%3D+seven'
end
it 'should combine with & when URL params already exist' do
expect(@request.process_url_params('https://example.com/path?foo=1', params: {bar: 2})).
expect(@request.process_url_params('https://example.com/path?foo=1', {bar: 2})).
to eq 'https://example.com/path?foo=1&bar=2'
end
it 'should handle complex nested URL params per Rack / Rails conventions' do
expect(@request.process_url_params('https://example.com/', params: {
expect(@request.process_url_params('https://example.com/', {
foo: [1,2,3],
null: nil,
false: false,
@ -1258,7 +1213,7 @@ describe RestClient::Request, :include_helpers do
it 'should handle ParamsArray objects' do
expect(@request.process_url_params('https://example.com/',
params: RestClient::ParamsArray.new([[:foo, 1], [:foo, 2]])
RestClient::ParamsArray.new([[:foo, 1], [:foo, 2]])
)).to eq 'https://example.com/?foo=1&foo=2'
end
end

View File

@ -3,37 +3,37 @@ require_relative '_lib'
describe RestClient do
describe "API" do
it "GET" do
expect(RestClient::Request).to receive(:execute).with(:method => :get, :url => 'http://some/resource', :headers => {})
expect(RestClient::Request).to receive(:execute).with(:method => :get, :url => 'http://some/resource')
RestClient.get('http://some/resource')
end
it "POST" do
expect(RestClient::Request).to receive(:execute).with(:method => :post, :url => 'http://some/resource', :payload => 'payload', :headers => {})
expect(RestClient::Request).to receive(:execute).with(:method => :post, :url => 'http://some/resource', :payload => 'payload')
RestClient.post('http://some/resource', 'payload')
end
it "PUT" do
expect(RestClient::Request).to receive(:execute).with(:method => :put, :url => 'http://some/resource', :payload => 'payload', :headers => {})
expect(RestClient::Request).to receive(:execute).with(:method => :put, :url => 'http://some/resource', :payload => 'payload')
RestClient.put('http://some/resource', 'payload')
end
it "PATCH" do
expect(RestClient::Request).to receive(:execute).with(:method => :patch, :url => 'http://some/resource', :payload => 'payload', :headers => {})
expect(RestClient::Request).to receive(:execute).with(:method => :patch, :url => 'http://some/resource', :payload => 'payload')
RestClient.patch('http://some/resource', 'payload')
end
it "DELETE" do
expect(RestClient::Request).to receive(:execute).with(:method => :delete, :url => 'http://some/resource', :headers => {})
expect(RestClient::Request).to receive(:execute).with(:method => :delete, :url => 'http://some/resource')
RestClient.delete('http://some/resource')
end
it "HEAD" do
expect(RestClient::Request).to receive(:execute).with(:method => :head, :url => 'http://some/resource', :headers => {})
expect(RestClient::Request).to receive(:execute).with(:method => :head, :url => 'http://some/resource')
RestClient.head('http://some/resource')
end
it "OPTIONS" do
expect(RestClient::Request).to receive(:execute).with(:method => :options, :url => 'http://some/resource', :headers => {})
expect(RestClient::Request).to receive(:execute).with(:method => :options, :url => 'http://some/resource')
RestClient.options('http://some/resource')
end
end