switch back to chef::rest

lib/chef/provider/remote_file.rb

@@ -18,7 +18,6 @@
 #
 
 require 'chef/provider/file'
-require 'rest_client'
 require 'uri'
 require 'tempfile'
 
@@ -110,15 +109,8 @@ def try_multiple_sources(sources)
         begin
           uri = URI.parse(source)
           raw_file = grab_file_from_uri(uri)
-        rescue ArgumentError => e
-          raise e
-        rescue => e
-          if e.is_a?(RestClient::Exception)
-            error = "Request returned #{e.message}"
-          else
-            error = e.to_s
-          end
-          Chef::Log.info("#{@new_resource} cannot be downloaded from #{source}: #{error}")
+        rescue SocketError, Errno::ECONNREFUSED, Errno::ENOENT, Errno::EACCES, Timeout::Error, Net::HTTPFatalError, Net::FTPError => e
+          Chef::Log.warn("#{@new_resource} cannot be downloaded from #{source}: #{e.to_s}")
           if source = sources.shift
             Chef::Log.info("#{@new_resource} trying to download from another mirror")
             retry
@@ -146,7 +138,7 @@ def grab_file_from_uri(uri)
         end
         if URI::HTTP === uri
           #HTTP or HTTPS
-          raw_file, mtime, etag = HTTP::fetch(uri, proxy_uri(uri), if_modified_since, if_none_match)
+          raw_file, mtime, etag = HTTP::fetch(uri, if_modified_since, if_none_match)
         elsif URI::FTP === uri
           #FTP
           raw_file, mtime = FTP::fetch(uri, proxy_uri(uri), @new_resource.ftp_active_mode, if_modified_since)

lib/chef/provider/remote_file/http.rb

@@ -18,6 +18,7 @@
 
 require 'uri'
 require 'tempfile'
+require 'chef/rest'
 require 'chef/provider/remote_file'
 
 class Chef
@@ -26,41 +27,40 @@ class RemoteFile
       class HTTP
 
         # Fetches the file at uri, returning a Tempfile-like File handle
-        def self.fetch(uri, proxy_uri, if_modified_since, if_none_match)
-          request = HTTP.new(uri, proxy_uri, if_modified_since, if_none_match)
+        def self.fetch(uri, if_modified_since, if_none_match)
+          request = HTTP.new(uri, if_modified_since, if_none_match)
           request.execute
         end
 
         # Parse the uri into instance variables
-        def initialize(uri, proxy_uri, if_modified_since, if_none_match)
-          RestClient.proxy = proxy_uri.to_s
+        def initialize(uri, if_modified_since, if_none_match)
           @headers = Hash.new
           if if_none_match
-            @headers[:if_none_match] = "\"#{if_none_match}\""
+            @headers['if-none-match'] = "\"#{if_none_match}\""
           elsif if_modified_since
-            @headers[:if_modified_since] = if_modified_since.strftime("%a, %d %b %Y %H:%M:%S %Z")
+            @headers['if-modified-since'] = if_modified_since.strftime("%a, %d %b %Y %H:%M:%S %Z")
           end
           @uri = uri
         end
 
         def execute
           begin
-            rest = RestClient::Request.execute(:method => :get, :url => @uri.to_s, :headers => @headers, :raw_response => true)
-            tempfile = rest.file
-            if rest.headers.include?(:last_modified)
-              mtime = Time.parse(rest.headers[:last_modified])
-						elsif rest.headers.include?(:date)
-							mtime = Time.parse(rest.headers[:date])
-						else
-							mtime = Time.now
-						end
-            if rest.headers.include?(:etag)
-              etag = rest.headers[:etag]
-						else
-							etag = nil
+            rest = Chef::REST.new(@uri, nil, nil, http_client_opts)
+            tempfile = rest.streaming_request(@uri, @headers)
+            if rest.last_response['last_modified']
+              mtime = Time.parse(rest.last_response['last_modified'])
+            elsif rest.last_response['date']
+              mtime = Time.parse(rest.last_response['date'])
+            else
+              mtime = Time.now
+            end
+            if rest.last_response['etag']
+              etag = rest.last_response['etag']
+            else
+              etag = nil
             end
-          rescue RestClient::Exception => e
-            if e.http_code == 304
+          rescue Net::HTTPRetriableError => e
+            if e.response.is_a? Net::HTTPNotModified
               tempfile = nil
             else
               raise e
@@ -69,6 +69,24 @@ def execute
           return tempfile, mtime, etag
         end
 
+        private
+
+        def http_client_opts
+          opts={}
+          # CHEF-3140
+          # 1. If it's already compressed, trying to compress it more will
+          # probably be counter-productive.
+          # 2. Some servers are misconfigured so that you GET $URL/file.tgz but
+          # they respond with content type of tar and content encoding of gzip,
+          # which tricks Chef::REST into decompressing the response body. In this
+          # case you'd end up with a tar archive (no gzip) named, e.g., foo.tgz,
+          # which is not what you wanted.
+          if @uri.to_s =~ /gz$/
+            opts[:disable_gzip] = true
+          end
+          opts
+        end
+
       end
     end
   end

lib/chef/provider/remote_file/local_file.rb

@@ -31,7 +31,7 @@ def self.fetch(uri, if_modified_since)
           if mtime && if_modified_since && mtime.to_i <= if_modified_since.to_i
             tempfile = nil
           else
-            tempfile = Tempfile.new(File.basename(uri.path))
+            tempfile = Tempfile.new(::File.basename(uri.path))
             Chef::Log.debug("#{@new_resource} staging #{uri.path} to #{tempfile.path}")
             FileUtils.cp(uri.path, tempfile.path)
             tempfile

lib/chef/rest.rb

@@ -84,6 +84,10 @@ def signing_key
       @raw_key
     end
 
+    def last_response
+      @last_response
+    end
+
     # Send an HTTP GET request to the path
     #
     # Using this method to +fetch+ a file is considered deprecated.
@@ -164,6 +168,7 @@ def raw_http_request(method, url, headers, body)
       retriable_rest_request(method, url, body, headers) do |rest_request|
         begin
           response = rest_request.call {|r| r.read_body}
+          @last_response = response
 
           Chef::Log.debug("---- HTTP Status and Header Data: ----")
           Chef::Log.debug("HTTP #{response.http_version} #{response.code} #{response.msg}")
@@ -248,6 +253,7 @@ def streaming_request(url, headers, &block)
               tempfile = stream_to_tempfile(url, r)
             end
           end
+          @last_response = response
           if response.kind_of?(Net::HTTPSuccess)
             tempfile
           elsif redirect_location = redirected_to(response)

lib/chef/rest/rest_request.rb

@@ -21,6 +21,7 @@
 # limitations under the License.
 #
 require 'uri'
+require 'cgi'
 require 'net/http'
 require 'chef/rest/cookie_jar'
 
@@ -220,7 +221,11 @@ def configure_http_request(request_body=nil)
 
         @http_request.body = request_body if (request_body && @http_request.request_body_permitted?)
         # Optionally handle HTTP Basic Authentication
-        @http_request.basic_auth(url.user, url.password) if url.user
+        if url.user
+          user = CGI.unescape(url.user)
+          password = CGI.unescape(url.password) if url.password
+          @http_request.basic_auth(user, password)
+        end
         @http_request[USER_AGENT] = self.class.user_agent
       end
 

spec/unit/provider/remote_file_spec.rb

@@ -46,9 +46,11 @@
   describe "when fetching the file from the remote" do
     before(:each) do
       @tempfile = Tempfile.new("chef-rspec-remote_file_spec-line#{__LINE__}--")
-      @rawresp = RestClient::RawResponse.new(@tempfile, Hash.new, nil)
 
-      RestClient::Request.stub!(:execute).and_return(@rawresp)
+      @rest = mock(Chef::REST, { })
+      Chef::REST.stub!(:new).and_return(@rest)
+      @rest.stub!(:streaming_request).and_return(@tempfile)
+      @rest.stub!(:last_response).and_return({})
       @resource.cookbook_name = "monkey"
 
       @provider.stub!(:checksum).and_return("0fd012fdc96e96f8f7cf2046522a54aed0ce470224513e45da6bc1a17a4924aa")
@@ -79,19 +81,19 @@
 
     shared_examples_for "source specified with multiple URIs" do
       it "should try to download the next URI when the first one fails" do
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://foo", :headers => {}, :raw_response => true).once.and_raise(SocketError)
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://bar", :headers => {}, :raw_response => true).once.and_return(@rawresp)
+        @rest.should_receive(:streaming_request).with(URI.parse("http://foo"), {}).once.and_raise(SocketError)
+        @rest.should_receive(:streaming_request).with(URI.parse("http://bar"), {}).once.and_return(@tempfile)
         @provider.run_action(:create)
       end
 
       it "should raise an exception when all the URIs fail" do
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://foo", :headers => {}, :raw_response => true).once.and_raise(SocketError)
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://bar", :headers => {}, :raw_response => true).once.and_raise(SocketError)
+        @rest.should_receive(:streaming_request).with(URI.parse("http://foo"), {}).once.and_raise(SocketError)
+        @rest.should_receive(:streaming_request).with(URI.parse("http://bar"), {}).once.and_raise(SocketError)
         lambda { @provider.run_action(:create) }.should raise_error(SocketError)
       end
 
       it "should download from only one URI when the first one works" do
-        RestClient::Request.should_receive(:execute).once.and_return(@rawresp)
+        @rest.should_receive(:streaming_request).once.and_return(@tempfile)
         @provider.run_action(:create)
       end
 
@@ -121,7 +123,7 @@
         end
 
         it "does not download the file" do
-          RestClient::Request.should_not_receive(:execute)
+          @rest.should_not_receive(:fetch)
           @provider.run_action(:create)
         end
 
@@ -138,7 +140,7 @@
         end
 
         it "should not download the file if the checksum is a partial match from the beginning" do
-          RestClient::Request.should_not_receive(:execute)
+          @rest.should_not_receive(:fetch)
           @provider.run_action(:create)
         end
 
@@ -152,15 +154,15 @@
       describe "and the existing file doesn't match the given checksum" do
         it "downloads the file" do
           @resource.checksum("this hash doesn't match")
-          RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :headers => {}, :raw_response => true).and_return(@rawresp)
+          @rest.should_receive(:streaming_request).with(URI.parse("http://opscode.com/seattle.txt"), {}).and_return(@tempfile)
           @provider.stub!(:update_new_file_state)
           @provider.run_action(:create)
         end
 
         it "does not consider the checksum a match if the matching string is offset" do
           # i.e., the existing file is      "0fd012fdc96e96f8f7cf2046522a54aed0ce470224513e45da6bc1a17a4924aa"
           @resource.checksum("fd012fd")
-          RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :headers => {}, :raw_response => true).and_return(@rawresp)
+          @rest.should_receive(:streaming_request).with(URI.parse("http://opscode.com/seattle.txt"), {}).and_return(@tempfile)
           @provider.stub!(:update_new_file_state)
           @provider.run_action(:create)
         end
@@ -171,7 +173,7 @@
     describe "and the resource doesn't specify a checksum" do
       it "should download the file from the remote URL" do
         @resource.checksum(nil)
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :headers => {}, :raw_response => true).and_return(@rawresp)
+        @rest.should_receive(:streaming_request).with(URI.parse("http://opscode.com/seattle.txt"), {}).and_return(@tempfile)
         @provider.run_action(:create)
       end
     end
@@ -185,22 +187,10 @@
     # detect when users are fetching gzipped files and turn off gzip in
     # Chef::REST.
 
-    context "and the target file is a tarball" do
-      before do
-        @resource.path(File.expand_path(File.join(CHEF_SPEC_DATA, "seattle.tar.gz")))
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :headers => {}, :raw_response => true).and_return(@rawresp)
-      end
-
-      it "disables gzip in the http client" do
-        @provider.action_create
-      end
-
-    end
-
     context "and the source appears to be a tarball" do
       before do
         @resource.source("http://example.com/tarball.tgz")
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://example.com/tarball.tgz", :headers => {}, :raw_response => true).and_return(@rawresp)
+        Chef::REST.should_receive(:new).with(URI.parse("http://example.com/tarball.tgz"), nil, nil, :disable_gzip => true).and_return(@rest)
       end
 
       it "disables gzip in the http client" do
@@ -239,14 +229,14 @@
     it "should raise an exception if it's any other kind of retriable response than 304" do
       r = Net::HTTPMovedPermanently.new("one", "two", "three")
       e = Net::HTTPRetriableError.new("301", r)
-      RestClient::Request.stub!(:execute).and_raise(e)
+      @rest.stub!(:streaming_request).and_raise(e)
       lambda { @provider.run_action(:create) }.should raise_error(Net::HTTPRetriableError)
     end
 
     it "should raise an exception if anything else happens" do
       r = Net::HTTPBadRequest.new("one", "two", "three")
       e = Net::HTTPServerException.new("fake exception", r)
-      RestClient::Request.stub!(:execute).and_raise(e)
+      @rest.stub!(:streaming_request).and_raise(e)
       lambda { @provider.run_action(:create) }.should raise_error(Net::HTTPServerException)
     end
 
@@ -342,6 +332,7 @@
         @provider.should_receive(:set_all_access_controls).and_return(true)
         @provider.run_action(:create)
       end
+
     end
   end
 end