Merge pull request #42444 from OuYangJinTing/fix-ar-sanitization

Improve disallow_raw_sql error msg and fix typo

activerecord/lib/active_record/sanitization.rb

@@ -54,7 +54,7 @@ def sanitize_sql_for_assignment(assignments, default_table_name = table_name)
       # Accepts an array, or string of SQL conditions and sanitizes
       # them into a valid SQL fragment for an ORDER clause.
       #
-      #   sanitize_sql_for_order(["field(id, ?)", [1,3,2]])
+      #   sanitize_sql_for_order([Arel.sql("field(id, ?)"), [1,3,2]])
       #   # => "field(id, 1,3,2)"
       #
       #   sanitize_sql_for_order("id ASC")
@@ -143,8 +143,12 @@ def disallow_raw_sql!(args, permit: connection.column_name_matcher) # :nodoc:
 
         if unexpected
           raise(ActiveRecord::UnknownAttributeReference,
-            "Query method called with non-attribute argument(s): " +
-            unexpected.map(&:inspect).join(", ")
+            "Dangerous query method (method whose arguments are used as raw " \
+            "SQL) called with non-attribute argument(s): " \
+            "#{unexpected.map(&:inspect).join(", ")}." \
+            "This method should not be called with user-provided values, such as request " \
+            "parameters or model attributes. Known-safe values can be passed " \
+            "by wrapping them in Arel.sql()."
           )
         end
       end

activerecord/test/cases/sanitize_test.rb

@@ -95,6 +95,14 @@ def self.search_as_method(term)
     end
   end
 
+  def test_disallow_raw_sql_with_unknown_attribute_string
+    assert_raise(ActiveRecord::UnknownAttributeReference) { Binary.disallow_raw_sql!(["field(id, ?)"]) }
+  end
+
+  def test_disallow_raw_sql_with_unknown_attribute_sql_literal
+    assert_nothing_raised { Binary.disallow_raw_sql!([Arel.sql("field(id, ?)")]) }
+  end
+
   def test_bind_arity
     assert_nothing_raised                                { bind "" }
     assert_raise(ActiveRecord::PreparedStatementInvalid) { bind "", 1 }

activerecord/test/cases/unsafe_raw_sql_test.rb

@@ -171,7 +171,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
       Post.order("REPLACE(title, 'misc', 'zzzz')")
     end
 
-    assert_match(/Query method called with non-attribute argument\(s\):/, e.message)
+    assert_match(/Dangerous query method \(method whose arguments are used as raw SQL\) called with non-attribute argument\(s\):/, e.message)
   end
 
   test "pluck: allows string column name" do
@@ -269,6 +269,6 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
       Post.includes(:comments).pluck(:title, "REPLACE(title, 'misc', 'zzzz')")
     end
 
-    assert_match(/Query method called with non-attribute argument\(s\):/, e.message)
+    assert_match(/Dangerous query method \(method whose arguments are used as raw SQL\) called with non-attribute argument\(s\):/, e.message)
   end
 end