diff options
author | Jay Mundrawala <jdmundrawala@gmail.com> | 2015-05-21 11:47:02 -0700 |
---|---|---|
committer | Jay Mundrawala <jdmundrawala@gmail.com> | 2015-05-21 11:47:02 -0700 |
commit | 1502cedda53800e225e5b1f25e568f96eb8648c3 (patch) | |
tree | c614f6186f7eba48c8839acb3feca6a0a00aa104 | |
parent | 605552d20d7dee84484f9d197d11a42c05d0ee1f (diff) | |
parent | 9a8bddb8d0a19cc17cae04f61e325873aab82fc6 (diff) | |
download | chef-1502cedda53800e225e5b1f25e568f96eb8648c3.tar.gz |
Merge pull request #3398 from chef/jdm/1848
Allow spaces in files for remote_file
-rw-r--r-- | lib/chef/mixin/uris.rb | 11 | ||||
-rw-r--r-- | lib/chef/provider/remote_file/content.rb | 5 | ||||
-rw-r--r-- | lib/chef/provider/remote_file/local_file.rb | 14 | ||||
-rw-r--r-- | lib/chef/resource/remote_file.rb | 5 | ||||
-rw-r--r-- | spec/unit/mixin/uris_spec.rb | 34 | ||||
-rw-r--r-- | spec/unit/provider/remote_file/local_file_spec.rb | 31 | ||||
-rw-r--r-- | spec/unit/resource/remote_file_spec.rb | 5 |
7 files changed, 82 insertions, 23 deletions
diff --git a/lib/chef/mixin/uris.rb b/lib/chef/mixin/uris.rb index 6c1ae793e2..0136b55f6a 100644 --- a/lib/chef/mixin/uris.rb +++ b/lib/chef/mixin/uris.rb @@ -28,6 +28,17 @@ class Chef # From open-uri !!(%r{\A[A-Za-z][A-Za-z0-9+\-\.]*://} =~ source) end + + + def as_uri(source) + begin + URI.parse(source) + rescue URI::InvalidURIError + Chef::Log.warn("#{source} was an invalid URI. Trying to escape invalid characters") + URI.parse(URI.escape(source)) + end + end + end end end diff --git a/lib/chef/provider/remote_file/content.rb b/lib/chef/provider/remote_file/content.rb index 938a7e58f4..4f450ce333 100644 --- a/lib/chef/provider/remote_file/content.rb +++ b/lib/chef/provider/remote_file/content.rb @@ -20,6 +20,7 @@ require 'uri' require 'tempfile' require 'chef/file_content_management/content_base' +require 'chef/mixin/uris' class Chef class Provider @@ -28,6 +29,8 @@ class Chef private + include Chef::Mixin::Uris + def file_for_provider Chef::Log.debug("#{@new_resource} checking for changes") @@ -48,7 +51,7 @@ class Chef uri = if Chef::Provider::RemoteFile::Fetcher.network_share?(source) source else - URI.parse(source) + as_uri(source) end raw_file = grab_file_from_uri(uri) rescue SocketError, Errno::ECONNREFUSED, Errno::ENOENT, Errno::EACCES, Timeout::Error, Net::HTTPServerException, Net::HTTPFatalError, Net::FTPError => e diff --git a/lib/chef/provider/remote_file/local_file.rb b/lib/chef/provider/remote_file/local_file.rb index e78311f2c3..026206b64e 100644 --- a/lib/chef/provider/remote_file/local_file.rb +++ b/lib/chef/provider/remote_file/local_file.rb @@ -32,15 +32,21 @@ class Chef @new_resource = new_resource @uri = uri end - + # CHEF-4472: Remove the leading slash from windows paths that we receive from a file:// URI - def fix_windows_path(path) - path.gsub(/^\/([a-zA-Z]:)/,'\1') + def fix_windows_path(path) + path.gsub(/^\/([a-zA-Z]:)/,'\1') + end + + def source_path + @source_path ||= begin + path = URI.unescape(uri.path) + Chef::Platform.windows? ? fix_windows_path(path) : path + end end # Fetches the file at uri, returning a Tempfile-like File handle def fetch - source_path = Chef::Platform.windows? ? fix_windows_path(uri.path) : uri.path tempfile = Chef::FileContentManagement::Tempfile.new(new_resource).tempfile Chef::Log.debug("#{new_resource} staging #{source_path} to #{tempfile.path}") FileUtils.cp(source_path, tempfile.path) diff --git a/lib/chef/resource/remote_file.rb b/lib/chef/resource/remote_file.rb index 69614f0298..df9fb7ad76 100644 --- a/lib/chef/resource/remote_file.rb +++ b/lib/chef/resource/remote_file.rb @@ -21,6 +21,7 @@ require 'uri' require 'chef/resource/file' require 'chef/provider/remote_file' require 'chef/mixin/securable' +require 'chef/mixin/uris' class Chef class Resource @@ -127,6 +128,8 @@ class Chef private + include Chef::Mixin::Uris + def validate_source(source) source = Array(source).flatten raise ArgumentError, "#{resource_name} has an empty source" if source.empty? @@ -140,7 +143,7 @@ class Chef end def absolute_uri?(source) - Chef::Provider::RemoteFile::Fetcher.network_share?(source) or (source.kind_of?(String) and URI.parse(source).absolute?) + Chef::Provider::RemoteFile::Fetcher.network_share?(source) or (source.kind_of?(String) and as_uri(source).absolute?) rescue URI::InvalidURIError false end diff --git a/spec/unit/mixin/uris_spec.rb b/spec/unit/mixin/uris_spec.rb index 07e4f34508..d4985c4f67 100644 --- a/spec/unit/mixin/uris_spec.rb +++ b/spec/unit/mixin/uris_spec.rb @@ -26,20 +26,32 @@ end describe Chef::Mixin::Uris do let (:uris) { Chef::UrisTest.new } - it "matches 'scheme://foo.com'" do - expect(uris.uri_scheme?('scheme://foo.com')).to eq(true) + describe "#uri_scheme?" do + it "matches 'scheme://foo.com'" do + expect(uris.uri_scheme?('scheme://foo.com')).to eq(true) + end + + it "does not match 'c:/foo.com'" do + expect(uris.uri_scheme?('c:/foo.com')).to eq(false) + end + + it "does not match '/usr/bin/foo.com'" do + expect(uris.uri_scheme?('/usr/bin/foo.com')).to eq(false) + end + + it "does not match 'c:/foo.com://bar.com'" do + expect(uris.uri_scheme?('c:/foo.com://bar.com')).to eq(false) + end end - it "does not match 'c:/foo.com'" do - expect(uris.uri_scheme?('c:/foo.com')).to eq(false) - end - - it "does not match '/usr/bin/foo.com'" do - expect(uris.uri_scheme?('/usr/bin/foo.com')).to eq(false) - end + describe "#as_uri" do + it "parses a file scheme uri with spaces" do + expect{ uris.as_uri("file:///c:/foo bar.txt") }.not_to raise_exception + end - it "does not match 'c:/foo.com://bar.com'" do - expect(uris.uri_scheme?('c:/foo.com://bar.com')).to eq(false) + it "returns a URI object" do + expect( uris.as_uri("file:///c:/foo bar.txt") ).to be_a(URI) + end end end diff --git a/spec/unit/provider/remote_file/local_file_spec.rb b/spec/unit/provider/remote_file/local_file_spec.rb index b33d82f624..575996a540 100644 --- a/spec/unit/provider/remote_file/local_file_spec.rb +++ b/spec/unit/provider/remote_file/local_file_spec.rb @@ -25,26 +25,45 @@ describe Chef::Provider::RemoteFile::LocalFile do let(:new_resource) { Chef::Resource::RemoteFile.new("local file backend test (new_resource)") } let(:current_resource) { Chef::Resource::RemoteFile.new("local file backend test (current_resource)") } subject(:fetcher) { Chef::Provider::RemoteFile::LocalFile.new(uri, new_resource, current_resource) } - - context "when parsing source path" do + + context "when parsing source path on windows" do + + before do + allow(Chef::Platform).to receive(:windows?).and_return(true) + end + describe "when given local unix path" do let(:uri) { URI.parse("file:///nyan_cat.png") } it "returns a correct unix path" do - expect(fetcher.fix_windows_path(uri.path)).to eq("/nyan_cat.png") + expect(fetcher.source_path).to eq("/nyan_cat.png") end end describe "when given local windows path" do let(:uri) { URI.parse("file:///z:/windows/path/file.txt") } it "returns a valid windows local path" do - expect(fetcher.fix_windows_path(uri.path)).to eq("z:/windows/path/file.txt") + expect(fetcher.source_path).to eq("z:/windows/path/file.txt") + end + end + + describe "when given local windows path with spaces" do + let(:uri) { URI.parse(URI.escape("file:///z:/windows/path/foo & bar.txt")) } + it "returns a valid windows local path" do + expect(fetcher.source_path).to eq("z:/windows/path/foo & bar.txt") end end describe "when given unc windows path" do let(:uri) { URI.parse("file:////server/share/windows/path/file.txt") } it "returns a valid windows unc path" do - expect(fetcher.fix_windows_path(uri.path)).to eq("//server/share/windows/path/file.txt") + expect(fetcher.source_path).to eq("//server/share/windows/path/file.txt") + end + end + + describe "when given unc windows path with spaces" do + let(:uri) { URI.parse(URI.escape("file:////server/share/windows/path/foo & bar.txt")) } + it "returns a valid windows unc path" do + expect(fetcher.source_path).to eq("//server/share/windows/path/foo & bar.txt") end end end @@ -73,7 +92,7 @@ describe Chef::Provider::RemoteFile::LocalFile do it "stages the local file to a temporary file" do expect(Chef::FileContentManagement::Tempfile).to receive(:new).with(new_resource).and_return(chef_tempfile) expect(::FileUtils).to receive(:cp).with(uri.path, tempfile.path) - expect(tempfile).to receive(:close) + expect(tempfile).to receive(:close) result = fetcher.fetch expect(result).to eq(tempfile) diff --git a/spec/unit/resource/remote_file_spec.rb b/spec/unit/resource/remote_file_spec.rb index 0b3df4eb2c..0a379ff574 100644 --- a/spec/unit/resource/remote_file_spec.rb +++ b/spec/unit/resource/remote_file_spec.rb @@ -60,6 +60,11 @@ describe Chef::Resource::RemoteFile do expect(@resource.source).to eql([ "\\\\fakey\\fakerton\\fake.txt" ]) end + it 'should accept file URIs with spaces' do + @resource.source("file:///C:/foo bar") + expect(@resource.source).to eql(["file:///C:/foo bar"]) + end + it "should accept a delayed evalutator (string) for the remote file source" do @resource.source Chef::DelayedEvaluator.new {"http://opscode.com/"} expect(@resource.source).to eql([ "http://opscode.com/" ]) |