11 files changed, 337 insertions, 50 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 2b6425caa5..3b2b1330fc 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,5 +1,22 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Given
+
+        params.permit(:name)
+
+    `:name` passes if it is a key of `params` whose value is a permitted scalar.
+
+    Similarly, given
+
+        params.permit(tags: [])
+
+    `:tags` passes if it is a key of `params` whose value is an array of
+    permitted scalars.
+
+    Permitted scalars filtering happens at any level of nesting.
+
+    *Xavier Noria*
+
 *   `BestStandardsSupport` no longer duplicates `X-UA-Compatible` values on
     each request to prevent header size from blowing up.
 
diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb
index 2c96e03f55..0d84fbb62b 100644
--- a/actionpack/lib/action_controller/metal/strong_parameters.rb
+++ b/actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -184,6 +184,20 @@ module ActionController
     #   permitted.has_key?(:age)  # => true
     #   permitted.has_key?(:role) # => false
     #
+    # Only permitted scalars pass the filter. For example, given
+    #
+    #   params.permit(:name)
+    #
+    # +:name+ passes it is a key of +params+ whose associated value is of type
+    # +String+, +Symbol+, +NilClass+, +Numeric+, +TrueClass+, +FalseClass+,
+    # +Date+, +Time+, +DateTime+, +StringIO+, or +IO+. Otherwise, the key +:name+
+    # is filtered out.
+    #
+    # You may declare that the parameter should be an array of permitted scalars
+    # by mapping it to an empty array:
+    #
+    #   params.permit(tags: [])
+    #
     # You can also use +permit+ on nested parameters, like:
     #
     #   params = ActionController::Parameters.new({
@@ -230,29 +244,10 @@ module ActionController
 
       filters.flatten.each do |filter|
         case filter
-        when Symbol, String then
-          if has_key?(filter)
-            _value = self[filter]
-            params[filter] = _value unless Hash === _value
-          end
-          keys.grep(/\A#{Regexp.escape(filter)}\(\d+[if]?\)\z/) { |key| params[key] = self[key] }
+        when Symbol, String
+          permitted_scalar_filter(params, filter)
         when Hash then
-          filter = filter.with_indifferent_access
-
-          self.slice(*filter.keys).each do |key, values|
-            return unless values
-
-            key = key.to_sym
-
-            params[key] = each_element(values) do |value|
-              # filters are a Hash, so we expect value to be a Hash too
-              next if filter.is_a?(Hash) && !value.is_a?(Hash)
-
-              value = self.class.new(value) if !value.respond_to?(:permit)
-
-              value.permit(*Array.wrap(filter[key]))
-            end
-          end
+          hash_filter(params, filter)
         end
       end
 
@@ -352,6 +347,81 @@ module ActionController
       def unpermitted_keys(params)
         self.keys - params.keys - NEVER_UNPERMITTED_PARAMS
       end
+
+      #
+      # --- Filtering ----------------------------------------------------------
+      #
+
+      # This is a white list of permitted scalar types that includes the ones
+      # supported in XML and JSON requests.
+      #
+      # This list is in particular used to filter ordinary requests, String goes
+      # as first element to quickly short-circuit the common case.
+      #
+      # If you modify this collection please update the API of +permit+ above.
+      PERMITTED_SCALAR_TYPES = [
+        String,
+        Symbol,
+        NilClass,
+        Numeric,
+        TrueClass,
+        FalseClass,
+        Date,
+        Time,
+        # DateTimes are Dates, we document the type but avoid the redundant check.
+        StringIO,
+        IO,
+      ]
+
+      def permitted_scalar?(value)
+        PERMITTED_SCALAR_TYPES.any? {|type| value.is_a?(type)}
+      end
+
+      def permitted_scalar_filter(params, key)
+        if has_key?(key) && permitted_scalar?(self[key])
+          params[key] = self[key]
+        end
+
+        keys.grep(/\A#{Regexp.escape(key)}\(\d+[if]?\)\z/) do |k|
+          if permitted_scalar?(self[k])
+            params[k] = self[k]
+          end
+        end
+      end
+
+      def array_of_permitted_scalars?(value)
+        if value.is_a?(Array)
+          value.all? {|element| permitted_scalar?(element)}
+        end
+      end
+
+      def array_of_permitted_scalars_filter(params, key)
+        if has_key?(key) && array_of_permitted_scalars?(self[key])
+          params[key] = self[key]
+        end
+      end
+
+      def hash_filter(params, filter)
+        filter = filter.with_indifferent_access
+
+        # Slicing filters out non-declared keys.
+        slice(*filter.keys).each do |key, value|
+          return unless value
+
+          if filter[key] == []
+            # Declaration { comment_ids: [] }.
+            array_of_permitted_scalars_filter(params, key)
+          else
+            # Declaration { user: :name } or { user: [:name, :age, { adress: ... }] }.
+            params[key] = each_element(value) do |element|
+              if element.is_a?(Hash)
+                element = self.class.new(element) unless element.respond_to?(:permit)
+                element.permit(*Array.wrap(filter[key]))
+              end
+            end
+          end
+        end
+      end
   end
 
   # == Strong \Parameters
diff --git a/actionpack/test/controller/parameters/nested_parameters_test.rb b/actionpack/test/controller/parameters/nested_parameters_test.rb
index 6df849c4e2..8aec159499 100644
--- a/actionpack/test/controller/parameters/nested_parameters_test.rb
+++ b/actionpack/test/controller/parameters/nested_parameters_test.rb
@@ -2,6 +2,10 @@ require 'abstract_unit'
 require 'action_controller/metal/strong_parameters'
 
 class NestedParametersTest < ActiveSupport::TestCase
+  def assert_filtered_out(params, key)
+    assert !params.has_key?(key), "key #{key.inspect} has not been filtered out"
+  end
+
   test "permitted nested parameters" do
     params = ActionController::Parameters.new({
       book: {
@@ -11,6 +15,8 @@ class NestedParametersTest < ActiveSupport::TestCase
           born: "1564-04-26"
         }, {
           name: "Christopher Marlowe"
+        }, {
+          :name => %w(malicious injected names)
         }],
         details: {
           pages: 200,
@@ -30,10 +36,12 @@ class NestedParametersTest < ActiveSupport::TestCase
     assert_equal "William Shakespeare", permitted[:book][:authors][0][:name]
     assert_equal "Christopher Marlowe", permitted[:book][:authors][1][:name]
     assert_equal 200, permitted[:book][:details][:pages]
-    assert_nil permitted[:book][:id]
-    assert_nil permitted[:book][:details][:genre]
-    assert_nil permitted[:book][:authors][0][:born]
-    assert_nil permitted[:magazine]
+
+    assert_filtered_out permitted, :magazine
+    assert_filtered_out permitted[:book], :id
+    assert_filtered_out permitted[:book][:details], :genre
+    assert_filtered_out permitted[:book][:authors][0], :born
+    assert_filtered_out permitted[:book][:authors][2], :name
   end
 
   test "permitted nested parameters with a string or a symbol as a key" do
@@ -68,7 +76,7 @@ class NestedParametersTest < ActiveSupport::TestCase
       }
     })
 
-    permitted = params.permit :book => :genres
+    permitted = params.permit :book => {:genres => []}
     assert_equal ["Tragedy"], permitted[:book][:genres]
   end
 
@@ -124,19 +132,41 @@ class NestedParametersTest < ActiveSupport::TestCase
 
   test "fields_for-style nested params" do
     params = ActionController::Parameters.new({
-      book: {
-        authors_attributes: {
-          :'0' => { name: 'William Shakespeare', age_of_death: '52' },
-          :'-1' => { name: 'Unattributed Assistant' }
+      :book => {
+        :authors_attributes => {
+          :'0' => { :name => 'William Shakespeare', :age_of_death => '52' },
+          :'1' => { :name => 'Unattributed Assistant' },
+          :'2' => { :name => %w(injected names)}
         }
       }
     })
-    permitted = params.permit book: { authors_attributes: [ :name ] }
+    permitted = params.permit :book => { :authors_attributes => [ :name ] }
 
     assert_not_nil permitted[:book][:authors_attributes]['0']
-    assert_not_nil permitted[:book][:authors_attributes]['-1']
-    assert_nil permitted[:book][:authors_attributes]['0'][:age_of_death]
+    assert_not_nil permitted[:book][:authors_attributes]['1']
+    assert_empty permitted[:book][:authors_attributes]['2']
     assert_equal 'William Shakespeare', permitted[:book][:authors_attributes]['0'][:name]
-    assert_equal 'Unattributed Assistant', permitted[:book][:authors_attributes]['-1'][:name]
+    assert_equal 'Unattributed Assistant', permitted[:book][:authors_attributes]['1'][:name]
+
+    assert_filtered_out permitted[:book][:authors_attributes]['0'], :age_of_death
+  end
+
+  test "fields_for-style nested params with negative numbers" do
+    params = ActionController::Parameters.new({
+      :book => {
+        :authors_attributes => {
+          :'-1' => { :name => 'William Shakespeare', :age_of_death => '52' },
+          :'-2' => { :name => 'Unattributed Assistant' }
+        }
+      }
+    })
+    permitted = params.permit :book => { :authors_attributes => [:name] }
+
+    assert_not_nil permitted[:book][:authors_attributes]['-1']
+    assert_not_nil permitted[:book][:authors_attributes]['-2']
+    assert_equal 'William Shakespeare', permitted[:book][:authors_attributes]['-1'][:name]
+    assert_equal 'Unattributed Assistant', permitted[:book][:authors_attributes]['-2'][:name]
+
+    assert_filtered_out permitted[:book][:authors_attributes]['-1'], :age_of_death
   end
 end
diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb
index 7cc71fe6dc..303c13eb25 100644
--- a/actionpack/test/controller/parameters/parameters_permit_test.rb
+++ b/actionpack/test/controller/parameters/parameters_permit_test.rb
@@ -2,10 +2,131 @@ require 'abstract_unit'
 require 'action_controller/metal/strong_parameters'
 
 class ParametersPermitTest < ActiveSupport::TestCase
+  def assert_filtered_out(params, key)
+    assert !params.has_key?(key), "key #{key.inspect} has not been filtered out"
+  end
+
   setup do
     @params = ActionController::Parameters.new({ person: {
       age: "32", name: { first: "David", last: "Heinemeier Hansson" }
     }})
+
+    @struct_fields = []
+    %w(0 1 12).each do |number|
+      ['', 'i', 'f'].each do |suffix|
+        @struct_fields << "sf(#{number}#{suffix})"
+      end
+    end
+  end
+
+  test 'if nothing is permitted, the hash becomes empty' do
+    params = ActionController::Parameters.new(:id => '1234')
+    permitted = params.permit
+    assert permitted.permitted?
+    assert permitted.empty?
+  end
+
+  test 'key: permitted scalar values' do
+    values  = ['a', :a, nil]
+    values += [0, 1.0, 2**128, BigDecimal.new(1)]
+    values += [true, false]
+    values += [Date.today, Time.now, DateTime.now]
+    values += [StringIO.new]
+
+    values.each do |value|
+      params = ActionController::Parameters.new(:id => value)
+      permitted = params.permit(:id)
+      assert_equal value, permitted[:id]
+
+      @struct_fields.each do |sf|
+        params = ActionController::Parameters.new(sf => value)
+        permitted = params.permit(:sf)
+        assert_equal value, permitted[sf]
+      end
+    end
+  end
+
+  test 'key: unknown keys are filtered out' do
+    params = ActionController::Parameters.new(:id => '1234', :injected => 'injected')
+    permitted = params.permit(:id)
+    assert_equal '1234', permitted[:id]
+    assert_filtered_out permitted, :injected
+  end
+
+  test 'key: arrays are filtered out' do
+    [[], [1], ['1']].each do |array|
+      params = ActionController::Parameters.new(:id => array)
+      permitted = params.permit(:id)
+      assert_filtered_out permitted, :id
+
+      @struct_fields.each do |sf|
+        params = ActionController::Parameters.new(sf => array)
+        permitted = params.permit(:sf)
+        assert_filtered_out permitted, sf
+      end
+    end
+  end
+
+  test 'key: hashes are filtered out' do
+    [{}, {:foo => 1}, {:foo => 'bar'}].each do |hash|
+      params = ActionController::Parameters.new(:id => hash)
+      permitted = params.permit(:id)
+      assert_filtered_out permitted, :id
+
+      @struct_fields.each do |sf|
+        params = ActionController::Parameters.new(sf => hash)
+        permitted = params.permit(:sf)
+        assert_filtered_out permitted, sf
+      end
+    end
+  end
+
+  test 'key: non-permitted scalar values are filtered out' do
+    params = ActionController::Parameters.new(:id => Object.new)
+    permitted = params.permit(:id)
+    assert_filtered_out permitted, :id
+
+    @struct_fields.each do |sf|
+      params = ActionController::Parameters.new(sf => Object.new)
+      permitted = params.permit(:sf)
+      assert_filtered_out permitted, sf
+    end
+  end
+
+  test 'key: it is not assigned if not present in params' do
+    params = ActionController::Parameters.new(:name => 'Joe')
+    permitted = params.permit(:id)
+    assert !permitted.has_key?(:id)
+  end
+
+  test 'key to empty array: empty arrays pass' do
+    params = ActionController::Parameters.new(:id => [])
+    permitted = params.permit(:id => [])
+    assert_equal [], permitted[:id]
+  end
+
+  test 'key to empty array: arrays of permitted scalars pass' do
+    [['foo'], [1], ['foo', 'bar'], [1, 2, 3]].each do |array|
+      params = ActionController::Parameters.new(:id => array)
+      permitted = params.permit(:id => [])
+      assert_equal array, permitted[:id]
+    end
+  end
+
+  test 'key to empty array: permitted scalar values do not pass' do
+    ['foo', 1].each do |permitted_scalar|
+      params = ActionController::Parameters.new(:id => permitted_scalar)
+      permitted = params.permit(:id => [])
+      assert_filtered_out permitted, :id
+    end
+  end
+
+  test 'key to empty array: arrays of non-permitted scalar do not pass' do
+    [[Object.new], [[]], [[1]], [{}], [{:id => '1'}]].each do |non_permitted_scalar|
+      params = ActionController::Parameters.new(:id => non_permitted_scalar)
+      permitted = params.permit(:id => [])
+      assert_filtered_out permitted, :id
+    end
   end
 
   test "fetch raises ParameterMissing exception" do
@@ -73,10 +194,6 @@ class ParametersPermitTest < ActiveSupport::TestCase
     assert_equal "Jonas", @params[:person][:family][:brother]
   end
 
-  test "permitting parameters that are not there should not include the keys" do
-    assert !@params.permit(:person, :funky).has_key?(:funky)
-  end
-
   test "permit state is kept on a dup" do
     @params.permit!
     assert_equal @params.permitted?, @params.dup.permitted?
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 45a2b59d47..f1cca0ad76 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,10 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Added a state instance variable to each transaction. Will allow other objects
+    to know whether a transaction has been committed or rolled back.
+
+    *John Wang*
+
 *   Collection associations `#empty?` always respects builded records.
     Fix #8879.
 
diff --git a/activerecord/Rakefile b/activerecord/Rakefile
index 53ddff420e..0523314128 100644
--- a/activerecord/Rakefile
+++ b/activerecord/Rakefile
@@ -39,6 +39,11 @@ namespace :test do
   end
 end
 
+namespace :db do
+  task :create => ['mysql:build_databases', 'postgresql:build_databases']
+  task :drop => ['mysql:drop_databases', 'postgresql:drop_databases']
+end
+
 %w( mysql mysql2 postgresql sqlite3 sqlite3_mem firebird db2 oracle sybase openbase frontbase jdbcmysql jdbcpostgresql jdbcsqlite3 jdbcderby jdbch2 jdbchsqldb ).each do |adapter|
   Rake::TestTask.new("test_#{adapter}") { |t|
     adapter_short = adapter == 'db2' ? adapter : adapter[/^[a-z0-9]+/]
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
index 4cca94e40b..2b8026dbf9 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
@@ -5,7 +5,17 @@ module ActiveRecord
 
       def initialize(connection)
         @connection = connection
+        @state = nil
       end
+
+      def committed?
+        @state == :commit
+      end
+
+      def rolledback?
+        @state == :rollback
+      end
+
     end
 
     class ClosedTransaction < Transaction #:nodoc:
@@ -91,6 +101,7 @@ module ActiveRecord
       end
 
       def rollback_records
+        @state = :rollback
         records.uniq.each do |record|
           begin
             record.rolledback!(parent.closed?)
@@ -101,6 +112,7 @@ module ActiveRecord
       end
 
       def commit_records
+        @state = :commit
         records.uniq.each do |record|
           begin
             record.committed!
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index 94c6684700..812f1ce5c5 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -365,17 +365,18 @@ module ActiveRecord
       pk = self.class.primary_key
       @attributes[pk] = nil unless @attributes.key?(pk)
 
-      @aggregation_cache       = {}
-      @association_cache       = {}
-      @attributes_cache        = {}
-      @previously_changed      = {}
-      @changed_attributes      = {}
-      @readonly                = false
-      @destroyed               = false
-      @marked_for_destruction  = false
-      @new_record              = true
-      @txn                     = nil
-      @_start_transaction_state = {}
+      @aggregation_cache          = {}
+      @association_cache          = {}
+      @attributes_cache           = {}
+      @previously_changed         = {}
+      @changed_attributes         = {}
+      @readonly                   = false
+      @destroyed                  = false
+      @marked_for_destruction     = false
+      @new_record                 = true
+      @txn                        = nil
+      @_start_transaction_state   = {}
+      @transaction                = nil
     end
   end
 end
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 3a499a2025..64f1c9f6cc 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -404,6 +404,13 @@ class RelationTest < ActiveRecord::TestCase
     end
   end
 
+  def test_preload_applies_to_all_chained_preloaded_scopes
+    assert_queries(3) do
+      post = Post.with_comments.with_tags.first
+      assert post
+    end
+  end
+
   def test_find_with_included_associations
     assert_queries(2) do
       posts = Post.includes(:comments).order('posts.id')
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index bcbc48b38a..9d278480ef 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -451,6 +451,26 @@ class TransactionTest < ActiveRecord::TestCase
     end
   end
 
+  def test_transactions_state_from_rollback
+    connection = Topic.connection
+    transaction = ActiveRecord::ConnectionAdapters::ClosedTransaction.new(connection).begin
+
+    assert transaction.open?
+    transaction.perform_rollback
+    
+    assert transaction.rolledback?
+  end
+
+  def test_transactions_state_from_commit
+    connection = Topic.connection
+    transaction = ActiveRecord::ConnectionAdapters::ClosedTransaction.new(connection).begin
+
+    assert transaction.open?
+    transaction.perform_commit
+    
+    assert transaction.committed?
+  end
+
   private
 
   %w(validation save destroy).each do |filter|
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index 4433550dd5..603f1f2555 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -35,6 +35,9 @@ class Post < ActiveRecord::Base
   scope :with_very_special_comments, -> { joins(:comments).where(:comments => {:type => 'VerySpecialComment'}) }
   scope :with_post, ->(post_id) { joins(:comments).where(:comments => { :post_id => post_id }) }
 
+  scope :with_comments, -> { preload(:comments) }
+  scope :with_tags, -> { preload(:taggings) }
+
   has_many   :comments do
     def find_most_recent
       order("id DESC").first