From 0d8626ad3f1bac3b814d554147ff7926c0380fad Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Fri, 11 Aug 2017 09:55:54 -0600 Subject: remove :enabled option --- .../lib/active_record/attribute_methods.rb | 2 - activerecord/lib/active_record/core.rb | 5 +- activerecord/test/cases/unsafe_raw_sql_test.rb | 104 ++++++++++----------- 3 files changed, 52 insertions(+), 59 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index e167c4ad6f..5ce3ba7a63 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -168,8 +168,6 @@ module ActiveRecord end def enforce_raw_sql_whitelist(args, whitelist: attribute_names_and_aliases) # :nodoc: - return if allow_unsafe_raw_sql == :enabled - unexpected = args.reject do |arg| whitelist.include?(arg.to_s) || arg.kind_of?(Arel::Node) || arg.is_a?(Arel::Nodes::SqlLiteral) diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index b1e3f71dfe..b97b14644e 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -78,12 +78,11 @@ module ActiveRecord # :singleton-method: # Specify the behavior for unsafe raw query methods. Values are as follows - # enabled - Unsafe raw SQL can be passed to query methods. # deprecated - Warnings are logged when unsafe raw SQL is passed to # query methods. # disabled - Unsafe raw SQL passed to query methods results in - # ArguementError. - mattr_accessor :allow_unsafe_raw_sql, instance_writer: false, default: :enabled + # UnknownAttributeReference exception. + mattr_accessor :allow_unsafe_raw_sql, instance_writer: false, default: :deprecated ## # :singleton-method: diff --git a/activerecord/test/cases/unsafe_raw_sql_test.rb b/activerecord/test/cases/unsafe_raw_sql_test.rb index 53418bb914..74471b0ace 100644 --- a/activerecord/test/cases/unsafe_raw_sql_test.rb +++ b/activerecord/test/cases/unsafe_raw_sql_test.rb @@ -10,75 +10,75 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase test "order: allows string column name" do ids_expected = Post.order(Arel.sql("title")).pluck(:id) - ids_enabled = with_unsafe_raw_sql_enabled { Post.order("title").pluck(:id) } - ids_disabled = with_unsafe_raw_sql_disabled { Post.order("title").pluck(:id) } + ids_depr = with_unsafe_raw_sql_deprecated { Post.order("title").pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order("title").pluck(:id) } - assert_equal ids_expected, ids_enabled + assert_equal ids_expected, ids_depr assert_equal ids_expected, ids_disabled end test "order: allows symbol column name" do ids_expected = Post.order(Arel.sql("title")).pluck(:id) - ids_enabled = with_unsafe_raw_sql_enabled { Post.order(:title).pluck(:id) } - ids_disabled = with_unsafe_raw_sql_disabled { Post.order(:title).pluck(:id) } + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(:title).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(:title).pluck(:id) } - assert_equal ids_expected, ids_enabled + assert_equal ids_expected, ids_depr assert_equal ids_expected, ids_disabled end test "order: allows downcase symbol direction" do ids_expected = Post.order(Arel.sql("title") => Arel.sql("asc")).pluck(:id) - ids_enabled = with_unsafe_raw_sql_enabled { Post.order(title: :asc).pluck(:id) } - ids_disabled = with_unsafe_raw_sql_disabled { Post.order(title: :asc).pluck(:id) } + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(title: :asc).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(title: :asc).pluck(:id) } - assert_equal ids_expected, ids_enabled + assert_equal ids_expected, ids_depr assert_equal ids_expected, ids_disabled end test "order: allows upcase symbol direction" do ids_expected = Post.order(Arel.sql("title") => Arel.sql("ASC")).pluck(:id) - ids_enabled = with_unsafe_raw_sql_enabled { Post.order(title: :ASC).pluck(:id) } - ids_disabled = with_unsafe_raw_sql_disabled { Post.order(title: :ASC).pluck(:id) } + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(title: :ASC).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(title: :ASC).pluck(:id) } - assert_equal ids_expected, ids_enabled + assert_equal ids_expected, ids_depr assert_equal ids_expected, ids_disabled end test "order: allows string direction" do ids_expected = Post.order(Arel.sql("title") => Arel.sql("asc")).pluck(:id) - ids_enabled = with_unsafe_raw_sql_enabled { Post.order(title: "asc").pluck(:id) } - ids_disabled = with_unsafe_raw_sql_disabled { Post.order(title: "asc").pluck(:id) } + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(title: "asc").pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(title: "asc").pluck(:id) } - assert_equal ids_expected, ids_enabled + assert_equal ids_expected, ids_depr assert_equal ids_expected, ids_disabled end test "order: allows multiple columns" do ids_expected = Post.order(Arel.sql("author_id"), Arel.sql("title")).pluck(:id) - ids_enabled = with_unsafe_raw_sql_enabled { Post.order(:author_id, :title).pluck(:id) } - ids_disabled = with_unsafe_raw_sql_disabled { Post.order(:author_id, :title).pluck(:id) } + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(:author_id, :title).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(:author_id, :title).pluck(:id) } - assert_equal ids_expected, ids_enabled + assert_equal ids_expected, ids_depr assert_equal ids_expected, ids_disabled end test "order: allows mixed" do ids_expected = Post.order(Arel.sql("author_id"), Arel.sql("title") => Arel.sql("asc")).pluck(:id) - ids_enabled = with_unsafe_raw_sql_enabled { Post.order(:author_id, title: :asc).pluck(:id) } - ids_disabled = with_unsafe_raw_sql_disabled { Post.order(:author_id, title: :asc).pluck(:id) } + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(:author_id, title: :asc).pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(:author_id, title: :asc).pluck(:id) } - assert_equal ids_expected, ids_enabled + assert_equal ids_expected, ids_depr assert_equal ids_expected, ids_disabled end test "order: disallows invalid column name" do - with_unsafe_raw_sql_disabled do + with_unsafe_raw_sql_disabled do assert_raises(ActiveRecord::UnknownAttributeReference) do Post.order("title asc").pluck(:id) end @@ -86,7 +86,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase end test "order: disallows invalid direction" do - with_unsafe_raw_sql_disabled do + with_unsafe_raw_sql_disabled do assert_raises(ArgumentError) do Post.order(title: :foo).pluck(:id) end @@ -94,7 +94,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase end test "order: disallows invalid column with direction" do - with_unsafe_raw_sql_disabled do + with_unsafe_raw_sql_disabled do assert_raises(ActiveRecord::UnknownAttributeReference) do Post.order(foo: :asc).pluck(:id) end @@ -102,16 +102,16 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase end test "order: always allows Arel" do - ids_enabled = with_unsafe_raw_sql_enabled { Post.order(Arel.sql("length(title)")).pluck(:title) } - ids_disabled = with_unsafe_raw_sql_disabled { Post.order(Arel.sql("length(title)")).pluck(:title) } + ids_depr = with_unsafe_raw_sql_deprecated { Post.order(Arel.sql("length(title)")).pluck(:title) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order(Arel.sql("length(title)")).pluck(:title) } - assert_equal ids_enabled, ids_disabled + assert_equal ids_depr, 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| - msg =~ /\ADangerous query method used with .*length\(title\)/ + msg =~ /\ADangerous query method/ end Post.order("length(title)") @@ -121,55 +121,55 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase test "pluck: allows string column name" do titles_expected = Post.pluck(Arel.sql("title")) - titles_enabled = with_unsafe_raw_sql_enabled { Post.pluck("title") } - titles_disabled = with_unsafe_raw_sql_disabled { Post.pluck("title") } + titles_depr = with_unsafe_raw_sql_deprecated { Post.pluck("title") } + titles_disabled = with_unsafe_raw_sql_disabled { Post.pluck("title") } - assert_equal titles_expected, titles_enabled + assert_equal titles_expected, titles_depr assert_equal titles_expected, titles_disabled end test "pluck: allows symbol column name" do titles_expected = Post.pluck(Arel.sql("title")) - titles_enabled = with_unsafe_raw_sql_enabled { Post.pluck(:title) } - titles_disabled = with_unsafe_raw_sql_disabled { Post.pluck(:title) } + titles_depr = with_unsafe_raw_sql_deprecated { Post.pluck(:title) } + titles_disabled = with_unsafe_raw_sql_disabled { Post.pluck(:title) } - assert_equal titles_expected, titles_enabled + assert_equal titles_expected, titles_depr assert_equal titles_expected, titles_disabled end test "pluck: allows multiple column names" do values_expected = Post.pluck(Arel.sql("title"), Arel.sql("id")) - values_enabled = with_unsafe_raw_sql_enabled { Post.pluck(:title, :id) } - values_disabled = with_unsafe_raw_sql_disabled { Post.pluck(:title, :id) } + values_depr = with_unsafe_raw_sql_deprecated { Post.pluck(:title, :id) } + values_disabled = with_unsafe_raw_sql_disabled { Post.pluck(:title, :id) } - assert_equal values_expected, values_enabled + assert_equal values_expected, values_depr assert_equal values_expected, values_disabled end test "pluck: allows column names with includes" do values_expected = Post.includes(:comments).pluck(Arel.sql("title"), Arel.sql("id")) - values_enabled = with_unsafe_raw_sql_enabled { Post.includes(:comments).pluck(:title, :id) } - values_disabled = with_unsafe_raw_sql_disabled { Post.includes(:comments).pluck(:title, :id) } + values_depr = with_unsafe_raw_sql_deprecated { Post.includes(:comments).pluck(:title, :id) } + values_disabled = with_unsafe_raw_sql_disabled { Post.includes(:comments).pluck(:title, :id) } - assert_equal values_expected, values_enabled + assert_equal values_expected, values_depr assert_equal values_expected, values_disabled end test "pluck: allows auto-generated attributes" do values_expected = Post.pluck(Arel.sql("tags_count")) - values_enabled = with_unsafe_raw_sql_enabled { Post.pluck(:tags_count) } - values_disabled = with_unsafe_raw_sql_disabled { Post.pluck(:tags_count) } + values_depr = with_unsafe_raw_sql_deprecated { Post.pluck(:tags_count) } + values_disabled = with_unsafe_raw_sql_disabled { Post.pluck(:tags_count) } - assert_equal values_expected, values_enabled + assert_equal values_expected, values_depr assert_equal values_expected, values_disabled end test "pluck: disallows invalid column name" do - with_unsafe_raw_sql_disabled do + with_unsafe_raw_sql_disabled do assert_raises(ActiveRecord::UnknownAttributeReference) do Post.pluck("length(title)") end @@ -177,7 +177,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase end test "pluck: disallows invalid column name amongst valid names" do - with_unsafe_raw_sql_disabled do + with_unsafe_raw_sql_disabled do assert_raises(ActiveRecord::UnknownAttributeReference) do Post.pluck(:title, "length(title)") end @@ -185,7 +185,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase end test "pluck: disallows invalid column names with includes" do - with_unsafe_raw_sql_disabled do + with_unsafe_raw_sql_disabled do assert_raises(ActiveRecord::UnknownAttributeReference) do Post.includes(:comments).pluck(:title, "length(title)") end @@ -193,26 +193,22 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase end test "pluck: always allows Arel" do - values_enabled = with_unsafe_raw_sql_enabled { Post.includes(:comments).pluck(:title, Arel.sql("length(title)")) } - values_disabled = with_unsafe_raw_sql_disabled { Post.includes(:comments).pluck(:title, Arel.sql("length(title)")) } + values_depr = with_unsafe_raw_sql_deprecated { Post.includes(:comments).pluck(:title, Arel.sql("length(title)")) } + values_disabled = with_unsafe_raw_sql_disabled { Post.includes(:comments).pluck(:title, Arel.sql("length(title)")) } - assert_equal values_enabled, values_disabled + assert_equal values_depr, values_disabled end test "pluck: logs deprecation warning" do with_unsafe_raw_sql_deprecated do ActiveSupport::Deprecation.expects(:warn).with do |msg| - msg =~ /\ADangerous query method used with .*length\(title\)/ + msg =~ /\ADangerous query method/ end Post.includes(:comments).pluck(:title, "length(title)") end end - def with_unsafe_raw_sql_enabled(&blk) - with_config(:enabled, &blk) - end - def with_unsafe_raw_sql_disabled(&blk) with_config(:disabled, &blk) end -- cgit v1.2.3