From 7d699dad334996838dd529e4b75e1648692d56f8 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Mon, 8 Jul 2019 09:26:03 +0900
Subject: [PATCH] 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.
---
 .../cases/adapters/mysql2/annotate_test.rb    | 37 ---------------
 .../adapters/postgresql/annotate_test.rb      | 37 ---------------
 .../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

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 @@ def test_find_in_batches_shouldnt_execute_query_unless_needed
 
   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 @@ def test_exists_passing_active_record_object_is_not_permitted
   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 @@ def test_alt_eager_loading
   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
-- 
2.30.2