mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
push order arg checks down to allow for binds
This commit is contained in:
parent
b76cc29865
commit
8ef71ac4a1
3 changed files with 47 additions and 28 deletions
|
@ -297,11 +297,6 @@ module ActiveRecord
|
|||
|
||||
# Same as #order but operates on relation in-place instead of copying.
|
||||
def order!(*args) # :nodoc:
|
||||
@klass.enforce_raw_sql_whitelist(
|
||||
column_names_from_order_arguments(args),
|
||||
whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST
|
||||
)
|
||||
|
||||
preprocess_order_args(args)
|
||||
|
||||
self.order_values += args
|
||||
|
@ -324,11 +319,6 @@ module ActiveRecord
|
|||
|
||||
# Same as #reorder but operates on relation in-place instead of copying.
|
||||
def reorder!(*args) # :nodoc:
|
||||
@klass.enforce_raw_sql_whitelist(
|
||||
column_names_from_order_arguments(args),
|
||||
whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST
|
||||
)
|
||||
|
||||
preprocess_order_args(args)
|
||||
|
||||
self.reordering_value = true
|
||||
|
@ -928,23 +918,6 @@ module ActiveRecord
|
|||
|
||||
private
|
||||
|
||||
# Extract column names from arguments passed to #order or #reorder.
|
||||
def column_names_from_order_arguments(args)
|
||||
args.flat_map do |arg|
|
||||
case arg
|
||||
when Hash
|
||||
# Tag.order(id: :desc)
|
||||
arg.keys
|
||||
when Array
|
||||
# Tag.order([Arel.sql("field(id, ?)"), [1, 3, 2]])
|
||||
arg.flatten
|
||||
else
|
||||
# Tag.order(:id)
|
||||
arg
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def assert_mutability!
|
||||
raise ImmutableRelation if @loaded
|
||||
raise ImmutableRelation if defined?(@arel) && @arel
|
||||
|
@ -1146,6 +1119,12 @@ module ActiveRecord
|
|||
klass.send(:sanitize_sql_for_order, arg)
|
||||
end
|
||||
order_args.flatten!
|
||||
|
||||
@klass.enforce_raw_sql_whitelist(
|
||||
order_args.flat_map { |a| a.is_a?(Hash) ? a.keys : a },
|
||||
whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST
|
||||
)
|
||||
|
||||
validate_order_args(order_args)
|
||||
|
||||
references = order_args.grep(String)
|
||||
|
|
|
@ -63,13 +63,17 @@ module ActiveRecord
|
|||
# # => "id ASC"
|
||||
def sanitize_sql_for_order(condition) # :doc:
|
||||
if condition.is_a?(Array) && condition.first.to_s.include?("?")
|
||||
enforce_raw_sql_whitelist([condition.first],
|
||||
whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST
|
||||
)
|
||||
|
||||
# Ensure we aren't dealing with a subclass of String that might
|
||||
# override methods we use (eg. Arel::Nodes::SqlLiteral).
|
||||
if condition.first.kind_of?(String) && !condition.first.instance_of?(String)
|
||||
condition = [String.new(condition.first), *condition[1..-1]]
|
||||
end
|
||||
|
||||
sanitize_sql_array(condition)
|
||||
Arel.sql(sanitize_sql_array(condition))
|
||||
else
|
||||
condition
|
||||
end
|
||||
|
|
|
@ -138,6 +138,42 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
|
|||
assert_equal ids_depr, ids_disabled
|
||||
end
|
||||
|
||||
test "order: allows Arel.sql with binds" do
|
||||
ids_expected = Post.order(Arel.sql('INSTR(title, "comments"), id')).pluck(:id)
|
||||
|
||||
ids_depr = with_unsafe_raw_sql_deprecated { Post.order([Arel.sql("INSTR(title, ?), id"), "comments"]).pluck(:id) }
|
||||
ids_disabled = with_unsafe_raw_sql_disabled { Post.order([Arel.sql("INSTR(title, ?), id"), "comments"]).pluck(:id) }
|
||||
|
||||
assert_equal ids_expected, ids_depr
|
||||
assert_equal ids_expected, ids_disabled
|
||||
end
|
||||
|
||||
test "order: disallows invalid bind statement" do
|
||||
with_unsafe_raw_sql_disabled do
|
||||
assert_raises(ActiveRecord::UnknownAttributeReference) do
|
||||
Post.order(["INSTR(title, ?), id", "comments"]).pluck(:id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
test "order: disallows invalid Array arguments" do
|
||||
with_unsafe_raw_sql_disabled do
|
||||
assert_raises(ActiveRecord::UnknownAttributeReference) do
|
||||
Post.order(["author_id", "length(title)"]).pluck(:id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
test "order: allows valid Array arguments" do
|
||||
ids_expected = Post.order(Arel.sql("author_id, length(title)")).pluck(:id)
|
||||
|
||||
ids_depr = with_unsafe_raw_sql_deprecated { Post.order(["author_id", Arel.sql("length(title)")]).pluck(:id) }
|
||||
ids_disabled = with_unsafe_raw_sql_disabled { Post.order(["author_id", Arel.sql("length(title)")]).pluck(:id) }
|
||||
|
||||
assert_equal ids_expected, ids_depr
|
||||
assert_equal ids_expected, ids_disabled
|
||||
end
|
||||
|
||||
test "order: logs deprecation warning for unrecognized column" do
|
||||
with_unsafe_raw_sql_deprecated do
|
||||
ActiveSupport::Deprecation.expects(:warn).with do |msg|
|
||||
|
|
Loading…
Reference in a new issue