Navigation Menu

Skip to content

Commit

Permalink
whitelist NULLS { FIRST | LAST } in order clauses
Browse files Browse the repository at this point in the history
  • Loading branch information
fxn committed Mar 6, 2018
1 parent 718e4c6 commit e4a921a
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
9 changes: 8 additions & 1 deletion activerecord/lib/active_record/attribute_methods.rb
Expand Up @@ -177,7 +177,14 @@ def attribute_names
# "#{table_name}.#{column_name} #{direction}" # "#{table_name}.#{column_name} #{direction}"
# "#{column_name}" # "#{column_name}"
# "#{column_name} #{direction}" # "#{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: def enforce_raw_sql_whitelist(args, whitelist: COLUMN_NAME_WHITELIST) # :nodoc:
unexpected = args.reject do |arg| unexpected = args.reject do |arg|
Expand Down
20 changes: 20 additions & 0 deletions activerecord/test/cases/unsafe_raw_sql_test.rb
Expand Up @@ -107,6 +107,26 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
assert_equal ids_expected, ids_disabled assert_equal ids_expected, ids_disabled
end 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 test "order: disallows invalid column name" do
with_unsafe_raw_sql_disabled do with_unsafe_raw_sql_disabled do
assert_raises(ActiveRecord::UnknownAttributeReference) do assert_raises(ActiveRecord::UnknownAttributeReference) do
Expand Down

0 comments on commit e4a921a

Please sign in to comment.