diff options
| -rw-r--r-- | lib/chef/provider/remote_file.rb | 14 | ||||
| -rw-r--r-- | lib/chef/provider/remote_file/http.rb | 60 | ||||
| -rw-r--r-- | lib/chef/provider/remote_file/local_file.rb | 2 | ||||
| -rw-r--r-- | lib/chef/rest.rb | 6 | ||||
| -rw-r--r-- | lib/chef/rest/rest_request.rb | 7 | ||||
| -rw-r--r-- | spec/unit/provider/remote_file_spec.rb | 45 |
6 files changed, 73 insertions, 61 deletions
diff --git a/lib/chef/provider/remote_file.rb b/lib/chef/provider/remote_file.rb index 66e8fcccf6..a81e5b7737 100644 --- a/lib/chef/provider/remote_file.rb +++ b/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 @@ class Chef 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 @@ class Chef 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) diff --git a/lib/chef/provider/remote_file/http.rb b/lib/chef/provider/remote_file/http.rb index 157f4f3291..44e2b11d74 100644 --- a/lib/chef/provider/remote_file/http.rb +++ b/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 Chef 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 @@ class Chef 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 diff --git a/lib/chef/provider/remote_file/local_file.rb b/lib/chef/provider/remote_file/local_file.rb index 346867348d..9a07621d14 100644 --- a/lib/chef/provider/remote_file/local_file.rb +++ b/lib/chef/provider/remote_file/local_file.rb @@ -31,7 +31,7 @@ class Chef 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 diff --git a/lib/chef/rest.rb b/lib/chef/rest.rb index 21be437e24..8f5405f9d2 100644 --- a/lib/chef/rest.rb +++ b/lib/chef/rest.rb @@ -84,6 +84,10 @@ class Chef @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 @@ class Chef 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 @@ class Chef tempfile = stream_to_tempfile(url, r) end end + @last_response = response if response.kind_of?(Net::HTTPSuccess) tempfile elsif redirect_location = redirected_to(response) diff --git a/lib/chef/rest/rest_request.rb b/lib/chef/rest/rest_request.rb index 4ff0016205..fe9ba6f7b0 100644 --- a/lib/chef/rest/rest_request.rb +++ b/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 @@ class Chef @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 diff --git a/spec/unit/provider/remote_file_spec.rb b/spec/unit/provider/remote_file_spec.rb index 09af1422b1..9f22fc5df5 100644 --- a/spec/unit/provider/remote_file_spec.rb +++ b/spec/unit/provider/remote_file_spec.rb @@ -46,9 +46,11 @@ describe Chef::Provider::RemoteFile, "action_create" do 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 @@ describe Chef::Provider::RemoteFile, "action_create" do 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 @@ describe Chef::Provider::RemoteFile, "action_create" do 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 @@ describe Chef::Provider::RemoteFile, "action_create" do 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,7 +154,7 @@ describe Chef::Provider::RemoteFile, "action_create" do 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 @@ -160,7 +162,7 @@ describe Chef::Provider::RemoteFile, "action_create" do 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 Chef::Provider::RemoteFile, "action_create" do 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 @@ describe Chef::Provider::RemoteFile, "action_create" do # 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 @@ describe Chef::Provider::RemoteFile, "action_create" do 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 @@ describe Chef::Provider::RemoteFile, "action_create" do @provider.should_receive(:set_all_access_controls).and_return(true) @provider.run_action(:create) end + end end end |