From ec5c14d91f3c42e4f2683002d53435d1d88973a1 Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Tue, 22 Jan 2013 11:43:14 -0500 Subject: [PATCH 01/15] remote_file: CHEF-1031: FTP URLs; CHEF-3786: authenticated HTTP/S Conflicts: lib/chef/provider/remote_file.rb lib/chef/resource/remote_file.rb --- lib/chef/provider/remote_file.rb | 87 ++++++++++++++++++++------------ lib/chef/resource/remote_file.rb | 9 ++++ 2 files changed, 64 insertions(+), 32 deletions(-) diff --git a/lib/chef/provider/remote_file.rb b/lib/chef/provider/remote_file.rb index f9856824413..6d61930ab38 100644 --- a/lib/chef/provider/remote_file.rb +++ b/lib/chef/provider/remote_file.rb @@ -17,10 +17,11 @@ # require 'chef/provider/file' -require 'chef/rest' +require 'rest_client' require 'uri' require 'tempfile' require 'net/https' +require 'net/ftp' class Chef class Provider @@ -42,8 +43,14 @@ def action_create sources = @new_resource.source source = sources.shift begin - rest = Chef::REST.new(source, nil, nil, http_client_opts(source)) - raw_file = rest.streaming_request(rest.create_url(source), {}) + uri = URI.parse(source) + if URI::HTTP === uri + #HTTP or HTTPS + raw_file = RestClient::Request.execute(:method => :get, :url => source, :raw_response => true).file + else + #FTP + raw_file = ftp_fetch(uri) + end rescue SocketError, Errno::ECONNREFUSED, Timeout::Error, Net::HTTPFatalError => e Chef::Log.debug("#{@new_resource} cannot be downloaded from #{source}") if source = sources.shift @@ -102,40 +109,56 @@ def backup_new_resource end end - def source_file(source, current_checksum, &block) - if absolute_uri?(source) - fetch_from_uri(source, &block) - elsif !Chef::Config[:solo] - fetch_from_chef_server(source, current_checksum, &block) - else - fetch_from_local_cookbook(source, &block) - end - end + private - def http_client_opts(source) - 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 @new_resource.path =~ /gz$/ or source =~ /gz$/ - opts[:disable_gzip] = true + def ftp_fetch(uri) + # Shamelessly stolen from open-uri + # Fetches the file using Net::FTP, returning a Tempfile + path = uri.path + path = path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it. + directories = path.split(%r{/}, -1) + directories.each {|d| + d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") } + } + unless filename = directories.pop + raise ArgumentError, "no filename: #{uri.inspect}" + end + directories.each {|d| + if /[\r\n]/ =~ d + raise ArgumentError, "invalid directory: #{d.inspect}" + end + } + if /[\r\n]/ =~ filename + raise ArgumentError, "invalid filename: #{filename.inspect}" + end + typecode = uri.typecode + if typecode && /\A[aid]\z/ !~ typecode + raise ArgumentError, "invalid typecode: #{typecode.inspect}" end - opts - end - private + tempfile = Tempfile.new(filename) - def absolute_uri?(source) - URI.parse(source).absolute? - rescue URI::InvalidURIError - false - end + # The access sequence is defined by RFC 1738 + ftp = Net::FTP.new + ftp.connect(uri.hostname, uri.port) + ftp.passive = true if !@new_resource.ftp_active_mode + # todo: extract user/passwd from .netrc. + user = 'anonymous' + passwd = nil + user, passwd = uri.userinfo.split(/:/) if uri.userinfo + ftp.login(user, passwd) + directories.each {|cwd| + ftp.voidcmd("CWD #{cwd}") + } + if typecode + # xxx: typecode D is not handled. + ftp.voidcmd("TYPE #{typecode.upcase}") + end + ftp.getbinaryfile(filename, tempfile.path) + ftp.close + tempfile + end end end end diff --git a/lib/chef/resource/remote_file.rb b/lib/chef/resource/remote_file.rb index 2798cba3f27..21e1a2680fb 100644 --- a/lib/chef/resource/remote_file.rb +++ b/lib/chef/resource/remote_file.rb @@ -33,6 +33,7 @@ def initialize(name, run_context=nil) @resource_name = :remote_file @action = "create" @source = nil + @ftp_active_mode = false @provider = Chef::Provider::RemoteFile end @@ -54,6 +55,14 @@ def checksum(args=nil) ) end + def ftp_active_mode(args=nil) + set_or_return( + :ftp_active_mode, + args, + :kind_of => [ TrueClass, FalseClass ] + ) + end + def after_created validate_source(@source) end From ef9b1a83df9055ecf4c1e93632669f817f52e689 Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Tue, 22 Jan 2013 13:08:32 -0500 Subject: [PATCH 02/15] add attribution for ftp methods --- lib/chef/provider/remote_file.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/chef/provider/remote_file.rb b/lib/chef/provider/remote_file.rb index 6d61930ab38..cf4d129bea6 100644 --- a/lib/chef/provider/remote_file.rb +++ b/lib/chef/provider/remote_file.rb @@ -1,5 +1,6 @@ # # Author:: Adam Jacob () +# Author:: Jesse Campbell () # Copyright:: Copyright (c) 2008 Opscode, Inc. # License:: Apache License, Version 2.0 # From 7a2975342ea3e51d0875c125f0061d52bd559d6a Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Tue, 5 Feb 2013 11:07:58 -0500 Subject: [PATCH 03/15] pull ftp functionality out of remote_file.rb, move to remote_file/ftp.rb add file:// --- lib/chef/provider/remote_file.rb | 77 +++++++------------------- lib/chef/provider/remote_file/ftp.rb | 82 ++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 58 deletions(-) create mode 100644 lib/chef/provider/remote_file/ftp.rb diff --git a/lib/chef/provider/remote_file.rb b/lib/chef/provider/remote_file.rb index cf4d129bea6..27ee95acd3c 100644 --- a/lib/chef/provider/remote_file.rb +++ b/lib/chef/provider/remote_file.rb @@ -19,10 +19,9 @@ require 'chef/provider/file' require 'rest_client' +require 'chef/provider/remote_file/ftp' require 'uri' require 'tempfile' -require 'net/https' -require 'net/ftp' class Chef class Provider @@ -48,11 +47,16 @@ def action_create if URI::HTTP === uri #HTTP or HTTPS raw_file = RestClient::Request.execute(:method => :get, :url => source, :raw_response => true).file - else + elsif URI::FTP === uri #FTP - raw_file = ftp_fetch(uri) + raw_file = FTP::fetch(uri, @new_resource.ftp_active_mode) + elsif uri.scheme == "file" + #local/network file + raw_file = ::File.new(uri.path, "r") + else + raise ArgumentError, "Invalid uri. Only http(s), ftp, and file are currently supported" end - rescue SocketError, Errno::ECONNREFUSED, Timeout::Error, Net::HTTPFatalError => e + rescue => e Chef::Log.debug("#{@new_resource} cannot be downloaded from #{source}") if source = sources.shift Chef::Log.debug("#{@new_resource} trying to download from another mirror") @@ -71,12 +75,20 @@ def action_create backup_new_resource FileUtils.cp raw_file.path, @new_resource.path Chef::Log.info "#{@new_resource} updated" - raw_file.close! + if raw_file.is_a? Tempfile + raw_file.close! + else + raw_file.close + end end # whyrun mode cleanup - the temp file will never be used, # so close/unlink it here. if whyrun_mode? - raw_file.close! + if raw_file.is_a? Tempfile + raw_file.close! + else + raw_file.close + end end end end @@ -109,57 +121,6 @@ def backup_new_resource backup @new_resource.path end end - - private - - def ftp_fetch(uri) - # Shamelessly stolen from open-uri - # Fetches the file using Net::FTP, returning a Tempfile - path = uri.path - path = path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it. - directories = path.split(%r{/}, -1) - directories.each {|d| - d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") } - } - unless filename = directories.pop - raise ArgumentError, "no filename: #{uri.inspect}" - end - directories.each {|d| - if /[\r\n]/ =~ d - raise ArgumentError, "invalid directory: #{d.inspect}" - end - } - if /[\r\n]/ =~ filename - raise ArgumentError, "invalid filename: #{filename.inspect}" - end - typecode = uri.typecode - if typecode && /\A[aid]\z/ !~ typecode - raise ArgumentError, "invalid typecode: #{typecode.inspect}" - end - - tempfile = Tempfile.new(filename) - - # The access sequence is defined by RFC 1738 - ftp = Net::FTP.new - ftp.connect(uri.hostname, uri.port) - ftp.passive = true if !@new_resource.ftp_active_mode - # todo: extract user/passwd from .netrc. - user = 'anonymous' - passwd = nil - user, passwd = uri.userinfo.split(/:/) if uri.userinfo - ftp.login(user, passwd) - directories.each {|cwd| - ftp.voidcmd("CWD #{cwd}") - } - if typecode - # xxx: typecode D is not handled. - ftp.voidcmd("TYPE #{typecode.upcase}") - end - ftp.getbinaryfile(filename, tempfile.path) - ftp.close - - tempfile - end end end end diff --git a/lib/chef/provider/remote_file/ftp.rb b/lib/chef/provider/remote_file/ftp.rb new file mode 100644 index 00000000000..99738222232 --- /dev/null +++ b/lib/chef/provider/remote_file/ftp.rb @@ -0,0 +1,82 @@ +# +# Author:: Jesse Campbell () +# Copyright:: Copyright (c) 2008 Opscode, Inc. +# License:: Apache License, Version 2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require 'uri' +require 'tempfile' +require 'net/ftp' + +class Chef + class Provider + class RemoteFile < Chef::Provider::File + class FTP + + # Fetches the file at uri using Net::FTP, returning a Tempfile + # Parts shamelessly stolen from open-uri + def self.fetch(uri, ftp_active_mode) + path = uri.path + path = path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it. + directories = path.split(%r{/}, -1) + directories.each {|d| + d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") } + } + unless filename = directories.pop + raise ArgumentError, "no filename: #{uri.inspect}" + end + directories.each {|d| + if /[\r\n]/ =~ d + raise ArgumentError, "invalid directory: #{d.inspect}" + end + } + if /[\r\n]/ =~ filename + raise ArgumentError, "invalid filename: #{filename.inspect}" + end + typecode = uri.typecode + if typecode && /\A[aid]\z/ !~ typecode + raise ArgumentError, "invalid typecode: #{typecode.inspect}" + end + + tempfile = Tempfile.new(filename) + + # The access sequence is defined by RFC 1738 + ftp = Net::FTP.new + ftp.connect(uri.hostname, uri.port) + ftp.passive = true if !ftp_active_mode + # todo: extract user/passwd from .netrc. + user = 'anonymous' + passwd = nil + if uri.userinfo + user = URI.unescape(uri.user) + passwd = URI.unescape(uri.password) + end + ftp.login(user, passwd) + directories.each {|cwd| + ftp.voidcmd("CWD #{cwd}") + } + if typecode + # xxx: typecode D is not handled. + ftp.voidcmd("TYPE #{typecode.upcase}") + end + ftp.getbinaryfile(filename, tempfile.path) + ftp.close + + tempfile + end + end + end + end +end From 3dff33d68b43c67dac5b6f5b4bf27309f5c0a6ff Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Tue, 5 Feb 2013 16:13:22 -0500 Subject: [PATCH 04/15] Fix remote_file namespacing --- lib/chef/provider/remote_file.rb | 1 - lib/chef/provider/remote_file/ftp.rb | 3 ++- lib/chef/providers.rb | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/chef/provider/remote_file.rb b/lib/chef/provider/remote_file.rb index 27ee95acd3c..7e3816496b3 100644 --- a/lib/chef/provider/remote_file.rb +++ b/lib/chef/provider/remote_file.rb @@ -19,7 +19,6 @@ require 'chef/provider/file' require 'rest_client' -require 'chef/provider/remote_file/ftp' require 'uri' require 'tempfile' diff --git a/lib/chef/provider/remote_file/ftp.rb b/lib/chef/provider/remote_file/ftp.rb index 99738222232..bc4e5a51835 100644 --- a/lib/chef/provider/remote_file/ftp.rb +++ b/lib/chef/provider/remote_file/ftp.rb @@ -19,10 +19,11 @@ require 'uri' require 'tempfile' require 'net/ftp' +require 'chef/provider/remote_file' class Chef class Provider - class RemoteFile < Chef::Provider::File + class RemoteFile class FTP # Fetches the file at uri using Net::FTP, returning a Tempfile diff --git a/lib/chef/providers.rb b/lib/chef/providers.rb index ae95632eaa9..3a40c542ec4 100644 --- a/lib/chef/providers.rb +++ b/lib/chef/providers.rb @@ -101,5 +101,7 @@ require 'chef/provider/deploy/revision' require 'chef/provider/deploy/timestamped' +require 'chef/provider/remote_file/ftp' + require "chef/provider/lwrp_base" require 'chef/provider/registry_key' From 0a0c8c4e6134f843c16127eaa666f454ddd3e3fb Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Tue, 5 Feb 2013 16:13:50 -0500 Subject: [PATCH 05/15] Update existing tests to function with RestClient::Request --- spec/unit/provider/remote_file_spec.rb | 36 ++++++++++++-------------- spec/unit/resource/remote_file_spec.rb | 16 +++++++++--- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/spec/unit/provider/remote_file_spec.rb b/spec/unit/provider/remote_file_spec.rb index 78d7e771210..d7fbad52ef4 100644 --- a/spec/unit/provider/remote_file_spec.rb +++ b/spec/unit/provider/remote_file_spec.rb @@ -46,11 +46,9 @@ 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, nil, nil) - @rest = mock(Chef::REST, { }) - Chef::REST.stub!(:new).and_return(@rest) - @rest.stub!(:streaming_request).and_return(@tempfile) - @rest.stub!(:create_url) { |url| url } + RestClient::Request.stub!(:execute).and_return(@rawresp) @resource.cookbook_name = "monkey" @provider.stub!(:checksum).and_return("0fd012fdc96e96f8f7cf2046522a54aed0ce470224513e45da6bc1a17a4924aa") @@ -66,7 +64,7 @@ end before do - @resource.source("http://opscode.com/seattle.txt") + @resource.source("http://localhost:9000/seattle.txt") end describe "and the target location's enclosing directory does not exist" do @@ -81,19 +79,19 @@ shared_examples_for "source specified with multiple URIs" do it "should try to download the next URI when the first one fails" do - @rest.should_receive(:streaming_request).with("http://foo", {}).once.and_raise(SocketError) - @rest.should_receive(:streaming_request).with("http://bar", {}).once.and_return(@tempfile) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://foo", :raw_response => true).once.and_raise(SocketError) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://bar", :raw_response => true).once.and_return(@rawresp) @provider.run_action(:create) end it "should raise an exception when all the URIs fail" do - @rest.should_receive(:streaming_request).with("http://foo", {}).once.and_raise(SocketError) - @rest.should_receive(:streaming_request).with("http://bar", {}).once.and_raise(SocketError) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://foo", :raw_response => true).once.and_raise(SocketError) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://bar", :raw_response => true).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 - @rest.should_receive(:streaming_request).once.and_return(@tempfile) + RestClient::Request.should_receive(:execute).once.and_return(@rawresp) @provider.run_action(:create) end @@ -123,7 +121,7 @@ end it "does not download the file" do - @rest.should_not_receive(:fetch).with("http://opscode.com/seattle.txt").and_return(@tempfile) + RestClient::Request.should_not_receive(:execute).with("http://localhost:9000/seattle.txt").and_return(@tempfile) @provider.run_action(:create) end @@ -140,7 +138,7 @@ end it "should not download the file if the checksum is a partial match from the beginning" do - @rest.should_not_receive(:fetch).with("http://opscode.com/seattle.txt").and_return(@tempfile) + @rawresp.should_not_receive(:fetch).with("http://localhost:9000/seattle.txt").and_return(@tempfile) @provider.run_action(:create) end @@ -154,7 +152,7 @@ describe "and the existing file doesn't match the given checksum" do it "downloads the file" do @resource.checksum("this hash doesn't match") - @rest.should_receive(:streaming_request).with("http://opscode.com/seattle.txt", {}).and_return(@tempfile) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://localhost:9000/seattle.txt", :raw_response => true).and_return(@rawresp) @provider.stub!(:update_new_file_state) @provider.run_action(:create) end @@ -162,7 +160,7 @@ 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") - @rest.should_receive(:streaming_request).with("http://opscode.com/seattle.txt", {}).and_return(@tempfile) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://localhost:9000/seattle.txt", :raw_response => true).and_return(@rawresp) @provider.stub!(:update_new_file_state) @provider.run_action(:create) end @@ -173,7 +171,7 @@ describe "and the resource doesn't specify a checksum" do it "should download the file from the remote URL" do @resource.checksum(nil) - @rest.should_receive(:streaming_request).with("http://opscode.com/seattle.txt", {}).and_return(@tempfile) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://localhost:9000/seattle.txt", :raw_response => true).and_return(@rawresp) @provider.run_action(:create) end end @@ -190,7 +188,7 @@ context "and the target file is a tarball" do before do @resource.path(File.expand_path(File.join(CHEF_SPEC_DATA, "seattle.tar.gz"))) - Chef::REST.should_receive(:new).with("http://opscode.com/seattle.txt", nil, nil, :disable_gzip => true).and_return(@rest) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://localhost:9000/seattle.txt", :raw_response => true).and_return(@rawresp) end it "disables gzip in the http client" do @@ -202,7 +200,7 @@ context "and the source appears to be a tarball" do before do @resource.source("http://example.com/tarball.tgz") - Chef::REST.should_receive(:new).with("http://example.com/tarball.tgz", nil, nil, :disable_gzip => true).and_return(@rest) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://example.com/tarball.tgz", :raw_response => true).and_return(@rawresp) end it "disables gzip in the http client" do @@ -213,14 +211,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) - @rest.stub!(:streaming_request).and_raise(e) + RestClient::Request.stub!(:execute).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) - @rest.stub!(:streaming_request).and_raise(e) + RestClient::Request.stub!(:execute).and_raise(e) lambda { @provider.run_action(:create) }.should raise_error(Net::HTTPServerException) end diff --git a/spec/unit/resource/remote_file_spec.rb b/spec/unit/resource/remote_file_spec.rb index d91f80d1a70..b626a73882c 100644 --- a/spec/unit/resource/remote_file_spec.rb +++ b/spec/unit/resource/remote_file_spec.rb @@ -64,7 +64,7 @@ lambda { @resource.source("not-a-uri") }.should raise_error(Chef::Exceptions::InvalidRemoteFileURI) end - it "should raise and exception when source is an empty array" do + it "should raise an exception when source is an empty array" do lambda { @resource.source([]) }.should raise_error(ArgumentError) end @@ -80,7 +80,18 @@ @resource.checksum.should == nil end end - + + describe "ftp_active_mode" do + it "should accept a boolean for the ftp_active_mode object" do + @resource.ftp_active_mode true + @resource.ftp_active_mode.should be_true + end + + it "should default to false" do + @resource.ftp_active_mode.should be_false + end + end + describe "when it has group, mode, owner, source, and checksum" do before do if Chef::Platform.windows? @@ -119,5 +130,4 @@ end end end - end From 236be1df6a0f1c7f90eeaccb60f648a1227633d9 Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Tue, 5 Feb 2013 19:58:58 -0500 Subject: [PATCH 06/15] add new tests --- lib/chef/provider/remote_file/ftp.rb | 106 +++++++++---------- spec/unit/provider/remote_file/ftp_spec.rb | 117 +++++++++++++++++++++ spec/unit/provider/remote_file_spec.rb | 45 ++++++-- 3 files changed, 202 insertions(+), 66 deletions(-) create mode 100644 spec/unit/provider/remote_file/ftp_spec.rb diff --git a/lib/chef/provider/remote_file/ftp.rb b/lib/chef/provider/remote_file/ftp.rb index bc4e5a51835..995fb0ba72b 100644 --- a/lib/chef/provider/remote_file/ftp.rb +++ b/lib/chef/provider/remote_file/ftp.rb @@ -1,6 +1,6 @@ # # Author:: Jesse Campbell () -# Copyright:: Copyright (c) 2008 Opscode, Inc. +# Copyright:: Copyright (c) 2013 Jesse Campbell # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -20,64 +20,58 @@ require 'tempfile' require 'net/ftp' require 'chef/provider/remote_file' - + class Chef - class Provider - class RemoteFile - class FTP + class Provider + class RemoteFile + class FTP - # Fetches the file at uri using Net::FTP, returning a Tempfile - # Parts shamelessly stolen from open-uri - def self.fetch(uri, ftp_active_mode) - path = uri.path - path = path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it. - directories = path.split(%r{/}, -1) - directories.each {|d| - d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") } - } - unless filename = directories.pop - raise ArgumentError, "no filename: #{uri.inspect}" - end - directories.each {|d| - if /[\r\n]/ =~ d - raise ArgumentError, "invalid directory: #{d.inspect}" - end - } - if /[\r\n]/ =~ filename - raise ArgumentError, "invalid filename: #{filename.inspect}" - end - typecode = uri.typecode - if typecode && /\A[aid]\z/ !~ typecode - raise ArgumentError, "invalid typecode: #{typecode.inspect}" - end + # Fetches the file at uri using Net::FTP, returning a Tempfile + # Taken from open-uri + def self.fetch(uri, ftp_active_mode) + path = uri.path + path = path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it. + directories = path.split(%r{/}, -1) + directories.each {|d| + d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") } + } + unless filename = directories.pop + raise ArgumentError, "no filename: #{uri.inspect}" + end + if filename.length == 0 || filename.end_with?( "/" ) + raise ArgumentError, "no filename: #{uri.inspect}" + end + typecode = uri.typecode + # Only support ascii and binary types + if typecode && /\A[ai]\z/ !~ typecode + raise ArgumentError, "invalid typecode: #{typecode.inspect}" + end - tempfile = Tempfile.new(filename) + tempfile = Tempfile.new(filename) - # The access sequence is defined by RFC 1738 - ftp = Net::FTP.new - ftp.connect(uri.hostname, uri.port) - ftp.passive = true if !ftp_active_mode - # todo: extract user/passwd from .netrc. - user = 'anonymous' - passwd = nil - if uri.userinfo - user = URI.unescape(uri.user) - passwd = URI.unescape(uri.password) - end - ftp.login(user, passwd) - directories.each {|cwd| - ftp.voidcmd("CWD #{cwd}") - } - if typecode - # xxx: typecode D is not handled. - ftp.voidcmd("TYPE #{typecode.upcase}") - end - ftp.getbinaryfile(filename, tempfile.path) - ftp.close + # The access sequence is defined by RFC 1738 + ftp = Net::FTP.new + ftp.connect(uri.hostname, uri.port) + ftp.passive = !ftp_active_mode + user = 'anonymous' + passwd = nil + if uri.userinfo + user = URI.unescape(uri.user) + passwd = URI.unescape(uri.password) + end + ftp.login(user, passwd) + directories.each {|cwd| + ftp.voidcmd("CWD #{cwd}") + } + if typecode + ftp.voidcmd("TYPE #{typecode.upcase}") + end + ftp.getbinaryfile(filename, tempfile.path) + ftp.close - tempfile - end - end - end - end + tempfile + end + end + end + end end diff --git a/spec/unit/provider/remote_file/ftp_spec.rb b/spec/unit/provider/remote_file/ftp_spec.rb new file mode 100644 index 00000000000..368a7346c48 --- /dev/null +++ b/spec/unit/provider/remote_file/ftp_spec.rb @@ -0,0 +1,117 @@ +# +# Author:: Jesse Campbell () +# Copyright:: Copyright (c) 2013 Jesse Campbell +# License:: Apache License, Version 2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require 'spec_helper' + +describe Chef::Provider::RemoteFile::FTP, "fetch" do + before(:each) do + @ftp = mock(Net::FTP, { }) + Net::FTP.stub!(:new).and_return(@ftp) + @ftp.stub!(:connect) + @ftp.stub!(:login) + @ftp.stub!(:voidcmd) + @ftp.stub!(:getbinaryfile) + @ftp.stub!(:close) + @ftp.stub!(:passive=) + @tempfile = Tempfile.new("chef-rspec-ftp_spec-line#{__LINE__}--") + Tempfile.stub!(:new).and_return(@tempfile) + @uri = URI.parse("ftp://opscode.com/seattle.txt") + end + + describe "when parsing the uri" do + it "throws an argument exception when no path is given" do + @uri.path = "" + lambda { Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! }.should raise_error(ArgumentError) + end + + it "throws an argument exception when only a / is given" do + @uri.path = "/" + lambda { Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! }.should raise_error(ArgumentError) + end + + it "throws an argument exception when no filename is given" do + @uri.path = "/the/whole/path/" + lambda { Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! }.should raise_error(ArgumentError) + end + + it "throws an argument exception when the typecode is invalid" do + @uri.typecode = "d" + lambda { Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! }.should raise_error(ArgumentError) + end + end + + describe "when connecting to the remote" do + it "should connect to the host from the uri on the default port 21" do + @ftp.should_receive(:connect).with("opscode.com", 21) + Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! + end + + it "should connect on an alternate port when one is provided" do + @ftp.should_receive(:connect).with("opscode.com", 8021) + Chef::Provider::RemoteFile::FTP.fetch(URI.parse("ftp://opscode.com:8021/seattle.txt"), false).close! + end + + it "should set passive true when ftp_active_mode is false" do + @ftp.should_receive(:passive=).with(true) + Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! + end + + it "should set passive false when ftp_active_mode is false" do + @ftp.should_receive(:passive=).with(false) + Chef::Provider::RemoteFile::FTP.fetch(@uri, true).close! + end + + it "should use anonymous ftp when no userinfo is provided" do + @ftp.should_receive(:login).with("anonymous", nil) + Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! + end + + it "should use authenticated ftp when userinfo is provided" do + @ftp.should_receive(:login).with("the_user", "the_password") + Chef::Provider::RemoteFile::FTP.fetch(URI.parse("ftp://the_user:the_password@opscode.com/seattle.txt"), false).close! + end + + it "should accept ascii for the typecode" do + @uri.typecode = "a" + @ftp.should_receive(:voidcmd).with("TYPE A").once + Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! + end + + it "should accept image for the typecode" do + @uri.typecode = "i" + @ftp.should_receive(:voidcmd).with("TYPE I").once + Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! + end + + it "should fetch the file from the correct path" do + @ftp.should_receive(:voidcmd).with("CWD the").once + @ftp.should_receive(:voidcmd).with("CWD whole").once + @ftp.should_receive(:voidcmd).with("CWD path").once + @ftp.should_receive(:getbinaryfile).with("seattle.txt", @tempfile.path) + Chef::Provider::RemoteFile::FTP.fetch(URI.parse("ftp://opscode.com/the/whole/path/seattle.txt"), false).close! + end + end + + describe "when it finishes downloading" do + it "should return a tempfile" do + ftpfile = Chef::Provider::RemoteFile::FTP.fetch(@uri, false) + ftpfile.should equal @tempfile + ftpfile.close! + end + end +end diff --git a/spec/unit/provider/remote_file_spec.rb b/spec/unit/provider/remote_file_spec.rb index d7fbad52ef4..149f0462c33 100644 --- a/spec/unit/provider/remote_file_spec.rb +++ b/spec/unit/provider/remote_file_spec.rb @@ -64,7 +64,7 @@ end before do - @resource.source("http://localhost:9000/seattle.txt") + @resource.source("http://opscode.com/seattle.txt") end describe "and the target location's enclosing directory does not exist" do @@ -121,7 +121,7 @@ end it "does not download the file" do - RestClient::Request.should_not_receive(:execute).with("http://localhost:9000/seattle.txt").and_return(@tempfile) + RestClient::Request.should_not_receive(:execute).with("http://opscode.com/seattle.txt").and_return(@tempfile) @provider.run_action(:create) end @@ -138,7 +138,7 @@ end it "should not download the file if the checksum is a partial match from the beginning" do - @rawresp.should_not_receive(:fetch).with("http://localhost:9000/seattle.txt").and_return(@tempfile) + @rawresp.should_not_receive(:fetch).with("http://opscode.com/seattle.txt").and_return(@tempfile) @provider.run_action(:create) end @@ -152,7 +152,7 @@ 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://localhost:9000/seattle.txt", :raw_response => true).and_return(@rawresp) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :raw_response => true).and_return(@rawresp) @provider.stub!(:update_new_file_state) @provider.run_action(:create) end @@ -160,7 +160,7 @@ 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://localhost:9000/seattle.txt", :raw_response => true).and_return(@rawresp) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :raw_response => true).and_return(@rawresp) @provider.stub!(:update_new_file_state) @provider.run_action(:create) end @@ -171,7 +171,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://localhost:9000/seattle.txt", :raw_response => true).and_return(@rawresp) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :raw_response => true).and_return(@rawresp) @provider.run_action(:create) end end @@ -188,7 +188,7 @@ 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://localhost:9000/seattle.txt", :raw_response => true).and_return(@rawresp) + RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :raw_response => true).and_return(@rawresp) end it "disables gzip in the http client" do @@ -208,6 +208,34 @@ end end + context "and the uri scheme is ftp" do + before do + @resource.source("ftp://opscode.com/seattle.txt") + end + + it "should fetch with ftp in passive mode" do + Chef::Provider::RemoteFile::FTP.should_receive(:fetch).with(URI.parse("ftp://opscode.com/seattle.txt"), false).and_return(@tempfile) + @provider.run_action(:create) + end + + it "should fetch with ftp in active mode" do + @resource.ftp_active_mode true + Chef::Provider::RemoteFile::FTP.should_receive(:fetch).with(URI.parse("ftp://opscode.com/seattle.txt"), true).and_return(@tempfile) + @provider.run_action(:create) + end + end + + context "and the uri scheme is file" do + before do + @resource.source("file:///nyan_cat.png") + end + + it "should load the local file" do + File.should_receive(:new).with("/nyan_cat.png", "r").and_return(File.open(File.join(CHEF_SPEC_DATA, "remote_file", "nyan_cat.png"), "r")) + @provider.run_action(:create) + end + end + 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) @@ -314,9 +342,6 @@ @provider.should_receive(:set_all_access_controls).and_return(true) @provider.run_action(:create) end - - end - end end From 4b88e290160b17df67e404d2d14e54410650d558 Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Tue, 5 Feb 2013 20:20:14 -0500 Subject: [PATCH 07/15] correctly display source name in converge description --- lib/chef/provider/remote_file.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/chef/provider/remote_file.rb b/lib/chef/provider/remote_file.rb index 7e3816496b3..81dfb770004 100644 --- a/lib/chef/provider/remote_file.rb +++ b/lib/chef/provider/remote_file.rb @@ -68,7 +68,7 @@ def action_create Chef::Log.debug "#{@new_resource} target and source checksums are the same - not updating" else description = [] - description << "copy file downloaded from #{@new_resource.source} into #{@new_resource.path}" + description << "copy file downloaded from #{source} into #{@new_resource.path}" description << diff_current(raw_file.path) converge_by(description) do backup_new_resource From 8f686cc7b4908ee24d02ee7cabc24bd8b2f95d92 Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Mon, 11 Feb 2013 18:02:19 -0500 Subject: [PATCH 08/15] always re-raise on argument exception, refactor into smaller methods --- lib/chef/provider/remote_file.rb | 86 +++++++++++++++++----------- lib/chef/provider/remote_file/ftp.rb | 61 ++++++++++++-------- 2 files changed, 87 insertions(+), 60 deletions(-) diff --git a/lib/chef/provider/remote_file.rb b/lib/chef/provider/remote_file.rb index 81dfb770004..d87715fece9 100644 --- a/lib/chef/provider/remote_file.rb +++ b/lib/chef/provider/remote_file.rb @@ -40,54 +40,23 @@ def action_create Chef::Log.debug("#{@new_resource} checksum matches target checksum (#{@new_resource.checksum}) - not updating") else sources = @new_resource.source - source = sources.shift - begin - uri = URI.parse(source) - if URI::HTTP === uri - #HTTP or HTTPS - raw_file = RestClient::Request.execute(:method => :get, :url => source, :raw_response => true).file - elsif URI::FTP === uri - #FTP - raw_file = FTP::fetch(uri, @new_resource.ftp_active_mode) - elsif uri.scheme == "file" - #local/network file - raw_file = ::File.new(uri.path, "r") - else - raise ArgumentError, "Invalid uri. Only http(s), ftp, and file are currently supported" - end - rescue => e - Chef::Log.debug("#{@new_resource} cannot be downloaded from #{source}") - if source = sources.shift - Chef::Log.debug("#{@new_resource} trying to download from another mirror") - retry - else - raise e - end - end + raw_file = try_multiple_sources(sources) if matches_current_checksum?(raw_file) Chef::Log.debug "#{@new_resource} target and source checksums are the same - not updating" else description = [] - description << "copy file downloaded from #{source} into #{@new_resource.path}" + description << "copy file downloaded from #{@new_resource.source} into #{@new_resource.path}" description << diff_current(raw_file.path) converge_by(description) do backup_new_resource FileUtils.cp raw_file.path, @new_resource.path Chef::Log.info "#{@new_resource} updated" - if raw_file.is_a? Tempfile - raw_file.close! - else - raw_file.close - end + raw_file.close! end # whyrun mode cleanup - the temp file will never be used, # so close/unlink it here. if whyrun_mode? - if raw_file.is_a? Tempfile - raw_file.close! - else - raw_file.close - end + raw_file.close! end end end @@ -120,6 +89,53 @@ def backup_new_resource backup @new_resource.path end end + + private + + # Given an array of source uris, iterate through them until one does not fail + def try_multiple_sources(sources) + source = sources.shift + begin + uri = URI.parse(source) + raw_file = grab_file_from_uri(uri) + rescue ArgumentError => e + raise e + rescue => e + Chef::Log.debug("#{@new_resource} cannot be downloaded from #{source}") + if source = sources.shift + Chef::Log.debug("#{@new_resource} trying to download from another mirror") + retry + else + raise e + end + end + if uri.userinfo + uri.password = "********" + end + @new_resource.source uri.to_s + raw_file + end + + # Given a source uri, return a Tempfile, or a File that acts like a Tempfile (close! method) + def grab_file_from_uri(uri) + if URI::HTTP === uri + #HTTP or HTTPS + raw_file = RestClient::Request.execute(:method => :get, :url => uri.to_s, :raw_response => true).file + elsif URI::FTP === uri + #FTP + raw_file = FTP::fetch(uri, @new_resource.ftp_active_mode) + elsif uri.scheme == "file" + #local/network file + raw_file = ::File.new(uri.path, "r") + def raw_file.close! + self.close + end + else + raise ArgumentError, "Invalid uri. Only http(s), ftp, and file are currently supported" + end + raw_file + end + end end end diff --git a/lib/chef/provider/remote_file/ftp.rb b/lib/chef/provider/remote_file/ftp.rb index 995fb0ba72b..d2ebbc5faa4 100644 --- a/lib/chef/provider/remote_file/ftp.rb +++ b/lib/chef/provider/remote_file/ftp.rb @@ -27,50 +27,61 @@ class RemoteFile class FTP # Fetches the file at uri using Net::FTP, returning a Tempfile - # Taken from open-uri def self.fetch(uri, ftp_active_mode) - path = uri.path - path = path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it. - directories = path.split(%r{/}, -1) - directories.each {|d| + self.new(uri, ftp_active_mode).fetch() + end + + # Parse the uri into instance variables + def initialize(uri, ftp_active_mode) + @path = uri.path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it. + @directories = @path.split(%r{/}, -1) + @directories.each {|d| d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") } } - unless filename = directories.pop + unless @filename = @directories.pop raise ArgumentError, "no filename: #{uri.inspect}" end - if filename.length == 0 || filename.end_with?( "/" ) + if @filename.length == 0 || @filename.end_with?( "/" ) raise ArgumentError, "no filename: #{uri.inspect}" end - typecode = uri.typecode + @typecode = uri.typecode # Only support ascii and binary types - if typecode && /\A[ai]\z/ !~ typecode - raise ArgumentError, "invalid typecode: #{typecode.inspect}" + if @typecode && /\A[ai]\z/ !~ @typecode + raise ArgumentError, "invalid typecode: #{@typecode.inspect}" end + @ftp_active_mode = ftp_active_mode + @hostname = uri.hostname + @port = uri.port + if uri.userinfo + @user = URI.unescape(uri.user) + @pass = URI.unescape(uri.password) + else + @user = 'anonymous' + @pass = nil + end + end - tempfile = Tempfile.new(filename) + # Fetches using Net::FTP, returns a Tempfile with the content + def fetch() + tempfile = Tempfile.new(@filename) # The access sequence is defined by RFC 1738 ftp = Net::FTP.new - ftp.connect(uri.hostname, uri.port) - ftp.passive = !ftp_active_mode - user = 'anonymous' - passwd = nil - if uri.userinfo - user = URI.unescape(uri.user) - passwd = URI.unescape(uri.password) - end - ftp.login(user, passwd) - directories.each {|cwd| + ftp.connect(@hostname, @port) + ftp.passive = !@ftp_active_mode + ftp.login(@user, @pass) + @directories.each do |cwd| ftp.voidcmd("CWD #{cwd}") - } - if typecode - ftp.voidcmd("TYPE #{typecode.upcase}") end - ftp.getbinaryfile(filename, tempfile.path) + if @typecode + ftp.voidcmd("TYPE #{@typecode.upcase}") + end + ftp.getbinaryfile(@filename, tempfile.path) ftp.close tempfile end + end end end From d457d8a0c4ddf2ec1af3059312914ae58b409f6b Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Mon, 11 Feb 2013 18:24:32 -0500 Subject: [PATCH 09/15] add additional exception details for restclient --- lib/chef/provider/remote_file.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/chef/provider/remote_file.rb b/lib/chef/provider/remote_file.rb index d87715fece9..44fa0ca5e98 100644 --- a/lib/chef/provider/remote_file.rb +++ b/lib/chef/provider/remote_file.rb @@ -101,7 +101,12 @@ def try_multiple_sources(sources) rescue ArgumentError => e raise e rescue => e - Chef::Log.debug("#{@new_resource} cannot be downloaded from #{source}") + if e.is_a?(RestClient::Exception) + error = "\nRequest returned #{e.message}\nresponse: #{e.response}" + else + error = e.to_s + end + Chef::Log.debug("#{@new_resource} cannot be downloaded from #{source}: #{error}") if source = sources.shift Chef::Log.debug("#{@new_resource} trying to download from another mirror") retry From aa4235dbd8b0d40d9fea29f75f3ba35e01ef0c43 Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Mon, 11 Feb 2013 18:35:54 -0500 Subject: [PATCH 10/15] e.response can sometimes trash a terminal session. --- lib/chef/provider/remote_file.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/chef/provider/remote_file.rb b/lib/chef/provider/remote_file.rb index 44fa0ca5e98..82ccec76c7d 100644 --- a/lib/chef/provider/remote_file.rb +++ b/lib/chef/provider/remote_file.rb @@ -102,7 +102,7 @@ def try_multiple_sources(sources) raise e rescue => e if e.is_a?(RestClient::Exception) - error = "\nRequest returned #{e.message}\nresponse: #{e.response}" + error = "Request returned #{e.message}" else error = e.to_s end From 77fed4d37369e92cca423efdef7b1a72a18a190f Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Wed, 13 Feb 2013 18:03:41 -0500 Subject: [PATCH 11/15] move dir parsing out of ftp.initialize, don't overwrite new_resource.source --- lib/chef/provider/remote_file.rb | 10 +++++----- lib/chef/provider/remote_file/ftp.rb | 27 ++++++++++++++++----------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/lib/chef/provider/remote_file.rb b/lib/chef/provider/remote_file.rb index 82ccec76c7d..4d1e696d0e0 100644 --- a/lib/chef/provider/remote_file.rb +++ b/lib/chef/provider/remote_file.rb @@ -1,6 +1,6 @@ # -# Author:: Adam Jacob () # Author:: Jesse Campbell () +# Author:: Adam Jacob () # Copyright:: Copyright (c) 2008 Opscode, Inc. # License:: Apache License, Version 2.0 # @@ -40,12 +40,12 @@ def action_create Chef::Log.debug("#{@new_resource} checksum matches target checksum (#{@new_resource.checksum}) - not updating") else sources = @new_resource.source - raw_file = try_multiple_sources(sources) + raw_file, raw_file_source = try_multiple_sources(sources) if matches_current_checksum?(raw_file) Chef::Log.debug "#{@new_resource} target and source checksums are the same - not updating" else description = [] - description << "copy file downloaded from #{@new_resource.source} into #{@new_resource.path}" + description << "copy file downloaded from #{raw_file_source} into #{@new_resource.path}" description << diff_current(raw_file.path) converge_by(description) do backup_new_resource @@ -94,6 +94,7 @@ def backup_new_resource # Given an array of source uris, iterate through them until one does not fail def try_multiple_sources(sources) + sources = sources.dup source = sources.shift begin uri = URI.parse(source) @@ -117,8 +118,7 @@ def try_multiple_sources(sources) if uri.userinfo uri.password = "********" end - @new_resource.source uri.to_s - raw_file + return raw_file, uri.to_s end # Given a source uri, return a Tempfile, or a File that acts like a Tempfile (close! method) diff --git a/lib/chef/provider/remote_file/ftp.rb b/lib/chef/provider/remote_file/ftp.rb index d2ebbc5faa4..3b142dcd935 100644 --- a/lib/chef/provider/remote_file/ftp.rb +++ b/lib/chef/provider/remote_file/ftp.rb @@ -33,17 +33,7 @@ def self.fetch(uri, ftp_active_mode) # Parse the uri into instance variables def initialize(uri, ftp_active_mode) - @path = uri.path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it. - @directories = @path.split(%r{/}, -1) - @directories.each {|d| - d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") } - } - unless @filename = @directories.pop - raise ArgumentError, "no filename: #{uri.inspect}" - end - if @filename.length == 0 || @filename.end_with?( "/" ) - raise ArgumentError, "no filename: #{uri.inspect}" - end + @directories, @filename = parse_path(uri.path) @typecode = uri.typecode # Only support ascii and binary types if @typecode && /\A[ai]\z/ !~ @typecode @@ -61,6 +51,21 @@ def initialize(uri, ftp_active_mode) end end + def parse_path(path) + path = path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it. + directories = path.split(%r{/}, -1) + directories.each {|d| + d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") } + } + unless filename = directories.pop + raise ArgumentError, "no filename: #{uri.inspect}" + end + if filename.length == 0 || filename.end_with?( "/" ) + raise ArgumentError, "no filename: #{uri.inspect}" + end + return directories, filename + end + # Fetches using Net::FTP, returns a Tempfile with the content def fetch() tempfile = Tempfile.new(@filename) From 31e5726fd6913227bd23faf0b48b2d19b306b5e2 Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Wed, 13 Feb 2013 18:49:30 -0500 Subject: [PATCH 12/15] incorrect refs to inaccessible 'uri' --- lib/chef/provider/remote_file/ftp.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/chef/provider/remote_file/ftp.rb b/lib/chef/provider/remote_file/ftp.rb index 3b142dcd935..0d841408c73 100644 --- a/lib/chef/provider/remote_file/ftp.rb +++ b/lib/chef/provider/remote_file/ftp.rb @@ -58,10 +58,10 @@ def parse_path(path) d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") } } unless filename = directories.pop - raise ArgumentError, "no filename: #{uri.inspect}" + raise ArgumentError, "no filename: #{path.inspect}" end if filename.length == 0 || filename.end_with?( "/" ) - raise ArgumentError, "no filename: #{uri.inspect}" + raise ArgumentError, "no filename: #{path.inspect}" end return directories, filename end From 0910034087558aa0fd4383a5f3c303ce8fac005a Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Wed, 13 Feb 2013 21:10:06 -0500 Subject: [PATCH 13/15] CHEF-3859: remote file resource fails ungracefully when no source is set. --- lib/chef/resource/remote_file.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/chef/resource/remote_file.rb b/lib/chef/resource/remote_file.rb index 21e1a2680fb..524e00186ba 100644 --- a/lib/chef/resource/remote_file.rb +++ b/lib/chef/resource/remote_file.rb @@ -32,7 +32,7 @@ def initialize(name, run_context=nil) super @resource_name = :remote_file @action = "create" - @source = nil + @source = [] @ftp_active_mode = false @provider = Chef::Provider::RemoteFile end From 8dcd04c05f57ca3f89893d0460fb867f2073df57 Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Mon, 18 Feb 2013 11:58:57 -0500 Subject: [PATCH 14/15] functional test for CHEF-3140 --- spec/data/remote_file/nyan_cat.png.gz | Bin 0 -> 14944 bytes spec/functional/resource/remote_file_spec.rb | 43 ++++++++++++++----- spec/tiny_server.rb | 21 ++++----- spec/unit/resource/remote_file_spec.rb | 2 +- 4 files changed, 45 insertions(+), 21 deletions(-) create mode 100644 spec/data/remote_file/nyan_cat.png.gz diff --git a/spec/data/remote_file/nyan_cat.png.gz b/spec/data/remote_file/nyan_cat.png.gz new file mode 100644 index 0000000000000000000000000000000000000000..efa9d4427a22b81daf06e50e0ece023d142d79da GIT binary patch literal 14944 zcmV-mI-kWKiwFn~OCnJK18#X?ZeL?zbS`jiXEFe_yLC_14X zG-M)VC@3g2Ss4knf93Ok?oTlPo`~^Nw*Lxf7d2@ysQPJ=(|;8>tM7{6p`d;xpuC#G z|Ep7&%cv~?^BX!Ej`e1Cc?)v@RGal^64hG(xmO=A?- zqD@j-Ix2a597m4f*IFx1I_0aI4DT98oz8$+I_xECupxc z6zL+0N+>irWqzU=LP9JyQvKEV;b;PDKpI99F&*s>IA!^)2^c=347u)Pi zK4R2x={^5jg%Q6S;cc+mC@V!&iv9Paw6RjS;0)2m$a`Zmjk6#0M;sWP8Z+EJYf|&4 zN7jNE!5EKP_j#UM79OhAkua@(x7<<0R?5?#Xn_h;G@pav#AMD8fv8~6s6l}Cbd-8U zfWR}|z*s^ZRi1Ph45S4wWBU318uI|Y`+hot+P^E*XX%5h39baLniaDMG%Bo7X#)YK zJ0hzBEAl-48tOfg7Wh;B{?~}L2@V@c!;qdwhZG6h+vsDqyC%P&+a6nN^h8?Lfc}in zNiF2A0@ULPdmH@jZ%lTYL;Dj_{bBP1X{iCE8%1(6d*TpHK9dh;+MZD%s}S+FKaGx| zEaPQi#t=k7qhi<>Tt&Lz7){}W+(;IUpL|EO;MnwstaQ%UFn7Y4nHm~}n_C8+B1N(h zj(W2Uj+r_r+t$`wBYHzCXl?*lM5!;pJ#0H$Vvbo0Pxae|JCrh(#*JKkD?>3?L;OMN ztU$4~#u=*;di9snLMT!SzTly^lifSY)i>U5KkzUo$CYR&k)7yM6EYjpV&J#TlJetK z?d=x{Vwp4VsoYtTyELCP3swf`5IEaU;O>#SfQOKU;W9cUJo3N$5~DR>svGPBt{|e% zJ+*rcb6~cf-`dt53h?KGd;WY(QZP4ciK1wlS(#gwt3urlp6x4tB76)!RP1{h;IKCj zE41Va!)a7pG6$JMjP?nJ<1%y__EqBG9_bG+H4rg8ORm3 z$`rOIk@=ePd_1pYSJ`)U)0uL2S{b=3_e)qur#& zCj$Rg!-*pVcn?H;WR)QJRJ%X|4Jj9Q`~X)NhmF0rZKR6>Crryl2^!Ibzton!4vD${ znW;wm@V~jlF!pp&zREdD4po_k)aE%XbB=3<2K6~lVUDux=aQ=wa7}QFEP99|)Bl_L zWYc}#`6&(&-533?x2 zxaWCk)S!Vvfrz_+TA|TgyIE!YuN~G0_wQx@4jJ?}8K@WDB-RoumJNEI@}V^00JKvM zT0!*CChJl=sd?tAk>o`C)P{xOi-ALy&rib;fFd;cmJSeD&m>^`k!f; zl@II}@;6wqJO1b2Ed(;&H<#Vt4w$*xM-zPPj{TEJ4|6>T1%4R~2z^B~8+d-(1(0ta zX7foWuqP^m*LI>6Ez&RA$?-g!Oq@{+cKDJG=(ROO_hFRrMzOY1Y`$OB&QBLonl8 zMXfhdTK&55$T)BLx^5D@98#{$|M2xJYneogk=#hq)fOK9G@$`krUq9*2Zgc7#tc1asNArp3U>07^B(UMNnk@YT9vPD`|{06XzY@zIVJB8&WvxEJJSU>Q%E z;{=b`9%I9!Q(r{ynSL|0@}L}3Zx3#;Z0AG^W9Z5J6Hj^vM`Nkx2u6UZdQvG%ONVfmJ^IRj;a;$DT!gVEd)H~;< zDr`H~tj*V&uyGNve(}+~_987kIyAI6+zaS1Gw-+2i23%&t0ZTxAyl9uRJg!Vm3I}v z&o|jBLAv;6tKM0-p&8wWQ{V*t`;M(Ch$ixM8 zAIV|Dn_1~dBw9Tn$CN<KD+(SXmcbJcd*1)+ z6N=6s2N>xP+5O&76FZL7K)E|If_-nUpObDGDV9R^$F6igUwn4_5o)s=eUb`0k<{KRBOC$2fy?{>{hD@Q#0yB0d#AP@rw?8|(D{w`{e?dFD^zqO4wod5` zsPi_-dp=ZW77PS*KM#99^eyD805LJpRH^;-GBTy(Y=Zx&8RdB(?L=^lcnwaybDz!3 zG!*a(hbQWY73Z|vep54D)gikHV!>92(j3mLs>eT4Xsq*7Q^J~`G{^7C6R0^D3|7Cg zz;m2nT+5GZ^wf91dDI*hz|XkQ3vSE|yBPU=I3t9S)c2WtqL|5)gNd^eEB&h_fq8D* z|#zGhPc;D(g9UH|jK{QmcR|D&!HQ**XFq)m;vcch~i0PX6Q%pWl^GP)n%yZIr8 zB2VC(jBOFq)&2~8dLNW=_iv}@6-}HG;6Fvcrc(jp6Zwr)mW`8%D|NgkV)0ZFh@b?F z_#$&meJDgX3pb-t(aVAfq$S}IBy@dLD&Qg~@<&V9x!c%t=7J*tDDd;?4^w#Z^4qtv zWUw}8MRb~*paxN4NfKrC$Cz=piQK2$@^$%2l8wn@>t@Qzh1c4Wt6G(S)k^;kB~S_m zH3QH&$Sp0bV?1AT+|H4aNt0V$Q>zMD%-=jRTNpLB>{a_?m`MJQYMKWRBe0pkTma+? zbAL9v(!%9`N!Wc^#Ul|T% zud4IJy=qFtG!{)oD1VHN(yu>%Ln?a?Btztgv-#beWa;%7lz*< zwT0v?<&IrqdC|%8Ht1!a{>CNOoRrXF7*;n)l}Y9%uAb(>&%wN7&$lP`b?IE`;gs#* z_yw@M0T|5fA;_Kbnfe`{BxEmgojq@x#**mF$@N}AykGEzeFP^YWfR`x}0GbQ`tid|HL zWGH^ZuVfeb-yoNNmT5j44Y)Aca3#*`!1=yb0#;}tb+ z%qi=Jy!2Y%d2f`8aWH98=yfH=j#4Ex$Pfr0#)A!%ikazbL85Qw3t;ErJdn49u2`zU z{{nh*2#64udiw((fqMQ?gx@Zp@8uDq2f&KdzBT#HSzFV=C|0R2C9HZCA$Z9Bq48qL zB+EA*cAiEvn&zuHEUpS;M3zylv{gcF$>i6ZnzN;G>zN*HtE#(Hi{CG&18Ht55nY4N z*KNa1&wS3}vg|?(OiKzqs}%Hx4d{uS^%ZmZt-1c; z?|dTQw7g&M7FDIaD)-~GDbT#Xattj5twD-nEg4)dO`pF*uYe~R7=8oCtOJ7^L{L#f zUq?QN?e+s;QK30L{YB%lzE-_suAB=fI~0UVkuoPPk1DJXH39^972T?S zKYbLmtUdmaMM;E@=$})+ol{rfwi*e8TZZb&WY~EwTRt4hB&pqTveNOjd`R$5b9cJ<&-N| zKgGMegsP%*NDL3&&N3^q)4(b1L$p6);ni&#?eFwd3b!)(o^Hh+N`jj%|BiPKBHwo| z^IB{cnm&n`|sW$(PTR5+eSP1bs=i1)yYMUHc-gLMR+wxs5{2C5oA2T zj=ci53$W}Kwd6YyOkkG`wX_&P;$8E5P42hzu>9420!DO~mX>^(t~Yjl`=Te2RIYFh zrICGHI^XD%@Fuf%Grca z&7;E|^pZD87MO>&8kM-+mqp;aC>XO`gj$SEX#b(pSaE|VxFJYGWHAfws>xPfA!X&Y zY?-cv-u@n6?VUi_k;dV{Jz%{Cx$Y@{$XRFKOZZr&pdWcF`CQ*+Q`V_vVRIeKu|iFp zSj^j{G!I=>@za4L$-%IGFd?*@D;a&>g!tfI+X>o&E_;5Sg4++N@oG> z4%Y72E(KG~a1t(9pb&Nj_0J*F-#9EejRS5=Oz;4*p-{go*Dwmz7Wrb?f1~_HvZa3& zdA|yt1!rFx)dBERJ{7hmu!?vyzpOsKwDOA7c;)fCoh{eJeAe#neE7`T<8<>Y*SweQ z<85`^TxN0y{nGy~hl?-bkx7MBN1FU2t{V!Wo%0fXX41By-gcsAQl-E1X{aUBh5uG= z)9q%X^N~!Ob7>lDN3dKbmxTNi7-0S(YK>G>fyBZ}2d^9)ADw!rn+SpP@EAB(ivvW2 zOJ$AnwefESA7hiClOG|1dq!fj9b`% ztUsK{I6gQq@V+T~!d)yiN~!bs8AfV+#$47I@HOVN+Uy7zO1OXe$C)?Ya`I$%nr_M@ zJyS?;wvLdHo zb-DF;AwZVKYZi=u-<;|n=XsDp9Xog23TitzfU~*n!i~5zbVyYnE8DEP6;ZNCXI?V_ z*2ONdpYsV5a+0?*i8#&T3GDIhToFKzM>OM7LJzz zg5nv&(MdZ2yJD%~D9b2DEPj$ueg}iyFzQ1Jx>~Y86boZXmp=*cnp#pYpU;;3AD(wS zdqlR@4OcYlaxbTa!pFze?*|_2^QkN+Hb2l_&+QANl?i8CfCwJl?)ai`a45*gCc8Me zxVwth+fh@hBflL6_;>Rp_@IP*Q)4%I%Pe$T>=)>C2IU~8nWsDi@~WyjTc*SuW<%=A zLNv%YI{4lov2{|Sq%}seuM&!AA+~`)_QY2W+k+rFl*x+O^V!+{6h|iCooPDE6KPFY zB>?X_c#@@(zD8+oBEKTjVRl=R{1shX1DsEj!_fB=LfG)U02b><;e@61{*}k?js@7x z>{0cL*5}TY*6kC2Q_Qb^-w*8Mm40)6vtz2TLzrwd9W?3tVPO#k^BW`b(Nk{4{aXwcfqT%v6*>$5{ouaHYhz@}%h3xt9?3K1 z$5cpzp!Z4Lp|VJN{Jm9yAJ0G%l{5@kETaT~!t6PueU?A$+B zpJ?XkEa(U#Jgpw^onbVUPZ{}HhTYcxy*_exW+#>~bT>gMOW=VyeAV=*PW*08my`l!PrQDVfR{FNikA_J%xa9;g9<54rut zn!A!WG=+*E>;R3@ds*zRnAwx@dmc+J^S5hLtQf;)#!k3RUVHXmx6-iypWRixr5)0w zYtDL0`RX+sB9LB>v$QFXRM@vDzMFh5Tz5^s-#~yW47l5;8FR;^zTOZR?HFv-5aYvS z3xRr5y8m!jzeS*NRKHD+-_jC`jE;R~{wO-2_HZ~ui-J0VAbJ4MIgdm>D06sI@Qta% z!hTg%-T198<-DC=;?mO6wmN|HIHAgwt#tDP^23PuNC*O)FgX2CHVh{{FlIF^_63lM z2YVrQiNRHR`45ZWQzmG9t(bb-#8zN=QIP8D&5Ch=(E2BJBfBccl;H+#L$|a?UG_Hv zYYdj%$j~`81dP0h7Hf`(eHl4DwfVHT>LsOZ>y<~;hSk^_n>@{A`9##Q#lAJ2`c zbOM?Roe)6H!RqSKNno9Eum9!xZ$Mj}ewz}?&5wn0MQmDy9o7zOhozG$1NrsgcO3iu zIFp@eZ644q`sE|_Wsd~m16~et1IuwurGP$PJId9;*-{PKPc8|`J15FA1ZyM@Y))~< z%2F5C#|mmqWjT7)q`4HDu;l*K*M_sBME1n!vA?jgt}QJ5*)6ti;A+j99k0LuqQK8! z9?XF-MA)5hV!;jCvskD;VlVjJ9ujar0jF;pYIbWLEngLtbk;SQc(0B>BQxG$tqC)9 z(1K7+3p23g+xdNzCS5d2beqD48eJ>;>)z#h>mYjyPxYCd{mD-ZE)}~x>k`Upp~Zxp zY&~i3UZ3OXynb1(_a!^IMnnG$k(=^!rh`&0M}U-9VKC?%j<3^^s`?9s?3160YQ<(CqIk{MA*1L3?r*+ah8*CC7|KW+6WBQ z3kr6xw%b_@t_ELon0;YRl}8U>xzU~)h0XVkRo){EAHY!=rxiwyklX5F(n>-oi?GAy zH{US0FN(nAAU7BGytzJ_LbX+&6+;hD9PgIr+I5Y&9$j*}pM71j40+{WnJgltDKyxU z+^!N@9Y}vY^!P|6;bsb~mhBJOMyo#q1=RQ{;jl=pZY?VorW&L`q2=X9S=dP===LD8 zw+J%GUQ~O~A?(C%G*f+FMNS@gAY!fC-UD(QA2_JMuGeUpdTvjDb*njVRwn7Cowjj5 zXhc8gK|y(!!-}yC>B_(CYzfJGyI)W)38B1j4ymeBOvIs59I(D2#pbqlP`|8aljDpC zU83$B*nyrbENFHe+e2_$RUiX*_9oSx#C%SnB>>s0Tx%|aal0tPZ>Nk1ZiZzTY2AS5 zahN>;Y&MNgQ8dK74mbgVl>mc1DRSJm6{d!+`Nhn?S{qTKcaxD}F6=$S_C{#qk3>m(U8c%Wu*@# zrMg5m>C=N;!~+eth?!QMl z%~msbb)NTg(vk_A1&8MB5s9dh|NO;;{f(PkE6R{wD(>QpbX>& zIvP>qP6%|-LYa$<(tW7>LR)Q|8n>8XSl#4O=0GjcYQ%=#N)PLullIpj7kULv8ve{_ z*Or0>d&5dKfP%zy3|=5tE9e)Bva*;Y1G5nkb1SzuD}<7&nKwX>E2FwfdA4p1oUkaa z*?T?JVtw)sK!XnN2FYShs!H*f1`c^m0bn*CoDc0OSF^2-=Xkn?{rO!?oqt2+N$1?$ zLy4}nlq!en$=rYuxv=lILJF;h;o)H$0B?Ntz+n5eHVsV%ZEdMg#@xBl!}=`OpS(}E zOJ94AANuJ6?0u9rOj-mKVuXAibN5!r174l_;qe2l46eUImq6#n7RP*Hy+PicT5J7D zJkIB|2Pf8bTwFOjBU)@Q?tOoIF^{e_-XI;6O(0iAXee7<8BUUSeCOTH199f!l)58sa{Qz<=4-z$ zBm=+OdvF*-g&?svnRbh!@j7l906w+9EPxwNGnjRQ!{n_oS=+yb8O9&G98F>h5gJ&x z(<1{QOW<@)%jEImom0El+jIBhUzG#fm+%3rhPkU#zf_8!nWZn`M~k&Tus);_pT4A2 zg3z9$(HKjPx8#8tYbtuLbMYMK=xcZVmvgK5?~#6U5${g1LYux^?h_m@!zEdkgULQu z^2Z*KHBVTX;ieDG3Y#OFXq)bJ^uF?SLy_w07Z{VNB(_b(-@p@Ym<3PV2f?#lupMw8 zNg-*J%m=kzGpR;CgSB`9MR{=;-?)}dE|%w0`}t2uB)FQfFE?IruCyr=XLxHT?D9vL zoHI^x9Tpv(lX_GoT?xyT>+2lrShP@-6tL!TFoeOJR(HJkALJ{;%ldulnC6VCAJT;_H-~|EH5*MKH0Z&Z_d zNF#%@6>#pq&``xVK={7ToR*0wCd0uRo>{Ye{b5zk>s&c@^&JRdL$~S&e-&(a7TMR; zMq)*PBuCu2WN5~|5w*pwU1rJZQqFw8h>7Gh3d&LsRDAkxLig*`+u@B3yJ1q(0kw)6 zbC`Rj)NPA{6_x8bm$_gU1-s+s{69y@hA`e(mb0)Df|bDLWRb#R0zSgwF zfYs2O4hb0XU8`??XG)G(K{|}l<8nZ=zca>vlQ{y@xrnUH`Op81%#=6Ti$vXPM{lFT zLYK~bz-}{zcvzfpo*mAZ@2e58U#w!p`!KU`TYFUATx@PyIXnB0O}z=x9m_a?F_k^J zcp<}d1-_pJFeh{9+&>4-z)>2=hWo+Yv&sbKTpIMTS$p4L&kvTb$e93%CvzZun;jpz zt*F0WkE(98EAFgKcfo(}W};Ma#CFTm z&b+QiGlP^_#woz2n(P+|m%eE@)k9?{n3pbT2#{M-+QIOZcMNmqD@v+z*#Wl@&df$c zUt3|nivZtp>J6>2mKtO#(rmYq1>NkF68^kuyrZKpo!x~L_yU;MYr-cEX4MI+pT^js zrF59hT(wR-Ilg~7{hey|LahpuBcP%4X~8PMU$_RxEy}yOSu^82=kHyP6zyh~UvPs6 zox6FY^DO)2v^XSgw(z@@q>*@fUz)c++EAt=(&qTQ4%%dEb8KLfd{pxy>6dKwNn)_& zC}DeEf;~1HWDU*?(PSzWUq5}XCF>~-hmPaHrDJ#cn6$CpHfWBMs^z7bkFUbn#YRa< z*TEs{lh=J-Zf@G@w?K@-;$vhCs!1$pv%y%{-GHGmqpHC6_Li}tz;CyE+M)JmWeXi& z1YY)YNfy<{Ys9;0l;~KP1wVHHFk(TRaF89N%+E%Q;z31?m-UjANP`o*;HyTWUCSUV z1gj{PeYzQw2C27yvc=+bwGPMUF27f+!y0zjq}pm?9tyDiXSW{8m*kN7V6KjUGMHhY zo9U$0j2U&MR=3#~#L%eR7>v}5PQVW39^=o-ar;Gx+5a^=F){FZ%jb&4wL^}9y)gH z*JXXq&b%zSsz%N@1bJ+;`2W)y!V2yjRZ`Q|39e8sE4%b0f-%~CmWU4ps%92ks)Nh@P;=VqJE0Chx-lNZH4zsK0lsoJ^UM#5- zaL$4&I-u(SZWCf;xa*|L{Oq7Qecs)eA!?YnN=6r5JY3Ud?SEm+^?Ao}e#}(p146>} zq=DbV(mFbP8_1n_Cm!F1ydlg_HZ(ipuaVA^#SjtaJ5Jrn9j=<%(Ac>Y5)I?6w^a}W zdF&B7ZQNsje_Xz1KOoHh*oWD(#bheK~Ct zVU8l|)V((g3`GQlEO;fKYg@L*BuYQ5lbTD3300JpMZNX4fEk|48v6N_&S^se6m|Gn6;VT{gG0%o~G517x61bKrxUmzSNhGl6NF-De)v;E6CG zW>FFRCI=53%T~LS`N2G^#I7B_c|DZtU^to|c4WqP15|M4kMO?ABbeUTy_L#wYX{=*p~x6~BD-PFMNnEJGy1ZVQerH~iPQK=QsUaw(K0DD|5 zXUY`nHc&L=W#z-O#op2X=8@T5=5^XYX>EK9*L4Q@dCu&4+0NcPi`zGXu+|dm_?E+* zlz7hxi}ym%t|8(}bmGs8NK&!_HRIm>2m076vjB+Ju~5ekq)mFAb7%{UI|OP=PUHr@ zop?p_)!7ECs-mk*On=;rE9JeYbSMCRp5X;v6wytVPFmVhXP(c5CKf32K2H=-u1L+I zF)S}{fXq;6Ux+#6AUNkMw>-<#P^#{|(4xW5Ao*WvJjp_KQ1L6NNZ_)5b>dPn|CJn? zs``jRGY-Q;#{91k>&}oBGo?fO?d)pH(&`0-%CVnh7N7eL2#fHC$6${0-JU-3HF|X- z7_QFh4a#>s#72+l1#V9ey<9!;{2tIc6e><5Xg@g%{^jEoz3d?02`* z85)o%@;AC@6wq~6Cw1co;R9X3iW2m>QS656ZIHe>^4t*r_&3lnHD~6N*&FL*(KP8s z$(!e2`(J3XSHGwU<%EJY1a`LWoqnH+m7VVAuEq&?Kf;uqri?!XsFy!#&8MX4lX|O( zM@7dn%kw)Vr$$xDv0!bUz-tl3*}*9g*W|uK4|r<07~8z3M{5Ggq19hh_oaEjWaY#?@w_=FST?W$&u@O86W0uM`+& z$6wh>ceN;&@uy~WdcOzAkU!OUPUZ5c*FyR3MbW8HEprLo&x+RxJWxMmub=VI#BDTT zXMeeB%x3Vz(ZT1xjo!*0x~GYV%!jk)LMFi->zaqFUp&_CH=+<8ZV>h*T9eYBC$&gP zNn^FjnaR3QjcM7a?4r*(Eu8Az5krjQ-RK9C}ij zd2-@yhE3TVqbOvr(l)KN0-_RCdOLqoG&TcfJTH|0uPl7?UnVY!03X5po3;knA=UR!{dh&(n; zzgLMlwf?`-Vpwn*|2r!taXXS81y8ynmvy!LGSjNn~tCT8Jf zIKF3wU_j0hJUw(XQlrjGa?!;FoyT@MDOIdXIFcJFxdwf?I%B5{w2&fKQC|7B!92(HTzc=cItz2A$b2$% z#PVh6x9K2)&L@H@%y^)QFwAaV`NQ8Ieay-HdG|T}kd!DKe4vdu%=wXQ{Jd;D7Wr2} zR1(~^?s>qmY#KkX$9~X&!#r&X-A9P3%vA+we}RSbvmpj>k?C}DP-lQ--0Y*y@v2Nx zi4ubcZ08bsA@Mg5;Jh@?%$lim$kQ<%!0$_;AgT#v^plhxwT}2+ydB6b-IK8@6l7@s z7uQ{V=b+=-&MW<-Qbz!K7ggS#-dgFEPJaU$^)SjN$-c>M7J}j7d4Iwc!M~5Hf^a8) z2I8iCaqn8Q%$H6*O;|dWJ*&lWvlhUI)HYvIn=*?2E&-N9ApOUN^`M!7&)%8mk0&m& zi+rU@^7AbvIXg!7tI1sU!-MIin7ya3yhjmHCa_WsM{oV4It3Rh$)x(7e9k}tEe{tQ zO>Epp*ILs3C)luagIvvEf$=c_y*Y`r4`2S-5g_Y2>)$YdQ2ZyW9eVy+Do9EgU1F)*g4m??mrM60OW!`v zQ@V=P$K%7M7PGE`!=pck@x|wT+H}j?NLdPtUPt@#3r(Wlaw^-j4aQ?+%p<CrMf5fVqFDJ^JaI6ki*^oFjnb3%b43y9c&#iUTc!xvW4dd+r`b->^pv@y34ZfN_CNdIZvMM+Qq z%S$3OCt<7!T!h>oc-YQM(N*P2yPd-C#b4@JePjCAEhRd4w;sM~;ol`~%boFJcksJ` zV`Y0~#+q>Pa1JdT4SFq)?imgZQFh3p4OUr*B>xyyd^aJr-D-f|QL*A%yd#mnjCWFv z-?)e$TaqCB6;HTNN@vNxc)Ik1wkTJC^vrX#z>SpiGT!xHf4*KYEZD0ok+oa>_+f3R zIj1~)ctVUJjz}fcxGF2y+C620ShiyG#r4>0YG%YkG#CAg7V558uU4&xCBTtP+X3$U z24_-;;uA&d1(TEq;)60Ku8mui%eV2}c(w#UqB*j*f>2V8`pa^CFia`V-{o3E$fnE8 zOtv5Xh6^+;$$-`vwMDh&1>QovZS{UZdFq~LS?_)U8Yu*86pnafOQ5~2V($lPd^c&% zGv9j@a-n6*B?k(008?5*a<3 zwZc;Uio~9Mg3X`-_YixQ=fw1=IR0`2r=?H!Jv8%FLt%5PZ`Lmmb%roadkMHfKf!ywD9?4e?HeLiyjeY>2qzWtv^k53I{ZVXRjIj5iHO8z>~6sIys*y z=51*F-91)L3lr>F6=>+G#E)8ESRXImb%upDgcI0@Z=YZ7La?l*=vG|)hzuj8gwQNi z*bDo))9*;#$m?+ksr39>bj=5(LTW{D)9Ia1AE-%v&CeEfEEZ65-Y81Aztb9>s-Dh> z$Ya-+r$2Ina`raW0|^qGB~cD)Hh0#1`1Zg!%IK^)M47Wp+_BIb>;=sX>kA@1+>G4 zfH+qK?F#BecEE5zC|Q(Z$A3Aj?domsWiMH6)`#3*Hy|<8r;kmanzB#r=iJWwn05q? z|Id#DJ0y=a%{T>dt&o?Vh5($ANyL|)BtQlr!MYtItB8uKj z&BN1i$9@5g%GAnVl5MSn(tVjO@L*3rDl>&YZxpC1^xn$C5!l>L(L@x~I`OC<3j6-1 z*52kyX787YO4*EzaK93odt-8F!6tlG`Z^X?Y%5#x&d5`{_1-H7{=R6ZJVxQPfBd8S zY1kt|+RKn!HB2&}p`+IQ$=BUgS5I9K{u zTM?n;-ygG5pPT&Lnx{L#wzI8H<){h!A6pA82u3H_b9^wuGC zyYE~Cp?`WosMhMic?~-)@ zl;uWe=VxPSw70X6I!)%W75XP+;h=?wZ}0HSv1YY6NuRGUce8?G%7YBHFjp^77ercr zY;bPpDMGO_Fu>m|DLBJ48cb=#2xO=EJ09M~k4M&j_4Dh~NF2CRFP*qAr^oqR~dCqK;i;&%A4h!MGKalXlZwe%2iZigVu&Co4o~{Td0vdAJ zE$XvVL=SDv014#V57~$<<{>=2-S~8Ca+gMYUgHMb)IsB#gF9P3X6F-E?=xVwt@Vx% z5g&BH+!ri#-{15-_PD!nq7GW|{?|y@6*VO>FE#$JV|d}XQML8|_a?P)@8rs; eUr-T6`17IuU9yr&67^yx!T$$ATt+`)I{*Np_jf`7 literal 0 HcmV?d00001 diff --git a/spec/functional/resource/remote_file_spec.rb b/spec/functional/resource/remote_file_spec.rb index 57a5321ea22..fbb921d48cd 100644 --- a/spec/functional/resource/remote_file_spec.rb +++ b/spec/functional/resource/remote_file_spec.rb @@ -23,14 +23,6 @@ include_context Chef::Resource::File let(:file_base) { "remote_file_spec" } - let(:source) { 'http://localhost:9000/nyan_cat.png' } - let(:expected_content) do - content = File.open(File.join(CHEF_SPEC_DATA, 'remote_file', 'nyan_cat.png'), "rb") do |f| - f.read - end - content.force_encoding(Encoding::BINARY) if content.respond_to?(:force_encoding) - content - end def create_resource node = Chef::Node.new @@ -71,13 +63,44 @@ def create_resource f.read end } + @api.get("/nyan_cat.png.gz", 200, nil, { 'Content-Type' => 'application/gzip', 'Content-Encoding' => 'gzip' } ) { + File.open(File.join(CHEF_SPEC_DATA, 'remote_file', 'nyan_cat.png.gz'), "rb") do |f| + f.read + end + } end after(:all) do @server.stop end - it_behaves_like "a file resource" + context "when using normal encoding" do + let(:source) { 'http://localhost:9000/nyan_cat.png' } + let(:expected_content) do + content = File.open(File.join(CHEF_SPEC_DATA, 'remote_file', 'nyan_cat.png'), "rb") do |f| + f.read + end + content.force_encoding(Encoding::BINARY) if content.respond_to?(:force_encoding) + content + end + + it_behaves_like "a file resource" - it_behaves_like "a securable resource with reporting" + it_behaves_like "a securable resource with reporting" + end + + context "when using gzip encoding" do + let(:source) { 'http://localhost:9000/nyan_cat.png.gz' } + let(:expected_content) do + content = File.open(File.join(CHEF_SPEC_DATA, 'remote_file', 'nyan_cat.png.gz'), "rb") do |f| + f.read + end + content.force_encoding(Encoding::BINARY) if content.respond_to?(:force_encoding) + content + end + + it_behaves_like "a file resource" + + it_behaves_like "a securable resource with reporting" + end end diff --git a/spec/tiny_server.rb b/spec/tiny_server.rb index 9eecf13cec7..eb5f5c0fc0d 100644 --- a/spec/tiny_server.rb +++ b/spec/tiny_server.rb @@ -127,20 +127,20 @@ def clear @routes = {GET => [], PUT => [], POST => [], DELETE => []} end - def get(path, response_code, data=nil, &block) - @routes[GET] << Route.new(path, Response.new(response_code,data, &block)) + def get(path, response_code, data=nil, headers=nil, &block) + @routes[GET] << Route.new(path, Response.new(response_code, data, headers, &block)) end - def put(path, response_code, data=nil, &block) - @routes[PUT] << Route.new(path, Response.new(response_code,data, &block)) + def put(path, response_code, data=nil, headers=nil, &block) + @routes[PUT] << Route.new(path, Response.new(response_code, data, headers, &block)) end - def post(path, response_code, data=nil, &block) - @routes[POST] << Route.new(path, Response.new(response_code,data, &block)) + def post(path, response_code, data=nil, headers=nil, &block) + @routes[POST] << Route.new(path, Response.new(response_code, data, headers, &block)) end - def delete(path, response_code, data=nil, &block) - @routes[DELETE] << Route.new(path, Response.new(response_code,data, &block)) + def delete(path, response_code, data=nil, headers=nil, &block) + @routes[DELETE] << Route.new(path, Response.new(response_code, data, headers, &block)) end def call(env) @@ -183,14 +183,15 @@ def to_s class Response HEADERS = {'Content-Type' => 'application/json'} - def initialize(response_code=200,data=nil, &block) + def initialize(response_code=200, data=nil, headers=nil, &block) @response_code, @data = response_code, data + @response_headers = headers ? HEADERS.merge(headers) : HEADERS @block = block_given? ? block : nil end def call data = @data || @block.call - [@response_code, HEADERS, Array(data)] + [@response_code, @response_headers, Array(data)] end def to_s diff --git a/spec/unit/resource/remote_file_spec.rb b/spec/unit/resource/remote_file_spec.rb index b626a73882c..064c97fc279 100644 --- a/spec/unit/resource/remote_file_spec.rb +++ b/spec/unit/resource/remote_file_spec.rb @@ -42,7 +42,7 @@ describe "source" do it "does not have a default value for 'source'" do - @resource.source.should be_nil + @resource.source.should eql([]) end it "should accept a URI for the remote file source" do From 6f6fff817866e1fb5ab3c90862bfaa627b5378fd Mon Sep 17 00:00:00 2001 From: Jesse Campbell Date: Mon, 18 Feb 2013 12:18:50 -0500 Subject: [PATCH 15/15] some cleanup utilizing the refactored RemoteFile::FTP --- lib/chef/provider/remote_file/ftp.rb | 32 ++++++++++++---------- spec/unit/provider/remote_file/ftp_spec.rb | 8 +++--- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/lib/chef/provider/remote_file/ftp.rb b/lib/chef/provider/remote_file/ftp.rb index 0d841408c73..3c5d3e0a915 100644 --- a/lib/chef/provider/remote_file/ftp.rb +++ b/lib/chef/provider/remote_file/ftp.rb @@ -51,21 +51,6 @@ def initialize(uri, ftp_active_mode) end end - def parse_path(path) - path = path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it. - directories = path.split(%r{/}, -1) - directories.each {|d| - d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") } - } - unless filename = directories.pop - raise ArgumentError, "no filename: #{path.inspect}" - end - if filename.length == 0 || filename.end_with?( "/" ) - raise ArgumentError, "no filename: #{path.inspect}" - end - return directories, filename - end - # Fetches using Net::FTP, returns a Tempfile with the content def fetch() tempfile = Tempfile.new(@filename) @@ -87,6 +72,23 @@ def fetch() tempfile end + private + + def parse_path(path) + path = path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it. + directories = path.split(%r{/}, -1) + directories.each {|d| + d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") } + } + unless filename = directories.pop + raise ArgumentError, "no filename: #{path.inspect}" + end + if filename.length == 0 || filename.end_with?( "/" ) + raise ArgumentError, "no filename: #{path.inspect}" + end + return directories, filename + end + end end end diff --git a/spec/unit/provider/remote_file/ftp_spec.rb b/spec/unit/provider/remote_file/ftp_spec.rb index 368a7346c48..03ef9424dd8 100644 --- a/spec/unit/provider/remote_file/ftp_spec.rb +++ b/spec/unit/provider/remote_file/ftp_spec.rb @@ -36,22 +36,22 @@ describe "when parsing the uri" do it "throws an argument exception when no path is given" do @uri.path = "" - lambda { Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! }.should raise_error(ArgumentError) + lambda { Chef::Provider::RemoteFile::FTP.new(@uri, false) }.should raise_error(ArgumentError) end it "throws an argument exception when only a / is given" do @uri.path = "/" - lambda { Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! }.should raise_error(ArgumentError) + lambda { Chef::Provider::RemoteFile::FTP.new(@uri, false) }.should raise_error(ArgumentError) end it "throws an argument exception when no filename is given" do @uri.path = "/the/whole/path/" - lambda { Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! }.should raise_error(ArgumentError) + lambda { Chef::Provider::RemoteFile::FTP.new(@uri, false) }.should raise_error(ArgumentError) end it "throws an argument exception when the typecode is invalid" do @uri.typecode = "d" - lambda { Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! }.should raise_error(ArgumentError) + lambda { Chef::Provider::RemoteFile::FTP.new(@uri, false) }.should raise_error(ArgumentError) end end