From 7d699dad334996838dd529e4b75e1648692d56f8 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 8 Jul 2019 09:26:03 +0900 Subject: Should `Regexp.escape` quoted table name in regex It is for agnostic test case, since quoted table name may include `.` for all adapters, and `[` / `]` for sqlserver adapter. --- .../test/cases/adapters/mysql2/annotate_test.rb | 37 ----------------- .../cases/adapters/postgresql/annotate_test.rb | 37 ----------------- .../test/cases/adapters/sqlite3/annotate_test.rb | 37 ----------------- activerecord/test/cases/annotate_test.rb | 46 ++++++++++++++++++++++ activerecord/test/cases/batches_test.rb | 2 +- activerecord/test/cases/finder_test.rb | 3 +- activerecord/test/cases/inheritance_test.rb | 4 +- 7 files changed, 51 insertions(+), 115 deletions(-) delete mode 100644 activerecord/test/cases/adapters/mysql2/annotate_test.rb delete mode 100644 activerecord/test/cases/adapters/postgresql/annotate_test.rb delete mode 100644 activerecord/test/cases/adapters/sqlite3/annotate_test.rb create mode 100644 activerecord/test/cases/annotate_test.rb (limited to 'activerecord') diff --git a/activerecord/test/cases/adapters/mysql2/annotate_test.rb b/activerecord/test/cases/adapters/mysql2/annotate_test.rb deleted file mode 100644 index b512540073..0000000000 --- a/activerecord/test/cases/adapters/mysql2/annotate_test.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -require "cases/helper" -require "models/post" - -class Mysql2AnnotateTest < ActiveRecord::Mysql2TestCase - fixtures :posts - - def test_annotate_wraps_content_in_an_inline_comment - assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* foo \*/}) do - posts = Post.select(:id).annotate("foo") - assert posts.first - end - end - - def test_annotate_is_sanitized - assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* foo \*/}) do - posts = Post.select(:id).annotate("*/foo/*") - assert posts.first - end - - assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* foo \*/}) do - posts = Post.select(:id).annotate("**//foo//**") - assert posts.first - end - - assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* foo \*/ /\* bar \*/}) do - posts = Post.select(:id).annotate("*/foo/*").annotate("*/bar") - assert posts.first - end - - assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* \+ MAX_EXECUTION_TIME\(1\) \*/}) do - posts = Post.select(:id).annotate("+ MAX_EXECUTION_TIME(1)") - assert posts.first - end - end -end diff --git a/activerecord/test/cases/adapters/postgresql/annotate_test.rb b/activerecord/test/cases/adapters/postgresql/annotate_test.rb deleted file mode 100644 index 42a2861511..0000000000 --- a/activerecord/test/cases/adapters/postgresql/annotate_test.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -require "cases/helper" -require "models/post" - -class PostgresqlAnnotateTest < ActiveRecord::PostgreSQLTestCase - fixtures :posts - - def test_annotate_wraps_content_in_an_inline_comment - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do - posts = Post.select(:id).annotate("foo") - assert posts.first - end - end - - def test_annotate_is_sanitized - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do - posts = Post.select(:id).annotate("*/foo/*") - assert posts.first - end - - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do - posts = Post.select(:id).annotate("**//foo//**") - assert posts.first - end - - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/ /\* bar \*/}) do - posts = Post.select(:id).annotate("*/foo/*").annotate("*/bar") - assert posts.first - end - - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* \+ MAX_EXECUTION_TIME\(1\) \*/}) do - posts = Post.select(:id).annotate("+ MAX_EXECUTION_TIME(1)") - assert posts.first - end - end -end diff --git a/activerecord/test/cases/adapters/sqlite3/annotate_test.rb b/activerecord/test/cases/adapters/sqlite3/annotate_test.rb deleted file mode 100644 index 6567a5eca3..0000000000 --- a/activerecord/test/cases/adapters/sqlite3/annotate_test.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -require "cases/helper" -require "models/post" - -class SQLite3AnnotateTest < ActiveRecord::SQLite3TestCase - fixtures :posts - - def test_annotate_wraps_content_in_an_inline_comment - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do - posts = Post.select(:id).annotate("foo") - assert posts.first - end - end - - def test_annotate_is_sanitized - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do - posts = Post.select(:id).annotate("*/foo/*") - assert posts.first - end - - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do - posts = Post.select(:id).annotate("**//foo//**") - assert posts.first - end - - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/ /\* bar \*/}) do - posts = Post.select(:id).annotate("*/foo/*").annotate("*/bar") - assert posts.first - end - - assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* \+ MAX_EXECUTION_TIME\(1\) \*/}) do - posts = Post.select(:id).annotate("+ MAX_EXECUTION_TIME(1)") - assert posts.first - end - end -end diff --git a/activerecord/test/cases/annotate_test.rb b/activerecord/test/cases/annotate_test.rb new file mode 100644 index 0000000000..4d71d28f83 --- /dev/null +++ b/activerecord/test/cases/annotate_test.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require "cases/helper" +require "models/post" + +class AnnotateTest < ActiveRecord::TestCase + fixtures :posts + + def test_annotate_wraps_content_in_an_inline_comment + quoted_posts_id, quoted_posts = regexp_escape_table_name("posts.id"), regexp_escape_table_name("posts") + + assert_sql(%r{\ASELECT #{quoted_posts_id} FROM #{quoted_posts} /\* foo \*/}i) do + posts = Post.select(:id).annotate("foo") + assert posts.first + end + end + + def test_annotate_is_sanitized + quoted_posts_id, quoted_posts = regexp_escape_table_name("posts.id"), regexp_escape_table_name("posts") + + assert_sql(%r{\ASELECT #{quoted_posts_id} FROM #{quoted_posts} /\* foo \*/}i) do + posts = Post.select(:id).annotate("*/foo/*") + assert posts.first + end + + assert_sql(%r{\ASELECT #{quoted_posts_id} FROM #{quoted_posts} /\* foo \*/}i) do + posts = Post.select(:id).annotate("**//foo//**") + assert posts.first + end + + assert_sql(%r{\ASELECT #{quoted_posts_id} FROM #{quoted_posts} /\* foo \*/ /\* bar \*/}i) do + posts = Post.select(:id).annotate("*/foo/*").annotate("*/bar") + assert posts.first + end + + assert_sql(%r{\ASELECT #{quoted_posts_id} FROM #{quoted_posts} /\* \+ MAX_EXECUTION_TIME\(1\) \*/}i) do + posts = Post.select(:id).annotate("+ MAX_EXECUTION_TIME(1)") + assert posts.first + end + end + + private + def regexp_escape_table_name(name) + Regexp.escape(Post.connection.quote_table_name(name)) + end +end diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index cf6e280898..0d0bf39f79 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -146,7 +146,7 @@ class EachTest < ActiveRecord::TestCase def test_find_in_batches_should_quote_batch_order c = Post.connection - assert_sql(/ORDER BY #{c.quote_table_name('posts')}\.#{c.quote_column_name('id')}/) do + assert_sql(/ORDER BY #{Regexp.escape(c.quote_table_name("posts.id"))}/i) do Post.find_in_batches(batch_size: 1) do |batch| assert_kind_of Array, batch assert_kind_of Post, batch.first diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 3aa610f86b..3752fd42e3 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -245,7 +245,8 @@ class FinderTest < ActiveRecord::TestCase end def test_exists_does_not_select_columns_without_alias - assert_sql(/SELECT\W+1 AS one FROM ["`]topics["`]/i) do + c = Topic.connection + assert_sql(/SELECT 1 AS one FROM #{Regexp.escape(c.quote_table_name("topics"))}/i) do Topic.exists? end end diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index 629167e9ed..01e4878c3f 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -471,9 +471,9 @@ class InheritanceTest < ActiveRecord::TestCase end def test_eager_load_belongs_to_primary_key_quoting - con = Account.connection + c = Account.connection bind_param = Arel::Nodes::BindParam.new(nil) - assert_sql(/#{con.quote_table_name('companies')}\.#{con.quote_column_name('id')} = (?:#{Regexp.quote(bind_param.to_sql)}|1)/) do + assert_sql(/#{Regexp.escape(c.quote_table_name("companies.id"))} = (?:#{Regexp.escape(bind_param.to_sql)}|1)/i) do Account.all.merge!(includes: :firm).find(1) end end -- cgit v1.2.3