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

Signed-off-by: Markus Roberts <[email protected]>
---
 lib/puppet/util/zaml.rb     |   52 ++++++++++++++++++++----------------------
 spec/unit/util/zaml_spec.rb |   26 ++++++++++++++++++++-
 2 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/lib/puppet/util/zaml.rb b/lib/puppet/util/zaml.rb
index 6ac9565..14aa90e 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 59590c5..782ebdb 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
 
-- 
1.7.0.4

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to