diff options
author | Jacob Helwig <jacob@puppetlabs.com> | 2011-08-02 11:04:26 -0700 |
---|---|---|
committer | Jacob Helwig <jacob@puppetlabs.com> | 2011-08-19 13:52:58 -0700 |
commit | 37f87b709598b48bc8b562422da27cc73cc1eff1 (patch) | |
tree | b5f96e5e69556e6031a8533960802fac268e88b7 | |
parent | 4a6d61717eb54a0cefff87bf67ae62df1e3b4317 (diff) | |
download | puppet-37f87b709598b48bc8b562422da27cc73cc1eff1.tar.gz puppet-37f87b709598b48bc8b562422da27cc73cc1eff1.tar.xz puppet-37f87b709598b48bc8b562422da27cc73cc1eff1.zip |
Treat Windows absolute paths as absolute paths
Previously, we only considered files that matched the *nix concept of
'absolute' as being absolute paths. Since absolute paths on Windows
look more like URLs with this world-view, we need to specifically look
for the Windows absolute paths, and treat them as such.
We will still treat *nix absolute paths as absolute on Windows, even
though they are actually relative to the "current" drive. We do not
currently limit which "style" of absolute path is allowed based on
what the agent is.
Reviewed-by: Nick Lewis <nick@puppetlabs.com>
(cherry picked from commit 568d25ee10effd5e87c57cdc8c24280eabf9cd93)
-rw-r--r-- | lib/puppet/file_serving/base.rb | 5 | ||||
-rw-r--r-- | lib/puppet/file_serving/indirection_hooks.rb | 1 | ||||
-rw-r--r-- | lib/puppet/indirector/request.rb | 4 | ||||
-rw-r--r-- | lib/puppet/type/file.rb | 20 | ||||
-rwxr-xr-x | lib/puppet/type/file/source.rb | 4 | ||||
-rwxr-xr-x | spec/unit/type/file_spec.rb | 380 |
6 files changed, 192 insertions, 222 deletions
diff --git a/lib/puppet/file_serving/base.rb b/lib/puppet/file_serving/base.rb index 1927b95d6..e936b5e75 100644 --- a/lib/puppet/file_serving/base.rb +++ b/lib/puppet/file_serving/base.rb @@ -49,7 +49,10 @@ class Puppet::FileServing::Base # Set our base path. attr_reader :path def path=(path) - raise ArgumentError.new("Paths must be fully qualified") unless path =~ /^#{::File::SEPARATOR}/ + unless path =~ /^#{::File::SEPARATOR}/ or path =~ /^[a-z]:[\/\\]/i + raise ArgumentError.new("Paths must be fully qualified") + end + @path = path end diff --git a/lib/puppet/file_serving/indirection_hooks.rb b/lib/puppet/file_serving/indirection_hooks.rb index 499767c41..bdcc8865e 100644 --- a/lib/puppet/file_serving/indirection_hooks.rb +++ b/lib/puppet/file_serving/indirection_hooks.rb @@ -13,6 +13,7 @@ module Puppet::FileServing::IndirectionHooks # Short-circuit to :file if it's a fully-qualified path or specifies a 'file' protocol. return PROTOCOL_MAP["file"] if request.key =~ /^#{::File::SEPARATOR}/ + return PROTOCOL_MAP["file"] if request.key =~ /^[a-z]:[\/\\]/i return PROTOCOL_MAP["file"] if request.protocol == "file" # We're heading over the wire the protocol is 'puppet' and we've got a server name or we're not named 'apply' or 'puppet' diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb index fd8d654dd..0388bd31a 100644 --- a/lib/puppet/indirector/request.rb +++ b/lib/puppet/indirector/request.rb @@ -76,7 +76,9 @@ class Puppet::Indirector::Request # because it rewrites the key. We could otherwise strip server/port/etc # info out in the REST class, but it seemed bad design for the REST # class to rewrite the key. - if key.to_s =~ /^\w+:\/\// # it's a URI + if key.to_s =~ /^[a-z]:[\/\\]/i # It's an absolute path for Windows. + @key = key + elsif key.to_s =~ /^\w+:\/\// # it's a URI set_uri_key(key) else @key = key diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 5398621f5..d3c66bc02 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -23,7 +23,7 @@ Puppet::Type.newtype(:file) do location, rather than using native resources, please contact Puppet Labs and we can hopefully work with you to develop a native resource to support what you are doing. - + **Autorequires:** If Puppet is managing the user or group that owns a file, the file resource will autorequire them. If Puppet is managing any parent directories of a file, the file resource will autorequire them." def self.title_patterns @@ -36,7 +36,7 @@ Puppet::Type.newtype(:file) do validate do |value| # accept various path syntaxes: lone slash, posix, win32, unc - unless (Puppet.features.posix? and value =~ /^\//) or (Puppet.features.microsoft_windows? and (value =~ /^[A-Za-z]:\// or value =~ /^\/\/[^\/]+\/[^\/]+/)) + unless (Puppet.features.posix? and value =~ /^\//) or (value =~ /^[A-Za-z]:\// or value =~ /^\/\/[^\/]+\/[^\/]+/) fail Puppet::Error, "File paths must be fully qualified, not '#{value}'" end end @@ -44,7 +44,21 @@ Puppet::Type.newtype(:file) do # convert the current path in an index into the collection and the last # path name. The aim is to use less storage for all common paths in a hierarchy munge do |value| - path, name = ::File.split(value.gsub(/\/+/,'/')) + # We need to save off, and remove the volume designator in the + # path if it is there, since File.split does not handle paths + # with volume designators properly, except when run on Windows. + # Since we are potentially compiling a catalog for a Windows + # machine on a non-Windows master, we need to handle this + # ourselves. + optional_volume_designator = value.match(/^([a-z]:)[\/\\].*/i) + value_without_designator = value.sub(/^(?:[a-z]:)?(.*)/i, '\1') + + path, name = ::File.split(value_without_designator.gsub(/\/+/,'/')) + + if optional_volume_designator + path = optional_volume_designator[1] + path + end + { :index => Puppet::FileCollection.collection.index(path), :name => name } end diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb index 39f85e2ad..8653a8f7a 100755 --- a/lib/puppet/type/file/source.rb +++ b/lib/puppet/type/file/source.rb @@ -72,7 +72,7 @@ module Puppet self.fail "Could not understand source #{source}: #{detail}" end - self.fail "Cannot use URLs of type '#{uri.scheme}' as source for fileserving" unless uri.scheme.nil? or %w{file puppet}.include?(uri.scheme) + self.fail "Cannot use URLs of type '#{uri.scheme}' as source for fileserving" unless uri.scheme.nil? or %w{file puppet}.include?(uri.scheme) or (Puppet.features.microsoft_windows? and uri.scheme =~ /^[a-z]$/i) end end @@ -180,6 +180,8 @@ module Puppet private def uri + return nil if metadata.source =~ /^[a-z]:[\/\\]/i # Abspath for Windows + @uri ||= URI.parse(URI.escape(metadata.source)) end end diff --git a/spec/unit/type/file_spec.rb b/spec/unit/type/file_spec.rb index 24141a9ef..0041ce9f2 100755 --- a/spec/unit/type/file_spec.rb +++ b/spec/unit/type/file_spec.rb @@ -37,7 +37,6 @@ describe Puppet::Type.type(:file) do end describe "#write" do - it "should propagate failures encountered when renaming the temporary file" do File.stubs(:open) @@ -92,7 +91,6 @@ describe Puppet::Type.type(:file) do lambda { @file.write :NOTUSED }.should_not raise_error(Puppet::Error) end - end end @@ -152,254 +150,204 @@ describe Puppet::Type.type(:file) do end describe "when using POSIX filenames" do - describe "on POSIX systems" do - before do - Puppet.features.stubs(:posix?).returns(true) - Puppet.features.stubs(:microsoft_windows?).returns(false) - end - - it "should autorequire its parent directory" do - file = Puppet::Type::File.new(:path => "/foo/bar") - dir = Puppet::Type::File.new(:path => "/foo") - @catalog.add_resource file - @catalog.add_resource dir - reqs = file.autorequire - reqs[0].source.must == dir - reqs[0].target.must == file - end - - it "should autorequire its nearest ancestor directory" do - file = Puppet::Type::File.new(:path => "/foo/bar/baz") - dir = Puppet::Type::File.new(:path => "/foo") - root = Puppet::Type::File.new(:path => "/") - @catalog.add_resource file - @catalog.add_resource dir - @catalog.add_resource root - reqs = file.autorequire - reqs.length.must == 1 - reqs[0].source.must == dir - reqs[0].target.must == file - end - - it "should not autorequire anything when there is no nearest ancestor directory" do - file = Puppet::Type::File.new(:path => "/foo/bar/baz") - @catalog.add_resource file - file.autorequire.should be_empty - end - - it "should not autorequire its parent dir if its parent dir is itself" do - file = Puppet::Type::File.new(:path => "/") - @catalog.add_resource file - file.autorequire.should be_empty - end - - it "should remove trailing slashes" do - file = Puppet::Type::File.new(:path => "/foo/bar/baz/") - file[:path].should == "/foo/bar/baz" - end - - it "should remove double slashes" do - file = Puppet::Type::File.new(:path => "/foo/bar//baz") - file[:path].should == "/foo/bar/baz" - end + it "should autorequire its parent directory" do + file = Puppet::Type::File.new(:path => "/foo/bar") + dir = Puppet::Type::File.new(:path => "/foo") + @catalog.add_resource file + @catalog.add_resource dir + reqs = file.autorequire + reqs[0].source.must == dir + reqs[0].target.must == file + end - it "should remove trailing double slashes" do - file = Puppet::Type::File.new(:path => "/foo/bar/baz//") - file[:path].should == "/foo/bar/baz" - end + it "should autorequire its nearest ancestor directory" do + file = Puppet::Type::File.new(:path => "/foo/bar/baz") + dir = Puppet::Type::File.new(:path => "/foo") + root = Puppet::Type::File.new(:path => "/") + @catalog.add_resource file + @catalog.add_resource dir + @catalog.add_resource root + reqs = file.autorequire + reqs.length.must == 1 + reqs[0].source.must == dir + reqs[0].target.must == file + end - it "should leave a single slash alone" do - file = Puppet::Type::File.new(:path => "/") - file[:path].should == "/" - end + it "should not autorequire anything when there is no nearest ancestor directory" do + file = Puppet::Type::File.new(:path => "/foo/bar/baz") + @catalog.add_resource file + file.autorequire.should be_empty + end - it "should accept a double-slash at the start of the path" do - expect { - file = Puppet::Type::File.new(:path => "//tmp/xxx") - # REVISIT: This should be wrong, later. See the next test. - # --daniel 2011-01-31 - file[:path].should == '/tmp/xxx' - }.should_not raise_error - end + it "should not autorequire its parent dir if its parent dir is itself" do + file = Puppet::Type::File.new(:path => "/") + @catalog.add_resource file + file.autorequire.should be_empty + end - # REVISIT: This is pending, because I don't want to try and audit the - # entire codebase to make sure we get this right. POSIX treats two (and - # exactly two) '/' characters at the start of the path specially. - # - # See sections 3.2 and 4.11, which allow DomainOS to be all special like - # and still have the POSIX branding and all. --daniel 2011-01-31 - it "should preserve the double-slash at the start of the path" + it "should remove trailing slashes" do + file = Puppet::Type::File.new(:path => "/foo/bar/baz/") + file[:path].should == "/foo/bar/baz" end - describe "on Microsoft Windows systems" do - before do - Puppet.features.stubs(:posix?).returns(false) - Puppet.features.stubs(:microsoft_windows?).returns(true) - end + it "should remove double slashes" do + file = Puppet::Type::File.new(:path => "/foo/bar//baz") + file[:path].should == "/foo/bar/baz" + end - it "should refuse to work" do - lambda { Puppet::Type::File.new(:path => "/foo/bar") }.should raise_error(Puppet::Error) - end + it "should remove trailing double slashes" do + file = Puppet::Type::File.new(:path => "/foo/bar/baz//") + file[:path].should == "/foo/bar/baz" end - end - describe "when using Microsoft Windows filenames", :if => Puppet.features.microsoft_windows? do - describe "on Microsoft Windows systems" do - before do - Puppet.features.stubs(:posix?).returns(false) - Puppet.features.stubs(:microsoft_windows?).returns(true) - end + it "should leave a single slash alone" do + file = Puppet::Type::File.new(:path => "/") + file[:path].should == "/" + end - it "should autorequire its parent directory" do - file = Puppet::Type::File.new(:path => "X:/foo/bar") - dir = Puppet::Type::File.new(:path => "X:/foo") - @catalog.add_resource file - @catalog.add_resource dir - reqs = file.autorequire - reqs[0].source.must == dir - reqs[0].target.must == file - end + it "should accept a double-slash at the start of the path" do + expect { + file = Puppet::Type::File.new(:path => "//tmp/xxx") + # REVISIT: This should be wrong, later. See the next test. + # --daniel 2011-01-31 + file[:path].should == '/tmp/xxx' + }.should_not raise_error + end - it "should autorequire its nearest ancestor directory" do - file = Puppet::Type::File.new(:path => "X:/foo/bar/baz") - dir = Puppet::Type::File.new(:path => "X:/foo") - root = Puppet::Type::File.new(:path => "X:/") - @catalog.add_resource file - @catalog.add_resource dir - @catalog.add_resource root - reqs = file.autorequire - reqs.length.must == 1 - reqs[0].source.must == dir - reqs[0].target.must == file - end + # REVISIT: This is pending, because I don't want to try and audit the + # entire codebase to make sure we get this right. POSIX treats two (and + # exactly two) '/' characters at the start of the path specially. + # + # See sections 3.2 and 4.11, which allow DomainOS to be all special like + # and still have the POSIX branding and all. --daniel 2011-01-31 + it "should preserve the double-slash at the start of the path" + end - it "should not autorequire anything when there is no nearest ancestor directory" do - file = Puppet::Type::File.new(:path => "X:/foo/bar/baz") - @catalog.add_resource file - file.autorequire.should be_empty - end + describe "when using Microsoft Windows filenames" do + it "should autorequire its parent directory" do + file = Puppet::Type::File.new(:path => "X:/foo/bar") + dir = Puppet::Type::File.new(:path => "X:/foo") + @catalog.add_resource file + @catalog.add_resource dir + reqs = file.autorequire + reqs[0].source.must == dir + reqs[0].target.must == file + end - it "should not autorequire its parent dir if its parent dir is itself" do - file = Puppet::Type::File.new(:path => "X:/") - @catalog.add_resource file - file.autorequire.should be_empty - end + it "should autorequire its nearest ancestor directory" do + file = Puppet::Type::File.new(:path => "X:/foo/bar/baz") + dir = Puppet::Type::File.new(:path => "X:/foo") + root = Puppet::Type::File.new(:path => "X:/") + @catalog.add_resource file + @catalog.add_resource dir + @catalog.add_resource root + reqs = file.autorequire + reqs.length.must == 1 + reqs[0].source.must == dir + reqs[0].target.must == file + end - it "should remove trailing slashes" do - file = Puppet::Type::File.new(:path => "X:/foo/bar/baz/") - file[:path].should == "X:/foo/bar/baz" - end + it "should not autorequire anything when there is no nearest ancestor directory" do + file = Puppet::Type::File.new(:path => "X:/foo/bar/baz") + @catalog.add_resource file + file.autorequire.should be_empty + end - it "should remove double slashes" do - file = Puppet::Type::File.new(:path => "X:/foo/bar//baz") - file[:path].should == "X:/foo/bar/baz" - end + it "should not autorequire its parent dir if its parent dir is itself" do + file = Puppet::Type::File.new(:path => "X:/") + @catalog.add_resource file + file.autorequire.should be_empty + end - it "should remove trailing double slashes" do - file = Puppet::Type::File.new(:path => "X:/foo/bar/baz//") - file[:path].should == "X:/foo/bar/baz" - end + it "should remove trailing slashes" do + file = Puppet::Type::File.new(:path => "X:/foo/bar/baz/") + file[:path].should == "X:/foo/bar/baz" + end - it "should leave a drive letter with a slash alone", :'fails_on_ruby_1.9.2' => true do - file = Puppet::Type::File.new(:path => "X:/") - file[:path].should == "X:/" - end + it "should remove double slashes" do + file = Puppet::Type::File.new(:path => "X:/foo/bar//baz") + file[:path].should == "X:/foo/bar/baz" + end - it "should not accept a drive letter without a slash", :'fails_on_ruby_1.9.2' => true do - lambda { Puppet::Type::File.new(:path => "X:") }.should raise_error(/File paths must be fully qualified/) - end + it "should remove trailing double slashes" do + file = Puppet::Type::File.new(:path => "X:/foo/bar/baz//") + file[:path].should == "X:/foo/bar/baz" end - describe "on POSIX systems" do - before do - Puppet.features.stubs(:posix?).returns(true) - Puppet.features.stubs(:microsoft_windows?).returns(false) - end + it "should leave a drive letter with a slash alone", :'fails_on_ruby_1.9.2' => true do + file = Puppet::Type::File.new(:path => "X:/") + file[:path].should == "X:/" + end - it "should refuse to work" do - lambda { Puppet::Type::File.new(:path => "X:/foo/bar") }.should raise_error(Puppet::Error) - end + it "should not accept a drive letter without a slash", :'fails_on_ruby_1.9.2' => true do + lambda { Puppet::Type::File.new(:path => "X:") }.should raise_error(/File paths must be fully qualified/) end end - describe "when using UNC filenames" do - describe "on Microsoft Windows systems", :if => Puppet.features.microsoft_windows?, :'fails_on_ruby_1.9.2' => true do - before do - Puppet.features.stubs(:posix?).returns(false) - Puppet.features.stubs(:microsoft_windows?).returns(true) - end - - it "should autorequire its parent directory" do - file = Puppet::Type::File.new(:path => "//server/foo/bar") - dir = Puppet::Type::File.new(:path => "//server/foo") - @catalog.add_resource file - @catalog.add_resource dir - reqs = file.autorequire - reqs[0].source.must == dir - reqs[0].target.must == file - end - - it "should autorequire its nearest ancestor directory" do - file = Puppet::Type::File.new(:path => "//server/foo/bar/baz/qux") - dir = Puppet::Type::File.new(:path => "//server/foo/bar") - root = Puppet::Type::File.new(:path => "//server/foo") - @catalog.add_resource file - @catalog.add_resource dir - @catalog.add_resource root - reqs = file.autorequire - reqs.length.must == 1 - reqs[0].source.must == dir - reqs[0].target.must == file - end + describe "when using UNC filenames", :'fails_on_ruby_1.9.2' => true do + before :each do + pending("UNC file paths not yet supported") + end - it "should not autorequire anything when there is no nearest ancestor directory" do - file = Puppet::Type::File.new(:path => "//server/foo/bar/baz/qux") - @catalog.add_resource file - file.autorequire.should be_empty - end + it "should autorequire its parent directory" do + file = Puppet::Type::File.new(:path => "//server/foo/bar") + dir = Puppet::Type::File.new(:path => "//server/foo") + @catalog.add_resource file + @catalog.add_resource dir + reqs = file.autorequire + reqs[0].source.must == dir + reqs[0].target.must == file + end - it "should not autorequire its parent dir if its parent dir is itself" do - file = Puppet::Type::File.new(:path => "//server/foo") - @catalog.add_resource file - puts file.autorequire - file.autorequire.should be_empty - end + it "should autorequire its nearest ancestor directory" do + file = Puppet::Type::File.new(:path => "//server/foo/bar/baz/qux") + dir = Puppet::Type::File.new(:path => "//server/foo/bar") + root = Puppet::Type::File.new(:path => "//server/foo") + @catalog.add_resource file + @catalog.add_resource dir + @catalog.add_resource root + reqs = file.autorequire + reqs.length.must == 1 + reqs[0].source.must == dir + reqs[0].target.must == file + end - it "should remove trailing slashes" do - file = Puppet::Type::File.new(:path => "//server/foo/bar/baz/") - file[:path].should == "//server/foo/bar/baz" - end + it "should not autorequire anything when there is no nearest ancestor directory" do + file = Puppet::Type::File.new(:path => "//server/foo/bar/baz/qux") + @catalog.add_resource file + file.autorequire.should be_empty + end - it "should remove double slashes" do - file = Puppet::Type::File.new(:path => "//server/foo/bar//baz") - file[:path].should == "//server/foo/bar/baz" - end + it "should not autorequire its parent dir if its parent dir is itself" do + file = Puppet::Type::File.new(:path => "//server/foo") + @catalog.add_resource file + puts file.autorequire + file.autorequire.should be_empty + end - it "should remove trailing double slashes" do - file = Puppet::Type::File.new(:path => "//server/foo/bar/baz//") - file[:path].should == "//server/foo/bar/baz" - end + it "should remove trailing slashes" do + file = Puppet::Type::File.new(:path => "//server/foo/bar/baz/") + file[:path].should == "//server/foo/bar/baz" + end - it "should remove a trailing slash from a sharename" do - file = Puppet::Type::File.new(:path => "//server/foo/") - file[:path].should == "//server/foo" - end + it "should remove double slashes" do + file = Puppet::Type::File.new(:path => "//server/foo/bar//baz") + file[:path].should == "//server/foo/bar/baz" + end - it "should not modify a sharename" do - file = Puppet::Type::File.new(:path => "//server/foo") - file[:path].should == "//server/foo" - end + it "should remove trailing double slashes" do + file = Puppet::Type::File.new(:path => "//server/foo/bar/baz//") + file[:path].should == "//server/foo/bar/baz" end - describe "on POSIX systems" do - before do - Puppet.features.stubs(:posix?).returns(true) - Puppet.features.stubs(:microsoft_windows?).returns(false) - end + it "should remove a trailing slash from a sharename" do + file = Puppet::Type::File.new(:path => "//server/foo/") + file[:path].should == "//server/foo" + end - it "should refuse to work" do - lambda { Puppet::Type::File.new(:path => "X:/foo/bar") }.should raise_error(Puppet::Error) - end + it "should not modify a sharename" do + file = Puppet::Type::File.new(:path => "//server/foo") + file[:path].should == "//server/foo" end end |