diff --git a/lib/restclient/request.rb b/lib/restclient/request.rb index 600924fb..02e8626c 100644 --- a/lib/restclient/request.rb +++ b/lib/restclient/request.rb @@ -334,19 +334,10 @@ def transmit uri, req, payload, & block net.use_ssl = uri.is_a?(URI::HTTPS) net.ssl_version = ssl_version if ssl_version net.ciphers = ssl_ciphers if ssl_ciphers - err_msg = nil - if verify_ssl - net.verify_mode = verify_ssl - net.verify_callback = lambda do |preverify_ok, ssl_context| - if (!preverify_ok) || ssl_context.error != 0 - err_msg = "SSL Verification failed -- Preverify: #{preverify_ok}, Error: #{ssl_context.error_string} (#{ssl_context.error})" - return false - end - true - end - else - net.verify_mode = verify_ssl - end + + # we no longer set net.verify_callback because it's not well supported on + # all platforms (see comments below) + net.verify_mode = verify_ssl net.cert = ssl_client_cert if ssl_client_cert net.key = ssl_client_key if ssl_client_key @@ -393,16 +384,31 @@ def transmit uri, req, payload, & block process_result res, & block end end - rescue OpenSSL::SSL::SSLError => e - if err_msg - raise SSLCertificateNotVerified.new(err_msg) - else - raise e - end rescue EOFError raise RestClient::ServerBrokeConnection rescue Timeout::Error, Errno::ETIMEDOUT raise RestClient::RequestTimeout + + rescue OpenSSL::SSL::SSLError => error + # TODO: deprecate and remove RestClient::SSLCertificateNotVerified and just + # pass through OpenSSL::SSL::SSLError directly. + # + # Exceptions in verify_callback are ignored [1], and jruby doesn't support + # it at all [2]. RestClient has to catch OpenSSL::SSL::SSLError and either + # re-throw it as is, or throw SSLCertificateNotVerified based on the + # contents of the message field of the original exception. + # + # The client has to handle OpenSSL::SSL::SSLError exceptions anyway, so + # we shouldn't make them handle both OpenSSL and RestClient exceptions. + # + # [1] https://github.com/ruby/ruby/blob/89e70fe8e7/ext/openssl/ossl.c#L238 + # [2] https://github.com/jruby/jruby/issues/597 + + if error.message.include?("certificate verify failed") + raise SSLCertificateNotVerified.new(error.message) + else + raise error + end end def setup_credentials(req) diff --git a/spec/integration/request_spec.rb b/spec/integration/request_spec.rb index 94702937..a3070090 100644 --- a/spec/integration/request_spec.rb +++ b/spec/integration/request_spec.rb @@ -28,16 +28,9 @@ expect { request.execute }.to_not raise_error end - # I don' think this feature is useful anymore (under 1.9.3 at the very least). - # - # Exceptions in verify_callback are ignored; RestClient has to catch OpenSSL::SSL::SSLError - # and either re-throw it as is, or throw SSLCertificateNotVerified - # based on the contents of the message field of the original exception - #. - # The client has to handle OpenSSL::SSL::SSLError exceptions anyway, - # why make them handle both OpenSSL *AND* RestClient exceptions??? - # - # also see https://github.com/ruby/ruby/blob/trunk/ext/openssl/ossl.c#L237 + # TODO: deprecate and remove RestClient::SSLCertificateNotVerified and just + # pass through OpenSSL::SSL::SSLError directly. See note in + # lib/restclient/request.rb. it "is unsuccessful with an incorrect ca_file" do request = RestClient::Request.new( :method => :get, diff --git a/spec/unit/request_spec.rb b/spec/unit/request_spec.rb index d9a4fd96..0776a2e2 100644 --- a/spec/unit/request_spec.rb +++ b/spec/unit/request_spec.rb @@ -549,7 +549,6 @@ :payload => 'payload', :verify_ssl => mode ) @net.should_receive(:verify_mode=).with(mode) - @net.should_receive(:verify_callback=) @http.stub(:request) @request.stub(:process_result) @request.stub(:response_log)