Fix to fix for #5755 -- backref serialization issues in zaml

This commit addresses the original issue that the change reverted in the previous
commit for #5755 was intended to fix by removing the special case on labels in
emit (lables, even if they are never generated, are not "new lines" and thus
@recent_nl should always be set to false when one is emitted).

It also partially addresses a related issue wherein temporary strings generated
when field names are constructed recycle their object_id (they are temporary)
and thus cause incorrect back references.  This commit "fixes" the problem by
never generating backrefs to strings (treating them as immutable).

It does not address other suspected issues such as thread safety durring serialization
due to the use of class variables to store the seen-object hash or the use of object
ids as "permanently unique" identifiers.

Paired with: Daniel Pittman
Advice & Commiseration: Jesse Wolfe

2 files changed, 50 insertions, 28 deletions

diff --git a/lib/puppet/util/zaml.rb b/lib/puppet/util/zaml.rb
index 6ac956565..14aa90e5f 100644
--- a/lib/puppet/util/zaml.rb
+++ b/lib/puppet/util/zaml.rb
@@ -91,7 +91,7 @@ class ZAML
   end
   def emit(s)
     @result << s
-    @recent_nl = false unless s.kind_of?(Label)
+    @recent_nl = false
   end
   def nl(s='')
     emit(@indent || "\n") unless @recent_nl
@@ -223,32 +223,30 @@ class String
     gsub( /([\x80-\xFF])/ ) { |x| "\\x#{x.unpack("C")[0].to_s(16)}" }
   end
   def to_zaml(z)
-    z.first_time_only(self) {
-      num = '[-+]?(0x)?\d+\.?\d*'
-      case
-        when self == ''
-          z.emit('""')
-        # when self =~ /[\x00-\x08\x0B\x0C\x0E-\x1F\x80-\xFF]/
-        #   z.emit("!binary |\n")
-        #   z.emit([self].pack("m*"))
-        when (
-          (self =~ /\A(true|false|yes|no|on|null|off|#{num}(:#{num})*|!|=|~)$/i) or
-          (self =~ /\A\n* /) or
-          (self =~ /[\s:]$/) or
-          (self =~ /^[>|][-+\d]*\s/i) or
-          (self[-1..-1] =~ /\s/) or
-          (self =~ /[\x00-\x08\x0B\x0C\x0E-\x1F\x80-\xFF]/) or
-          (self =~ /[,\[\]\{\}\r\t]|:\s|\s#/) or
-          (self =~ /\A([-:?!#&*'"]|<<|%.+:.)/)
-          )
-          z.emit("\"#{escaped_for_zaml}\"")
-        when self =~ /\n/
-          if self[-1..-1] == "\n" then z.emit('|+') else z.emit('|-') end
-          z.nested { split("\n",-1).each { |line| z.nl; z.emit(line.chomp("\n")) } }
-        else
-          z.emit(self)
-      end
-    }
+    num = '[-+]?(0x)?\d+\.?\d*'
+    case
+      when self == ''
+        z.emit('""')
+      # when self =~ /[\x00-\x08\x0B\x0C\x0E-\x1F\x80-\xFF]/
+      #   z.emit("!binary |\n")
+      #   z.emit([self].pack("m*"))
+      when (
+        (self =~ /\A(true|false|yes|no|on|null|off|#{num}(:#{num})*|!|=|~)$/i) or
+        (self =~ /\A\n* /) or
+        (self =~ /[\s:]$/) or
+        (self =~ /^[>|][-+\d]*\s/i) or
+        (self[-1..-1] =~ /\s/) or
+        (self =~ /[\x00-\x08\x0B\x0C\x0E-\x1F\x80-\xFF]/) or
+        (self =~ /[,\[\]\{\}\r\t]|:\s|\s#/) or
+        (self =~ /\A([-:?!#&*'"]|<<|%.+:.)/)
+        )
+        z.emit("\"#{escaped_for_zaml}\"")
+      when self =~ /\n/
+        if self[-1..-1] == "\n" then z.emit('|+') else z.emit('|-') end
+        z.nested { split("\n",-1).each { |line| z.nl; z.emit(line.chomp("\n")) } }
+      else
+        z.emit(self)
+    end
   end
 end
 
diff --git a/spec/unit/util/zaml_spec.rb b/spec/unit/util/zaml_spec.rb
index 59590c571..782ebdb0b 100755
--- a/spec/unit/util/zaml_spec.rb
+++ b/spec/unit/util/zaml_spec.rb
@@ -53,8 +53,32 @@ describe "Pure ruby yaml implementation" do
   it "should handle references to Scalar in Hash" do
     str = "hello"
     data = { "one" => str, "two" => str }
-    data.to_yaml.should == "--- \n  two: &id001 hello\n  one: *id001"
     expect { YAML.load(data.to_yaml).should == data }.should_not raise_error
   end
+
+  class Zaml_test_class_A
+    attr_reader :false,:true
+    def initialize
+      @false = @true = 7
+    end
+  end
+  it "should not blow up when magic strings are used as field names" do
+    data = Zaml_test_class_A.new
+    data.to_yaml.should == %Q{--- !ruby/object:Zaml_test_class_A\n  \"false\": 7\n  \"true\": 7}
+    expect { 
+      r = YAML.load(data.to_yaml)
+      r.class.should == data.class
+      r.true.should == data.true
+      r.false.should == data.false
+    }.should_not raise_error
+  end
+
+  it "should not blow up on back references inside arrays" do
+    s = [1,2]
+    data = [s,s]
+    data.to_yaml.should == %Q{--- \n  - &id001 \n    - 1\n    - 2\n  - *id001}
+    expect { YAML.load(data.to_yaml).should == data }.should_not raise_error
+  end
+
 end