From ea76e9a3126998578d683783483aa695cb6b657e Mon Sep 17 00:00:00 2001
From: Michal Zima <xhire@mujmalysvet.cz>
Date: Tue, 7 Aug 2012 13:39:21 +0200
Subject: [PATCH] Length validation handles correctly nil. Fix #7180

When nil or empty string are not allowed, they are not valid.
---
 activemodel/CHANGELOG.md                      |  4 ++
 .../lib/active_model/validations/length.rb    | 16 +++++++-
 .../validations/length_validation_test.rb     | 39 +++++++++++++++++++
 3 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md
index 133bb558a95..321320cdc20 100644
--- a/activemodel/CHANGELOG.md
+++ b/activemodel/CHANGELOG.md
@@ -1,5 +1,9 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Fixed length validator to correctly handle nil values. Fixes #7180.
+
+    *Michal Zima*
+
 *   Use BCrypt's MIN_COST in the test environment for speedier tests when using `has_secure_pasword`.
 
     *Brian Cardarella + Jeremy Kemper + Trevor Turk*
diff --git a/activemodel/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb
index 70ef589cd75..675fb5f1e50 100644
--- a/activemodel/lib/active_model/validations/length.rb
+++ b/activemodel/lib/active_model/validations/length.rb
@@ -14,6 +14,10 @@ module ActiveModel
           options[:minimum], options[:maximum] = range.min, range.max
         end
 
+        if options[:allow_blank] == false && options[:minimum].nil? && options[:is].nil?
+          options[:minimum] = 1
+        end
+
         super
       end
 
@@ -40,7 +44,10 @@ module ActiveModel
 
         CHECKS.each do |key, validity_check|
           next unless check_value = options[key]
-          next if value_length.send(validity_check, check_value)
+
+          if !value.nil? || skip_nil_check?(key)
+            next if value_length.send(validity_check, check_value)
+          end
 
           errors_options[:count] = check_value
 
@@ -58,6 +65,10 @@ module ActiveModel
           options[:tokenizer].call(value)
         end || value
       end
+
+      def skip_nil_check?(key)
+        key == :maximum && options[:allow_nil].nil? && options[:allow_blank].nil?
+      end
     end
 
     module HelperMethods
@@ -79,7 +90,8 @@ module ActiveModel
       #
       # Configuration options:
       # * <tt>:minimum</tt> - The minimum size of the attribute.
-      # * <tt>:maximum</tt> - The maximum size of the attribute.
+      # * <tt>:maximum</tt> - The maximum size of the attribute. Allows +nil+ by
+      #   default if not used with :minimum.
       # * <tt>:is</tt> - The exact size of the attribute.
       # * <tt>:within</tt> - A range specifying the minimum and maximum size of
       #   the attribute.
diff --git a/activemodel/test/cases/validations/length_validation_test.rb b/activemodel/test/cases/validations/length_validation_test.rb
index 113bfd63372..1a40ca8efc2 100644
--- a/activemodel/test/cases/validations/length_validation_test.rb
+++ b/activemodel/test/cases/validations/length_validation_test.rb
@@ -375,4 +375,43 @@ class LengthValidationTest < ActiveModel::TestCase
     t.author_name = "A very long author name that should still be valid." * 100
     assert t.valid?
   end
+
+  def test_validates_length_of_using_maximum_should_not_allow_nil_when_nil_not_allowed
+    Topic.validates_length_of :title, :maximum => 10, :allow_nil => false
+    t = Topic.new
+    assert t.invalid?
+  end
+
+  def test_validates_length_of_using_maximum_should_not_allow_nil_and_empty_string_when_blank_not_allowed
+    Topic.validates_length_of :title, :maximum => 10, :allow_blank => false
+    t = Topic.new
+    assert t.invalid?
+
+    t.title = ""
+    assert t.invalid?
+  end
+
+  def test_validates_length_of_using_both_minimum_and_maximum_should_not_allow_nil
+    Topic.validates_length_of :title, :minimum => 5, :maximum => 10
+    t = Topic.new
+    assert t.invalid?
+  end
+
+  def test_validates_length_of_using_minimum_0_should_not_allow_nil
+    Topic.validates_length_of :title, :minimum => 0
+    t = Topic.new
+    assert t.invalid?
+
+    t.title = ""
+    assert t.valid?
+  end
+
+  def test_validates_length_of_using_is_0_should_not_allow_nil
+    Topic.validates_length_of :title, :is => 0
+    t = Topic.new
+    assert t.invalid?
+
+    t.title = ""
+    assert t.valid?
+  end
 end