From b7a1c393083ae43d8d2a6cff94a0a3889aac6f47 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 20 Apr 2012 16:10:28 -0700 Subject: [PATCH] Fix autorequiring when using different root key forms Previously, autorequiring would not add relationships when resources mix short and long forms of the root key, e.g. hklm and hkey_local_machine. It also did not handle different cases, hklm vs HKLM. This commit ensures that we always create a canonical path for the `KeyPath` and `ValuePath`, which always use the downcased root key name. In the process, the ascend logic was greatly simplified. This commit does not fix general case-insensitivity in key and value names, e.g. so hklm\Software and hklm\software are still seen as different resources. This will be addressed in a separate commit. --- lib/puppet/type/registry_key.rb | 7 +-- lib/puppet/type/registry_value.rb | 6 +- lib/puppet/util/key_path.rb | 64 ++++++++++++-------- lib/puppet/util/value_path.rb | 11 ++-- spec/unit/puppet/type/registry_key_spec.rb | 7 +-- spec/unit/puppet/type/registry_value_spec.rb | 19 +++++- 6 files changed, 66 insertions(+), 48 deletions(-) diff --git a/lib/puppet/type/registry_key.rb b/lib/puppet/type/registry_key.rb index ae281d97..fe1d1ef1 100644 --- a/lib/puppet/type/registry_key.rb +++ b/lib/puppet/type/registry_key.rb @@ -19,11 +19,6 @@ def self.title_patterns end autorequire(:registry_key) do - parents = [] - path = parameter(:path) - path.ascend do |hkey, subkey| - parents << "#{hkey.keyname}\\#{subkey}" unless subkey == path.subkey # skip ourselves - end - parents + parameter(:path).enum_for(:ascend).select { |p| self[:path] != p } end end diff --git a/lib/puppet/type/registry_value.rb b/lib/puppet/type/registry_value.rb index 518762c5..453cd72d 100644 --- a/lib/puppet/type/registry_value.rb +++ b/lib/puppet/type/registry_value.rb @@ -44,11 +44,7 @@ def self.title_patterns end autorequire(:registry_key) do - parents = [] - parameter(:path).ascend do |hkey, subkey| - parents << "#{hkey.keyname}\\#{subkey}" - end - parents + parameter(:path).enum_for(:ascend) end end diff --git a/lib/puppet/util/key_path.rb b/lib/puppet/util/key_path.rb index 1a220f61..1e3bb3b5 100644 --- a/lib/puppet/util/key_path.rb +++ b/lib/puppet/util/key_path.rb @@ -4,40 +4,54 @@ class Puppet::Util::KeyPath < Puppet::Parameter include Puppet::Util::RegistryBase - attr_reader :hkey, :subkey + attr_reader :root, :hkey, :subkey - def validate(path) - split(path.to_s) - end - - def split(path) - unless match = /^([^\\]*)((?:\\[^\\]{1,255})*)$/.match(path) + def munge(path) + unless captures = /^([^\\]*)((?:\\[^\\]{1,255})*)$/.match(path) raise ArgumentError, "Invalid registry key: #{path}" end - @hkey = - case match[1].downcase - when /hkey_local_machine/, /hklm/ - HKEYS[:hklm] - when /hkey_classes_root/, /hkcr/ - HKEYS[:hkcr] - else - raise ArgumentError, "Unsupported prefined key: #{path}" - end - - # leading backslash is not part of the subkey name - @subkey = match[2] - @subkey = @subkey[1..-1] unless @subkey.empty? + # canonical root key symbol + @root = case captures[1].downcase + when /hkey_local_machine/, /hklm/ + :hklm + when /hkey_classes_root/, /hkcr/ + :hkcr + when /hkey_current_user/, /hkcu/, + /hkey_users/, /hku/, + /hkey_current_config/, /hkcc/, + /hkey_performance_data/, + /hkey_performance_text/, + /hkey_performance_nlstext/, + /hkey_dyn_data/ + raise ArgumentError, "Unsupported prefined key: #{path}" + else + raise ArgumentError, "Invalid registry key: #{path}" + end + + # the hkey object for the root key + @hkey = HKEYS[root] + + @subkey = captures[2] + if @subkey.empty? + canonical = root.to_s + else + # Leading backslash is not part of the subkey name + @subkey.sub!(/^\\(.*)$/, '\1') + canonical = "#{root.to_s}\\#{subkey}" + end + + canonical end def ascend(&block) - s = subkey + p = self.value - yield hkey, s + yield p - while idx = s.rindex('\\') - s = s[0, idx] - yield hkey, s + while idx = p.rindex('\\') + p = p[0, idx] + yield p end end end diff --git a/lib/puppet/util/value_path.rb b/lib/puppet/util/value_path.rb index bacadf14..b2c74254 100644 --- a/lib/puppet/util/value_path.rb +++ b/lib/puppet/util/value_path.rb @@ -1,13 +1,11 @@ require 'puppet/parameter' -require 'puppet/util/registry_base' +require 'puppet/util/key_path' class Puppet::Util::ValuePath < Puppet::Parameter::KeyPath attr_reader :valuename - def split(path) - unless path - raise ArgumentError, "Invalid registry value" - end + def munge(path) + raise ArgumentError, "Invalid registry value" unless path if path[-1, 1] == '\\' # trailing backslash implies default value super(path.gsub(/\\*$/, '')) @@ -19,5 +17,8 @@ def split(path) super(path[0, idx]) @valuename = path[idx+1..-1] if idx > 0 end + + canonical = subkey.empty? ? "#{root}\\#{valuename}" : "#{root}\\#{subkey}\\#{valuename}" + canonical.downcase end end diff --git a/spec/unit/puppet/type/registry_key_spec.rb b/spec/unit/puppet/type/registry_key_spec.rb index f26b57d4..b50a688b 100644 --- a/spec/unit/puppet/type/registry_key_spec.rb +++ b/spec/unit/puppet/type/registry_key_spec.rb @@ -26,13 +26,13 @@ end end - %w[unknown unknown\subkey HKEY_PERFORMANCE_DATA].each do |path| - it "should reject #{path} as unsupported" do + %w[HKEY_DYN_DATA HKEY_PERFORMANCE_DATA].each do |path| + it "should reject #{path} as unsupported case insensitively" do expect { key[:path] = path }.should raise_error(Puppet::Error, /Unsupported/) end end - %[hklm\\ hklm\foo\\].each do |path| + %[hklm\\ hklm\foo\\ unknown unknown\subkey].each do |path| it "should reject #{path} as invalid" do path = "hklm\\" + 'a' * 256 expect { key[:path] = path }.should raise_error(Puppet::Error, /Invalid registry key/) @@ -41,7 +41,6 @@ %w[HKLM HKEY_LOCAL_MACHINE hklm].each do |root| it "should canonicalize the root key #{root}" do - pending("Not implemented") key[:path] = root key[:path].should == 'hklm' end diff --git a/spec/unit/puppet/type/registry_value_spec.rb b/spec/unit/puppet/type/registry_value_spec.rb index 44026396..dffa5acf 100644 --- a/spec/unit/puppet/type/registry_value_spec.rb +++ b/spec/unit/puppet/type/registry_value_spec.rb @@ -34,19 +34,27 @@ described_class.new(:path => 'hklm\\software\\\\') end - %w[unknown\name unknown\subkey\name HKEY_PERFORMANCE_DATA\name].each do |path| + %w[HKEY_DYN_DATA\\ HKEY_PERFORMANCE_DATA\name].each do |path| it "should reject #{path} as unsupported" do expect { described_class.new(:path => path) }.to raise_error(Puppet::Error, /Unsupported/) end end - %[hklm hkcr].each do |path| + %[hklm hkcr unknown\\name unknown\\subkey\\name].each do |path| it "should reject #{path} as invalid" do pending 'wrong message' expect { described_class.new(:path => path) }.should raise_error(Puppet::Error, /Invalid registry key/) end end + %w[HKLM\\name HKEY_LOCAL_MACHINE\\name hklm\\name].each do |root| + it "should canonicalize root key #{root}" do + value = described_class.new(:path => root) + value[:path].should == 'hklm\name' + end + end + + it 'should validate the length of the value name' it 'should validate the length of the value data' it 'should canonicalize the root key' @@ -91,11 +99,16 @@ value[:data] = 'foobar' end - it "should support integer data" do + it "should support dword data" do value[:type] = :dword value[:data] = 0 end + it "should support qword data" do + value[:type] = :qword + value[:data] = 0xFFFF + end + it "should support binary data" do value[:type] = :binary value[:data] = "CA FE BE EF"