Merge branch 'master' into nested_has_many_through

13 files changed, 65 insertions, 44 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 8f283e1117..75657cb6ee 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -15,6 +15,10 @@
 
   [Aaron Patterson]
 
+*Rails 3.0.1 (October 15, 2010)*
+
+* Introduce a fix for CVE-2010-3993
+
 *Rails 3.0.0 (August 29, 2010)*
 
 * Changed update_attribute to not run callbacks and update the record directly in the database [Neeraj Singh]
diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index 7297af9f79..67f70c434e 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -18,7 +18,7 @@ module ActiveRecord
       # method is defined by Active Record though.
       def instance_method_already_implemented?(method_name)
         method_name = method_name.to_s
-        @_defined_class_methods         ||= ancestors.first(ancestors.index(ActiveRecord::Base)).sum([]) { |m| m.public_instance_methods(false) | m.private_instance_methods(false) | m.protected_instance_methods(false) }.map {|m| m.to_s }.to_set
+        @_defined_class_methods         ||= ancestors.first(ancestors.index(ActiveRecord::Base)).sum([]) { |m| m.instance_methods(false) | m.private_instance_methods(false) }.map {|m| m.to_s }.to_set
         @@_defined_activerecord_methods ||= defined_activerecord_methods
         raise DangerousAttributeError, "#{method_name} is defined by ActiveRecord" if @@_defined_activerecord_methods.include?(method_name)
         @_defined_class_methods.include?(method_name)
diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb
index 01699746d8..ad5a3e7562 100644
--- a/activerecord/lib/active_record/attribute_methods/read.rb
+++ b/activerecord/lib/active_record/attribute_methods/read.rb
@@ -85,7 +85,7 @@ module ActiveRecord
       def _read_attribute(attr_name)
         attr_name = attr_name.to_s
         attr_name = self.class.primary_key if attr_name == 'id'
-        if !(value = @attributes[attr_name]).nil?
+        if value = @attributes[attr_name]
           if column = column_for_attribute(attr_name)
             if unserializable_attribute?(attr_name, column)
               unserialize_attribute(attr_name)
diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index 49671a1042..47428cfd0f 100644
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -218,16 +218,6 @@ module ActiveRecord
   # needs to be aware of it because an ordinary +save+ will raise such exception
   # instead of quietly returning +false+.
   #
-  # == Debugging callbacks
-  #
-  # To list the methods and procs registered with a particular callback, append <tt>_callback_chain</tt> to
-  # the callback name that you wish to list and send that to your class from the Rails console:
-  #
-  #   >> Topic.after_save_callback_chain
-  #   => [#<ActiveSupport::Callbacks::Callback:0x3f6a448
-  #       @method=#<Proc:0x03f9a42c@/Users/foo/bar/app/models/topic.rb:43>, kind:after_save, identifiernil,
-  #       options{}]
-  #
   module Callbacks
     extend ActiveSupport::Concern
 
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index ec7035e540..3716937689 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -72,7 +72,7 @@ module ActiveRecord
           end
 
           adapter_method = "#{spec[:adapter]}_connection"
-          if !respond_to?(adapter_method)
+          unless respond_to?(adapter_method)
             raise AdapterNotFound, "database configuration specifies nonexistent #{spec[:adapter]} adapter"
           end
 
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index dce9e99d27..5949985e4d 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -465,8 +465,8 @@ module ActiveRecord
               #  (2) $12.345.678,12
               case data
               when /^-?\D+[\d,]+\.\d{2}$/  # (1)
-                data.gsub!(/[^-\d\.]/, '')
-              when /^-?\D+[\d\.]+,\d{2}$/  # (2)
+                data.gsub!(/[^-\d.]/, '')
+              when /^-?\D+[\d.]+,\d{2}$/  # (2)
                 data.gsub!(/[^-\d,]/, '').sub!(/,/, '.')
               end
             end
@@ -849,6 +849,10 @@ module ActiveRecord
         execute "DROP INDEX #{quote_table_name(index_name)}"
       end
 
+      def rename_index(table_name, old_name, new_name)
+        execute "ALTER INDEX #{quote_column_name(old_name)} RENAME TO #{quote_table_name(new_name)}"
+      end
+
       def index_name_length
         63
       end
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index bdd940f3ee..aca91c907d 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -324,9 +324,7 @@ module ActiveRecord
         assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy]) unless call_reject_if(association_name, attributes)
 
       elsif attributes['id']
-        existing_record = self.class.reflect_on_association(association_name).klass.find(attributes['id'])
-        assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
-        self.send(association_name.to_s+'=', existing_record)
+        raise_nested_attributes_record_not_found(association_name, attributes['id'])
 
       elsif !reject_new_record?(association_name, attributes)
         method = "build_#{association_name}"
@@ -402,15 +400,12 @@ module ActiveRecord
             association.build(attributes.except(*UNASSIGNABLE_KEYS))
           end
 
-        elsif existing_records.count == 0 #Existing record but not yet associated
-          existing_record = self.class.reflect_on_association(association_name).klass.find(attributes['id'])
-          association.send(:add_record_to_target_with_callbacks, existing_record) if !association.loaded? && !call_reject_if(association_name, attributes)
-          assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
-
         elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes['id'].to_s }
           association.send(:add_record_to_target_with_callbacks, existing_record) if !association.loaded? && !call_reject_if(association_name, attributes)
           assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
 
+        else
+          raise_nested_attributes_record_not_found(association_name, attributes['id'])
         end
       end
     end
@@ -430,7 +425,7 @@ module ActiveRecord
       ConnectionAdapters::Column.value_to_boolean(hash['_destroy'])
     end
 
-    # Determines if a new record should be built by checking for
+    # Determines if a new record should be build by checking for
     # has_destroy_flag? or if a <tt>:reject_if</tt> proc exists for this
     # association and evaluates to +true+.
     def reject_new_record?(association_name, attributes)
@@ -446,5 +441,9 @@ module ActiveRecord
       end
     end
 
+    def raise_nested_attributes_record_not_found(association_name, record_id)
+      reflection = self.class.reflect_on_association(association_name)
+      raise RecordNotFound, "Couldn't find #{reflection.klass.name} with ID=#{record_id} for #{self.class.name} with ID=#{id}"
+    end
   end
 end
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 04ba5b291e..f129b54f9a 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -379,7 +379,7 @@ module ActiveRecord
       return [] if string.blank?
       # always convert table names to downcase as in Oracle quoted table names are in uppercase
       # ignore raw_sql_ that is used by Oracle adapter as alias for limit/offset subqueries
-      string.scan(/([a-zA-Z_][\.\w]+).?\./).flatten.map{ |s| s.downcase }.uniq - ['raw_sql_']
+      string.scan(/([a-zA-Z_][.\w]+).?\./).flatten.map{ |s| s.downcase }.uniq - ['raw_sql_']
     end
 
   end
diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb
index f5331bb8a9..e30b481fe1 100644
--- a/activerecord/lib/active_record/schema_dumper.rb
+++ b/activerecord/lib/active_record/schema_dumper.rb
@@ -114,9 +114,9 @@ HEADER
                                  column.type.to_s
                                end
             spec[:limit]     = column.limit.inspect if column.limit != @types[column.type][:limit] && spec[:type] != 'decimal'
-            spec[:precision] = column.precision.inspect if !column.precision.nil?
-            spec[:scale]     = column.scale.inspect if !column.scale.nil?
-            spec[:null]      = 'false' if !column.null
+            spec[:precision] = column.precision.inspect if column.precision
+            spec[:scale]     = column.scale.inspect if column.scale
+            spec[:null]      = 'false' unless column.null
             spec[:default]   = default_string(column.default) if column.has_default?
             (spec.keys - [:name, :type]).each{ |k| spec[k].insert(0, "#{k.inspect} => ")}
             spec
diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index a7709b9489..ab737f0f88 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -224,8 +224,8 @@ module ActiveRecord
     end
 
     # See ActiveRecord::Transactions::ClassMethods for detailed documentation.
-    def transaction(&block)
-      self.class.transaction(&block)
+    def transaction(options = {}, &block)
+      self.class.transaction(options, &block)
     end
 
     def destroy #:nodoc:
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 16fd9a7465..8acee9ac71 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -48,6 +48,11 @@ class Boolean < ActiveRecord::Base; end
 class BasicsTest < ActiveRecord::TestCase
   fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :categorizations, :categories, :posts
 
+  def test_select_symbol
+    topic_ids = Topic.select(:id).map(&:id).sort
+    assert_equal Topic.find(:all).map(&:id).sort, topic_ids
+  end
+
   def test_table_exists
     assert !NonExistentTable.table_exists?
     assert Topic.table_exists?
diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb
index 8382ca048b..60f89da95e 100644
--- a/activerecord/test/cases/nested_attributes_test.rb
+++ b/activerecord/test/cases/nested_attributes_test.rb
@@ -203,6 +203,12 @@ class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase
     assert_equal 'Davy Jones Gold Dagger', @pirate.ship.name
   end
 
+  def test_should_raise_RecordNotFound_if_an_id_is_given_but_doesnt_return_a_record
+    assert_raise_with_message ActiveRecord::RecordNotFound, "Couldn't find Ship with ID=1234567890 for Pirate with ID=#{@pirate.id}" do
+      @pirate.ship_attributes = { :id => 1234567890 }
+    end
+  end
+
   def test_should_take_a_hash_with_string_keys_and_update_the_associated_model
     @pirate.reload.ship_attributes = { 'id' => @ship.id, 'name' => 'Davy Jones Gold Dagger' }
 
@@ -382,13 +388,10 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase
     assert_equal 'Arr', @ship.pirate.catchphrase
   end
 
-  def test_should_associate_with_record_if_parent_record_is_not_saved
-    @ship.destroy
-    @pirate = Pirate.create(:catchphrase => 'Arr')
-    @ship = Ship.new(:name => 'Nights Dirty Lightning', :pirate_attributes => { :id => @pirate.id, :catchphrase => @pirate.catchphrase})
-
-    assert_equal @ship.name, 'Nights Dirty Lightning'
-    assert_equal @pirate, @ship.pirate
+  def test_should_raise_RecordNotFound_if_an_id_is_given_but_doesnt_return_a_record
+    assert_raise_with_message ActiveRecord::RecordNotFound, "Couldn't find Pirate with ID=1234567890 for Ship with ID=#{@ship.id}" do
+      @ship.pirate_attributes = { :id => 1234567890 }
+    end
   end
 
   def test_should_take_a_hash_with_string_keys_and_update_the_associated_model
@@ -518,11 +521,6 @@ module NestedAttributesOnACollectionAssociationTests
     assert_equal ['Grace OMalley', 'Privateers Greed'], [@child_1.reload.name, @child_2.reload.name]
   end
 
-  def test_should_assign_existing_children_if_parent_is_new
-    @pirate = Pirate.new({:catchphrase => "Don' botharr talkin' like one, savvy?"}.merge(@alternate_params))
-    assert_equal ['Grace OMalley', 'Privateers Greed'], [@pirate.send(@association_name)[0].name, @pirate.send(@association_name)[1].name]
-  end
-
   def test_should_also_work_with_a_HashWithIndifferentAccess
     @pirate.send(association_setter, HashWithIndifferentAccess.new('foo' => HashWithIndifferentAccess.new(:id => @child_1.id, :name => 'Grace OMalley')))
     @pirate.save
@@ -586,8 +584,8 @@ module NestedAttributesOnACollectionAssociationTests
     assert_equal ['Grace OMalley', 'Privateers Greed'], [@child_1.name, @child_2.name]
   end
 
-  def test_should_not_raise_RecordNotFound_if_an_id_is_given_but_doesnt_return_a_record
-    assert_nothing_raised ActiveRecord::RecordNotFound do
+  def test_should_raise_RecordNotFound_if_an_id_is_given_but_doesnt_return_a_record
+    assert_raise_with_message ActiveRecord::RecordNotFound, "Couldn't find #{@child_1.class.name} with ID=1234567890 for Pirate with ID=#{@pirate.id}" do
       @pirate.attributes = { association_getter => [{ :id => 1234567890 }] }
     end
   end
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index 8385286fd0..44af54b143 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -263,6 +263,27 @@ class TransactionTest < ActiveRecord::TestCase
     assert !@second.reload.approved?
   end if Topic.connection.supports_savepoints?
 
+  def test_force_savepoint_on_instance
+    @first.transaction do
+      @first.approved  = true
+      @second.approved = false
+      @first.save!
+      @second.save!
+
+      begin
+        @second.transaction :requires_new => true do
+          @first.happy = false
+          @first.save!
+          raise
+        end
+      rescue
+      end
+    end
+
+    assert @first.reload.approved?
+    assert !@second.reload.approved?
+  end if Topic.connection.supports_savepoints?
+
   def test_no_savepoint_in_nested_transaction_without_force
     Topic.transaction do
       @first.approved = true