Merge pull request #115 from johnsyweb/handle-quotation-marks-in-section-names

Handle quotation marks in section names

lib/puppet/provider/ini_setting/ruby.rb

@@ -38,7 +38,7 @@ Puppet::Type.type(:ini_setting).provide(:ruby) do
   end
 
   def exists?
-    ini_file.get_value(section, setting)
+    !ini_file.get_value(section, setting).nil?
   end
 
   def create

lib/puppet/util/ini_file.rb

@@ -5,7 +5,7 @@ module Puppet
 module Util
   class IniFile
 
-    @@SECTION_REGEX = /^\s*\[([\w\d\.\\\/\-\:\s]*[\w\d\.\\\/\-])\]\s*$/
+    @@SECTION_REGEX = /^\s*\[([^\]]*)\]\s*$/
     @@SETTING_REGEX = /^(\s*)([^\[#;][\w\d\.\\\/\-\s\[\]\']*[\w\d\.\\\/\-\]])([ \t]*=[ \t]*)([\S\s]*?)\s*$/
     @@COMMENTED_SETTING_REGEX = /^(\s*)[#;]+(\s*)([^\[]*[\w\d\.\\\/\-]+[\w\d\.\\\/\-\[\]\']+)([ \t]*=[ \t]*)([\S\s]*?)\s*$/
 

spec/unit/puppet/provider/ini_setting/ruby_spec.rb

@@ -144,7 +144,7 @@ subby=bar
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
           :setting => 'yahoo', :value => 'yippee'))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
 # This is a comment
@@ -172,7 +172,7 @@ subby=bar
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
           :section => 'section:sub', :setting => 'yahoo', :value => 'yippee'))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
 # This is a comment
@@ -200,7 +200,7 @@ yahoo = yippee
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
            :setting => 'baz', :value => 'bazvalue2'))
       provider = described_class.new(resource)
-      provider.exists?.should == 'bazvalue'
+      provider.exists?.should be true
       provider.value=('bazvalue2')
       validate_file(<<-EOS
 # This is a comment
@@ -227,7 +227,7 @@ subby=bar
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
            :section => 'section:sub', :setting => 'subby', :value => 'foo'))
       provider = described_class.new(resource)
-      provider.exists?.should == 'bar'
+      provider.exists?.should be true
       provider.value.should == 'bar'
       provider.value=('foo')
       validate_file(<<-EOS
@@ -255,7 +255,7 @@ subby=foo
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
            :setting => 'url', :value => 'http://192.168.0.1:8080'))
       provider = described_class.new(resource)
-      provider.exists?.should == 'http://192.168.1.1:8080'
+      provider.exists?.should be true
       provider.value.should == 'http://192.168.1.1:8080'
       provider.value=('http://192.168.0.1:8080')
 
@@ -284,14 +284,14 @@ subby=bar
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
            :setting => 'baz', :value => 'bazvalue'))
       provider = described_class.new(resource)
-      provider.exists?.should == 'bazvalue'
+      provider.exists?.should be true
     end
 
     it "should add a new section if the section does not exist" do
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
           :section => "section3", :setting => 'huzzah', :value => 'shazaam'))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
 # This is a comment
@@ -321,7 +321,7 @@ huzzah = shazaam
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
           :section => "section:subsection", :setting => 'huzzah', :value => 'shazaam'))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
 # This is a comment
@@ -351,7 +351,7 @@ huzzah = shazaam
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
           :section => "section1", :setting => 'setting1', :value => 'hellowworld', :path => emptyfile))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file("
 [section1]
@@ -363,7 +363,7 @@ setting1 = hellowworld
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
           :section => "section:subsection", :setting => 'setting1', :value => 'hellowworld', :path => emptyfile))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file("
 [section:subsection]
@@ -375,7 +375,7 @@ setting1 = hellowworld
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
           :section => "section1", :setting => 'master', :value => true))
       provider = described_class.new(resource)
-      provider.exists?.should == 'true'
+      provider.exists?.should be true
       provider.value.should == 'true'
     end
 
@@ -397,7 +397,7 @@ foo = http://192.168.1.1:8080
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
           :section => '', :setting => 'bar', :value => 'yippee'))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
 # This is a comment
@@ -414,7 +414,7 @@ foo = http://192.168.1.1:8080
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
            :section => '', :setting => 'foo', :value => 'yippee'))
       provider = described_class.new(resource)
-      provider.exists?.should == 'blah'
+      provider.exists?.should be true
       provider.value.should == 'blah'
       provider.value=('yippee')
       validate_file(<<-EOS
@@ -431,7 +431,7 @@ foo = http://192.168.1.1:8080
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
            :section => '', :setting => 'foo', :value => 'blah'))
       provider = described_class.new(resource)
-      provider.exists?.should == 'blah'
+      provider.exists?.should be true
     end
   end
 
@@ -447,7 +447,7 @@ foo = http://192.168.1.1:8080
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
            :section => '', :setting => 'foo', :value => 'yippee'))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
 foo = yippee
@@ -462,7 +462,7 @@ foo = http://192.168.1.1:8080
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
           :section => 'section2', :setting => 'foo', :value => 'yippee'))
       provider = described_class.new(resource)
-      provider.exists?.should == 'http://192.168.1.1:8080'
+      provider.exists?.should be true
       provider.value.should == 'http://192.168.1.1:8080'
       provider.value=('yippee')
       validate_file(<<-EOS
@@ -476,7 +476,7 @@ foo = yippee
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
           :section => 'section2', :setting => 'bar', :value => 'baz'))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
 [section2]
@@ -522,7 +522,7 @@ foo=bar
                                                    :value             => 'yippee',
                                                    :key_val_separator => '='))
       provider = described_class.new(resource)
-      provider.exists?.should == 'bar'
+      provider.exists?.should be true
       provider.value.should == 'bar'
       provider.value=('yippee')
       validate_file(<<-EOS
@@ -539,7 +539,7 @@ foo=yippee
                                                    :value             => 'baz',
                                                    :key_val_separator => '='))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
 [section2]
@@ -576,7 +576,7 @@ EOS
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
       :section => 'section1', :setting => 'foo', :ensure => 'absent'))
       provider = described_class.new(resource)
-      provider.exists?.should be_true
+      provider.exists?.should be true
       provider.destroy
       validate_file(<<-EOS
 [section1]
@@ -601,7 +601,7 @@ EOS
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
                                                    :section => 'section:sub', :setting => 'foo', :ensure => 'absent'))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.destroy
       validate_file(<<-EOS
 [section1]
@@ -652,7 +652,7 @@ subby=bar
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
                     :section => 'section1', :setting => 'yahoo', :value => 'yippee'))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
 # This is a comment
@@ -681,7 +681,7 @@ subby=bar
       resource = Puppet::Type::Ini_setting.new(
           common_params.merge(:section => 'section1', :setting => 'bar', :value => 'barvalue2'))
       provider = described_class.new(resource)
-      provider.exists?.should be_true
+      provider.exists?.should be true
       provider.create
       validate_file(<<-EOS
 # This is a comment
@@ -709,7 +709,7 @@ subby=bar
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
                                                    :section => 'section2', :setting => 'yahoo', :value => 'yippee'))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
 # This is a comment
@@ -738,7 +738,7 @@ subby=bar
       resource = Puppet::Type::Ini_setting.new(
           common_params.merge(:section => 'section2', :setting => 'baz', :value => 'bazvalue2'))
       provider = described_class.new(resource)
-      provider.exists?.should be_true
+      provider.exists?.should be true
       provider.create
       validate_file(<<-EOS
 # This is a comment
@@ -766,7 +766,7 @@ subby=bar
       resource = Puppet::Type::Ini_setting.new(
           common_params.merge(:section => 'section:sub', :setting => 'yahoo', :value => 'yippee'))
       provider = described_class.new(resource)
-      provider.exists?.should be_nil
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
 # This is a comment
@@ -795,7 +795,7 @@ subby=bar
       resource = Puppet::Type::Ini_setting.new(
           common_params.merge(:section => 'section:sub', :setting => 'fleezy', :value => 'flam2'))
       provider = described_class.new(resource)
-      provider.exists?.should be_true
+      provider.exists?.should be true
       provider.create
       validate_file(<<-EOS
 # This is a comment
@@ -842,7 +842,7 @@ blah = blah
       resource = Puppet::Type::Ini_setting.new(
           common_params.merge(:section => 'section2', :setting => 'foo', :value => 'foo3'))
       provider = described_class.new(resource)
-      provider.exists?.should be_false
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
      [section1]
@@ -864,7 +864,7 @@ blah = blah
       resource = Puppet::Type::Ini_setting.new(
           common_params.merge(:section => 'section1', :setting => 'foo', :value => 'foo3'))
       provider = described_class.new(resource)
-      provider.exists?.should be_true
+      provider.exists?.should be true
       provider.create
       validate_file(<<-EOS
      [section1]
@@ -885,7 +885,7 @@ blah = blah
       resource = Puppet::Type::Ini_setting.new(
           common_params.merge(:section => 'section2', :setting => 'bar', :value => 'bar3'))
       provider = described_class.new(resource)
-      provider.exists?.should be_false
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
      [section1]
@@ -907,7 +907,7 @@ blah = blah
       resource = Puppet::Type::Ini_setting.new(
           common_params.merge(:section => 'section2', :setting => 'baz', :value => 'bazvalue'))
       provider = described_class.new(resource)
-      provider.exists?.should be_false
+      provider.exists?.should be false
       provider.create
       validate_file(<<-EOS
      [section1]
@@ -938,7 +938,7 @@ EOS
           common_params.merge(:section => 'section1', :setting => 'foo', :value => 'foovalue2')
         )
         provider = described_class.new(resource)
-        provider.exists?.should be_false
+        provider.exists?.should be false
         provider.create
         validate_file(<<-EOS
 [section1]
@@ -953,7 +953,7 @@ foo=foovalue2
           common_params.merge(:section => 'section1', :setting => 'bar', :value => 'barvalue2')
         )
         provider = described_class.new(resource)
-        provider.exists?.should be_false
+        provider.exists?.should be false
         provider.create
         validate_file(<<-EOS
 [section1]
@@ -991,7 +991,7 @@ subby=bar
         resource = Puppet::Type::Ini_setting.new(common_params.merge(
             :section => 'section - two', :setting => 'yahoo', :value => 'yippee'))
         provider = described_class.new(resource)
-        provider.exists?.should be_nil
+        provider.exists?.should be false
         provider.create
         validate_file(<<-EOS
 # This is a comment
@@ -1019,4 +1019,45 @@ subby=bar
 
   end
 
+  context "when sections have spaces and quotations" do
+    let(:orig_content) do
+      <<-EOS
+[branch "master"]
+        remote = origin
+        merge = refs/heads/master
+
+[alias]
+to-deploy = log --merges --grep='pull request' --format='%s (%cN)' origin/production..origin/master
+[branch "production"]
+        remote = origin
+        merge = refs/heads/production
+      EOS
+    end
+
+    it "should add a missing setting to the correct section" do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(
+        :section => 'alias',
+        :setting => 'foo',
+        :value => 'bar'
+      ))
+      provider = described_class.new(resource)
+      provider.exists?.should be false
+      provider.create
+      validate_file(<<-EOS
+[branch "master"]
+        remote = origin
+        merge = refs/heads/master
+
+[alias]
+to-deploy = log --merges --grep='pull request' --format='%s (%cN)' origin/production..origin/master
+foo = bar
+[branch "production"]
+        remote = origin
+        merge = refs/heads/production
+                    EOS
+                   )
+    end
+
+  end
+
 end

spec/unit/puppet/util/ini_file_spec.rb

@@ -141,4 +141,82 @@ foo=
       subject.get_value("section1", "baz").should == "bazvalue"
     end
   end
+
+  context 'the file has quotation marks in its section names' do
+    let(:sample_content) do
+      template = <<-EOS
+[branch "master"]
+        remote = origin
+        merge = refs/heads/master
+
+[alias]
+to-deploy = log --merges --grep='pull request' --format='%s (%cN)' origin/production..origin/master
+[branch "production"]
+        remote = origin
+        merge = refs/heads/production
+      EOS
+      template.split("\n")
+    end
+
+    it 'should parse the sections' do
+      subject.section_names.should match_array ['',
+                                                'branch "master"',
+                                                'alias',
+                                                'branch "production"'
+      ]
+    end
+  end
+
+  context 'Samba INI file with dollars in section names' do
+    let(:sample_content) do
+      template = <<-EOS
+      [global]
+        workgroup = FELLOWSHIP
+        ; ...
+        idmap config * : backend = tdb
+
+      [printers]
+        comment = All Printers
+        ; ...
+        browseable = No
+
+      [print$]
+        comment = Printer Drivers
+        path = /var/lib/samba/printers
+
+      [Shares]
+        path = /home/shares
+        read only = No
+        guest ok = Yes
+      EOS
+      template.split("\n")
+    end
+
+    it "should parse the correct section_names" do
+      subject.section_names.should match_array [
+        '',
+        'global',
+        'printers',
+        'print$',
+        'Shares'
+      ]
+    end
+  end
+
+  context 'section names with forward slashes in them' do
+    let(:sample_content) do
+      template = <<-EOS
+[monitor:///var/log/*.log]
+disabled = test_value
+      EOS
+      template.split("\n")
+    end
+
+    it "should parse the correct section_names" do
+      subject.section_names.should match_array [
+        '',
+        'monitor:///var/log/*.log'
+      ]
+    end
+  end
 end