closes #293. Protect read conversion cache values

BallAerospace · May 5, 2016 · 2552bf5 · 2552bf5
1 parent 8328561
commit 2552bf5
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 0 deletions.
diff --git a/lib/cosmos/packets/packet.rb b/lib/cosmos/packets/packet.rb
@@ -338,6 +338,15 @@ def read_item(item, value_type = :CONVERTED, buffer = @buffer)
             synchronize_allow_reads() do
               if @read_conversion_cache[item]
                 value = @read_conversion_cache[item]
+
+                # Make sure cached value is not modified by anyone by creating
+                # a deep copy
+                if String === value
+                  value = value.clone
+                elsif Array === value
+                  value = Marshal.load(Marshal.dump(value))
+                end
+
                 using_cached_value = true
               end
             end
@@ -355,6 +364,14 @@ def read_item(item, value_type = :CONVERTED, buffer = @buffer)
               synchronize_allow_reads() do
                 @read_conversion_cache ||= {}
                 @read_conversion_cache[item] = value
+
+                # Make sure cached value is not modified by anyone by creating
+                # a deep copy
+                if String === value
+                  value = value.clone
+                elsif Array === value
+                  value = Marshal.load(Marshal.dump(value))
+                end
               end
             end
           end

diff --git a/spec/packets/packet_spec.rb b/spec/packets/packet_spec.rb
@@ -370,6 +370,36 @@ module Cosmos
         expect(@p.read_item(i, :CONVERTED, "\x02")).to eql 1
       end
 
+      it "prevents the read conversion cache from being corrupted" do
+        @p.append_item("item",8,:UINT)
+        i = @p.get_item("ITEM")
+        i.read_conversion = GenericConversion.new("'A String'")
+        i.units = "with units"
+        value = @p.read_item(i, :CONVERTED)
+        expect(value).to eql 'A String'
+        value << 'That got modified'
+        value = @p.read_item(i, :WITH_UNITS)
+        expect(value).to eql 'A String with units'
+        value << 'That got modified'
+        expect(@p.read_item(i, :WITH_UNITS)).to eql 'A String with units'
+        value = @p.read_item(i, :WITH_UNITS)
+        value << ' more things'
+        expect(@p.read_item(i, :WITH_UNITS)).to eql 'A String with units'
+
+        @p.buffer = "\x00"
+        i.read_conversion = GenericConversion.new("['A', 'B', 'C']")
+        value = @p.read_item(i, :CONVERTED)
+        expect(value).to eql ['A', 'B', 'C']
+        value << 'D'
+        value = @p.read_item(i, :WITH_UNITS)
+        expect(value).to eql ['A with units', 'B with units', 'C with units']
+        value << 'D'
+        expect(@p.read_item(i, :WITH_UNITS)).to eql ['A with units', 'B with units', 'C with units']
+        value = @p.read_item(i, :WITH_UNITS)
+        value << 'D'
+        expect(@p.read_item(i, :WITH_UNITS)).to eql ['A with units', 'B with units', 'C with units']
+      end
+
       it "reads the CONVERTED value with states" do
         @p.append_item("item",8,:UINT)
         i = @p.get_item("ITEM")