From 45732a8f28ef082e7a6672af316495dff95ecb76 Mon Sep 17 00:00:00 2001
From: Brandon Keepers <brandon@collectiveidea.com>
Date: Wed, 31 Dec 2008 09:51:29 -0500
Subject: [PATCH] Dup keys in OrderedHash to prevent them from being modified [#1676 state:resolved]

---
 activesupport/lib/active_support/ordered_hash.rb |   31 +++++++++++++++------
 activesupport/test/ordered_hash_test.rb          |   10 ++++++-
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb
index 3def0be..bc108d8 100644
--- a/activesupport/lib/active_support/ordered_hash.rb
+++ b/activesupport/lib/active_support/ordered_hash.rb
@@ -10,10 +10,14 @@ module ActiveSupport
         @keys = []
       end
 
+      def initialize_copy(other)
+        super
+        # make a deep copy of keys
+        @keys = other.keys.dup
+      end
+
       def []=(key, value)
-        if !has_key?(key)
-          @keys << key
-        end
+        @keys << key if !has_key?(key)
         super
       end
 
@@ -24,6 +28,12 @@ module ActiveSupport
         end
         super
       end
+      
+      def delete_if
+        super
+        sync_keys!
+        self
+      end
 
       def reject!
         super
@@ -36,7 +46,7 @@ module ActiveSupport
       end
 
       def keys
-        @keys
+        @keys.dup
       end
 
       def values
@@ -56,7 +66,7 @@ module ActiveSupport
       end
 
       def each
-        keys.each {|key| yield [key, self[key]]}
+        @keys.each {|key| yield [key, self[key]]}
       end
 
       alias_method :each_pair, :each
@@ -73,13 +83,16 @@ module ActiveSupport
         [k, v]
       end
 
+      def merge!(other_hash)
+        other_hash.each {|k,v| self[k] = v }
+        self
+      end
+
       def merge(other_hash)
-        result = dup
-        other_hash.each {|k,v| result[k]=v}
-        result
+        dup.merge!(other_hash)
       end
 
-      private
+    private
 
       def sync_keys!
         @keys.delete_if {|k| !has_key?(k)}
diff --git a/activesupport/test/ordered_hash_test.rb b/activesupport/test/ordered_hash_test.rb
index 0e2aa45..fb76ca1 100644
--- a/activesupport/test/ordered_hash_test.rb
+++ b/activesupport/test/ordered_hash_test.rb
@@ -98,7 +98,8 @@ class OrderedHashTest < Test::Unit::TestCase
   end
 
   def test_delete_if
-    (copy = @ordered_hash.dup).delete('pink')
+    copy = @ordered_hash.dup
+    copy.delete('pink')
     assert_equal copy, @ordered_hash.delete_if { |k, _| k == 'pink' }
     assert !@ordered_hash.keys.include?('pink')
   end
@@ -115,6 +116,7 @@ class OrderedHashTest < Test::Unit::TestCase
     new_ordered_hash = @ordered_hash.reject { |k, _| k == 'pink' }
     assert_equal copy, @ordered_hash
     assert !new_ordered_hash.keys.include?('pink')
+    assert @ordered_hash.keys.include?('pink')
   end
 
   def test_clear
@@ -140,4 +142,10 @@ class OrderedHashTest < Test::Unit::TestCase
     assert_equal [@keys.first, @values.first], pair
     assert !@ordered_hash.keys.include?(pair.first)
   end
+  
+  def test_keys
+    original = @ordered_hash.keys.dup
+    @ordered_hash.keys.pop
+    assert_equal original, @ordered_hash.keys
+  end
 end
\ No newline at end of file
-- 
1.6.0.1