whitelist NULLS { FIRST | LAST } in order clauses

2 files changed, 28 insertions, 1 deletions

diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index 0ca0e5dcdb..122140eb55 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -177,7 +177,14 @@ module ActiveRecord
       #   "#{table_name}.#{column_name} #{direction}"
       #   "#{column_name}"
       #   "#{column_name} #{direction}"
-      COLUMN_NAME_ORDER_WHITELIST = /\A(?:\w+\.)?\w+(?:\s+asc|\s+desc)?\z/i
+      COLUMN_NAME_ORDER_WHITELIST = /
+        \A
+        (?:\w+\.)?
+        \w+
+        (?:\s+asc|\s+desc)?
+        (?:\s+nulls\s+(?:first|last))?
+        \z
+      /ix
 
       def enforce_raw_sql_whitelist(args, whitelist: COLUMN_NAME_WHITELIST) # :nodoc:
         unexpected = args.reject do |arg|
diff --git a/activerecord/test/cases/unsafe_raw_sql_test.rb b/activerecord/test/cases/unsafe_raw_sql_test.rb
index 72d4997d0b..d5d8f2a09a 100644
--- a/activerecord/test/cases/unsafe_raw_sql_test.rb
+++ b/activerecord/test/cases/unsafe_raw_sql_test.rb
@@ -107,6 +107,26 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
     assert_equal ids_expected, ids_disabled
   end
 
+  test "order: allows NULLS FIRST and NULLS LAST too" do
+    raise "precondition failed" if Post.count < 2
+
+    # Ensure there are NULL and non-NULL post types.
+    Post.first.update_column(:type, nil)
+    Post.last.update_column(:type, "Programming")
+
+    ["asc", "desc", ""].each do |direction|
+      %w(first last).each do |position|
+        ids_expected = Post.order(Arel.sql("type #{direction} nulls #{position}")).pluck(:id)
+
+        ids_depr     = with_unsafe_raw_sql_deprecated { Post.order("type #{direction} nulls #{position}").pluck(:id) }
+        ids_disabled = with_unsafe_raw_sql_disabled   { Post.order("type #{direction} nulls #{position}").pluck(:id) }
+
+        assert_equal ids_expected, ids_depr
+        assert_equal ids_expected, ids_disabled
+      end
+    end
+  end if current_adapter?(:PostgreSQLAdapter)
+
   test "order: disallows invalid column name" do
     with_unsafe_raw_sql_disabled do
       assert_raises(ActiveRecord::UnknownAttributeReference) do