diff options
7 files changed, 67 insertions, 42 deletions
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index e34f4f745f..1b6ba8ce97 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -158,7 +158,10 @@ module ActiveRecord COLUMN_NAME = / \A ( - (?:\w+\.)?\w+ + (?: + # table_name.column_name | function(one or no argument) + ((?:\w+\.)?\w+) | \w+\((?:|\g<2>)\) + ) (?:(?:\s+AS)?\s+\w+)? ) (?:\s*,\s*\g<1>)* @@ -179,7 +182,10 @@ module ActiveRecord COLUMN_NAME_WITH_ORDER = / \A ( - (?:\w+\.)?\w+ + (?: + # table_name.column_name | function(one or no argument) + ((?:\w+\.)?\w+) | \w+\((?:|\g<2>)\) + ) (?:\s+ASC|\s+DESC)? (?:\s+NULLS\s+(?:FIRST|LAST))? ) diff --git a/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb b/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb index a0829b1115..dfed5471f4 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb @@ -43,7 +43,10 @@ module ActiveRecord COLUMN_NAME = / \A ( - (?:\w+\.|`\w+`\.)?(?:\w+|`\w+`) + (?: + # `table_name`.`column_name` | function(one or no argument) + ((?:\w+\.|`\w+`\.)?(?:\w+|`\w+`)) | \w+\((?:|\g<2>)\) + ) (?:(?:\s+AS)?\s+(?:\w+|`\w+`))? ) (?:\s*,\s*\g<1>)* @@ -53,7 +56,10 @@ module ActiveRecord COLUMN_NAME_WITH_ORDER = / \A ( - (?:\w+\.|`\w+`\.)?(?:\w+|`\w+`) + (?: + # `table_name`.`column_name` | function(one or no argument) + ((?:\w+\.|`\w+`\.)?(?:\w+|`\w+`)) | \w+\((?:|\g<2>)\) + ) (?:\s+ASC|\s+DESC)? ) (?:\s*,\s*\g<1>)* diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index d18c5c5c12..0c800dca83 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -89,7 +89,10 @@ module ActiveRecord COLUMN_NAME = / \A ( - (?:\w+\.|"\w+"\.)?(?:\w+|"\w+")(?:::\w+)? + (?: + # "table_name"."column_name"::type_name | function(one or no argument)::type_name + ((?:\w+\.|"\w+"\.)?(?:\w+|"\w+")(?:::\w+)?) | \w+\((?:|\g<2>)\)(?:::\w+)? + ) (?:(?:\s+AS)?\s+(?:\w+|"\w+"))? ) (?:\s*,\s*\g<1>)* @@ -99,7 +102,10 @@ module ActiveRecord COLUMN_NAME_WITH_ORDER = / \A ( - (?:\w+\.|"\w+"\.)?(?:\w+|"\w+")(?:::\w+)? + (?: + # "table_name"."column_name"::type_name | function(one or no argument)::type_name + ((?:\w+\.|"\w+"\.)?(?:\w+|"\w+")(?:::\w+)?) | \w+\((?:|\g<2>)\)(?:::\w+)? + ) (?:\s+ASC|\s+DESC)? (?:\s+NULLS\s+(?:FIRST|LAST))? ) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/quoting.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/quoting.rb index 5d6932e4ca..54808de714 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/quoting.rb @@ -56,7 +56,10 @@ module ActiveRecord COLUMN_NAME = / \A ( - (?:\w+\.|"\w+"\.)?(?:\w+|"\w+") + (?: + # "table_name"."column_name" | function(one or no argument) + ((?:\w+\.|"\w+"\.)?(?:\w+|"\w+")) | \w+\((?:|\g<2>)\) + ) (?:(?:\s+AS)?\s+(?:\w+|"\w+"))? ) (?:\s*,\s*\g<1>)* @@ -66,7 +69,10 @@ module ActiveRecord COLUMN_NAME_WITH_ORDER = / \A ( - (?:\w+\.|"\w+"\.)?(?:\w+|"\w+") + (?: + # "table_name"."column_name" | function(one or no argument) + ((?:\w+\.|"\w+"\.)?(?:\w+|"\w+")) | \w+\((?:|\g<2>)\) + ) (?:\s+ASC|\s+DESC)? ) (?:\s*,\s*\g<1>)* diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 38723b6c19..cb46f9e053 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -968,14 +968,14 @@ class EagerAssociationTest < ActiveRecord::TestCase posts(:thinking, :sti_comments), Post.all.merge!( includes: [:author, :comments], where: { "authors.name" => "David" }, - order: Arel.sql("UPPER(posts.title)"), limit: 2, offset: 1 + order: "UPPER(posts.title)", limit: 2, offset: 1 ).to_a ) assert_equal( posts(:sti_post_and_comments, :sti_comments), Post.all.merge!( includes: [:author, :comments], where: { "authors.name" => "David" }, - order: Arel.sql("UPPER(posts.title) DESC"), limit: 2, offset: 1 + order: "UPPER(posts.title) DESC", limit: 2, offset: 1 ).to_a ) end @@ -985,14 +985,14 @@ class EagerAssociationTest < ActiveRecord::TestCase posts(:thinking, :sti_comments), Post.all.merge!( includes: [:author, :comments], where: { "authors.name" => "David" }, - order: [Arel.sql("UPPER(posts.title)"), "posts.id"], limit: 2, offset: 1 + order: ["UPPER(posts.title)", "posts.id"], limit: 2, offset: 1 ).to_a ) assert_equal( posts(:sti_post_and_comments, :sti_comments), Post.all.merge!( includes: [:author, :comments], where: { "authors.name" => "David" }, - order: [Arel.sql("UPPER(posts.title) DESC"), "posts.id"], limit: 2, offset: 1 + order: ["UPPER(posts.title) DESC", "posts.id"], limit: 2, offset: 1 ).to_a ) end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 732bca4bf5..a5c162af33 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -298,7 +298,7 @@ class RelationTest < ActiveRecord::TestCase end def test_reverse_order_with_function - topics = Topic.order(Arel.sql("length(title)")).reverse_order + topics = Topic.order("length(title)").reverse_order assert_equal topics(:second).title, topics.first.title end @@ -1696,7 +1696,7 @@ class RelationTest < ActiveRecord::TestCase scope = Post.order("comments.body asc") assert_equal ["comments"], scope.references_values - scope = Post.order(Arel.sql("foo(comments.body)")) + scope = Post.order("foo(comments.body)") assert_equal [], scope.references_values end @@ -1721,7 +1721,7 @@ class RelationTest < ActiveRecord::TestCase scope = Post.reorder("comments.body asc") assert_equal %w(comments), scope.references_values - scope = Post.reorder(Arel.sql("foo(comments.body)")) + scope = Post.reorder("foo(comments.body)") assert_equal [], scope.references_values end diff --git a/activerecord/test/cases/unsafe_raw_sql_test.rb b/activerecord/test/cases/unsafe_raw_sql_test.rb index d4cb51afba..87edb163f2 100644 --- a/activerecord/test/cases/unsafe_raw_sql_test.rb +++ b/activerecord/test/cases/unsafe_raw_sql_test.rb @@ -141,7 +141,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase test "order: disallows invalid column name" do with_unsafe_raw_sql_disabled do assert_raises(ActiveRecord::UnknownAttributeReference) do - Post.order("len(title) asc").pluck(:id) + Post.order("REPLACE(title, 'misc', 'zzzz') asc").pluck(:id) end end end @@ -157,7 +157,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase test "order: disallows invalid column with direction" do with_unsafe_raw_sql_disabled do assert_raises(ActiveRecord::UnknownAttributeReference) do - Post.order("len(title)" => :asc).pluck(:id) + Post.order("REPLACE(title, 'misc', 'zzzz')" => :asc).pluck(:id) end end end @@ -190,7 +190,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase 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) + Post.order(["author_id", "REPLACE(title, 'misc', 'zzzz')"]).pluck(:id) end end end @@ -198,8 +198,8 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase 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) } + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(["author_id", "length(title)"]).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(["author_id", "length(title)"]).pluck(:id) } assert_equal ids_expected, ids_depr assert_equal ids_expected, ids_disabled @@ -208,7 +208,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase test "order: logs deprecation warning for unrecognized column" do with_unsafe_raw_sql_deprecated do assert_deprecated(/Dangerous query method/) do - Post.order("length(title)") + Post.order("REPLACE(title, 'misc', 'zzzz')") end end end @@ -223,11 +223,11 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase assert_equal titles_expected, titles_disabled end - test "pluck: allows string column name with alias" do - titles_expected = Post.pluck(Arel.sql("title")) + test "pluck: allows string column name with function and alias" do + titles_expected = Post.pluck(Arel.sql("UPPER(title)")) - titles_depr = with_unsafe_raw_sql_deprecated { Post.pluck("title AS posts_title") } - titles_disabled = with_unsafe_raw_sql_disabled { Post.pluck("title AS posts_title") } + titles_depr = with_unsafe_raw_sql_deprecated { Post.pluck("UPPER(title) AS title") } + titles_disabled = with_unsafe_raw_sql_disabled { Post.pluck("UPPER(title) AS title") } assert_equal titles_expected, titles_depr assert_equal titles_expected, titles_disabled @@ -297,7 +297,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase test "pluck: disallows invalid column name" do with_unsafe_raw_sql_disabled do assert_raises(ActiveRecord::UnknownAttributeReference) do - Post.pluck("length(title)") + Post.pluck("REPLACE(title, 'misc', 'zzzz')") end end end @@ -305,7 +305,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase test "pluck: disallows invalid column name amongst valid names" do with_unsafe_raw_sql_disabled do assert_raises(ActiveRecord::UnknownAttributeReference) do - Post.pluck(:title, "length(title)") + Post.pluck(:title, "REPLACE(title, 'misc', 'zzzz')") end end end @@ -313,7 +313,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase test "pluck: disallows invalid column names with includes" do with_unsafe_raw_sql_disabled do assert_raises(ActiveRecord::UnknownAttributeReference) do - Post.includes(:comments).pluck(:title, "length(title)") + Post.includes(:comments).pluck(:title, "REPLACE(title, 'misc', 'zzzz')") end end end @@ -328,24 +328,25 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase test "pluck: logs deprecation warning" do with_unsafe_raw_sql_deprecated do assert_deprecated(/Dangerous query method/) do - Post.includes(:comments).pluck(:title, "length(title)") + Post.includes(:comments).pluck(:title, "REPLACE(title, 'misc', 'zzzz')") end end end - def with_unsafe_raw_sql_disabled(&blk) - with_config(:disabled, &blk) - end + private + def with_unsafe_raw_sql_disabled(&block) + with_config(:disabled, &block) + end - def with_unsafe_raw_sql_deprecated(&blk) - with_config(:deprecated, &blk) - end + def with_unsafe_raw_sql_deprecated(&block) + with_config(:deprecated, &block) + end - def with_config(new_value, &blk) - old_value = ActiveRecord::Base.allow_unsafe_raw_sql - ActiveRecord::Base.allow_unsafe_raw_sql = new_value - blk.call - ensure - ActiveRecord::Base.allow_unsafe_raw_sql = old_value - end + def with_config(new_value, &block) + old_value = ActiveRecord::Base.allow_unsafe_raw_sql + ActiveRecord::Base.allow_unsafe_raw_sql = new_value + yield + ensure + ActiveRecord::Base.allow_unsafe_raw_sql = old_value + end end |