Merge pull request #67 from appoptics/sanitize_AO-10959

 [sql_sanitize] fix bad sanitizing when there is an escaped `'`

Author: Maia Engeli

lib/appoptics_apm/util.rb

@@ -170,7 +170,7 @@ def sanitize_sql(sql)
         return sql unless AppOpticsAPM::Config[:sanitize_sql]
 
         regexp = Regexp.new(AppOpticsAPM::Config[:sanitize_sql_regexp], AppOpticsAPM::Config[:sanitize_sql_opts])
-        sql.gsub(regexp, '?')
+        sql.gsub(/\\\'/,'').gsub(regexp, '?')
       end
 
       ##

lib/rails/generators/appoptics_apm/templates/appoptics_apm_initializer.rb

@@ -75,7 +75,7 @@
   # collect and report query literals to AppOpticsAPM.
   #
   AppOpticsAPM::Config[:sanitize_sql] = true
-  AppOpticsAPM::Config[:sanitize_sql_regexp] = '(\'[\s\S][^\']*\'|\d*\.\d+|\d+|NULL)'
+  AppOpticsAPM::Config[:sanitize_sql_regexp] = '(\'[^\']*\'|\d*\.\d+|\d+|NULL)'
   AppOpticsAPM::Config[:sanitize_sql_opts]   = Regexp::IGNORECASE
 
   #

test/support/config_test.rb

@@ -151,7 +151,7 @@ class ConfigTest
       AppOpticsAPM::Config[:verbose].must_equal false
       AppOpticsAPM::Config[:tracing_mode].must_equal :always
       AppOpticsAPM::Config[:sanitize_sql].must_equal true
-      AppOpticsAPM::Config[:sanitize_sql_regexp].must_equal '(\'[\s\S][^\']*\'|\d*\.\d+|\d+|NULL)'
+      AppOpticsAPM::Config[:sanitize_sql_regexp].must_equal '(\'[^\']*\'|\d*\.\d+|\d+|NULL)'
       AppOpticsAPM::Config[:sanitize_sql_opts].must_equal Regexp::IGNORECASE
 
       AppOpticsAPM::Config[:dnt_regexp].must_equal '\.(jpg|jpeg|gif|png|ico|css|zip|tgz|gz|rar|bz2|pdf|txt|tar|wav|bmp|rtf|js|flv|swf|otf|eot|ttf|woff|woff2|svg|less)(\?.+){0,1}$'

test/support/sql_sanitize_test.rb

@@ -4,14 +4,17 @@
 require 'minitest_helper'
 
 class SQLSanitizeTest < Minitest::Test
+
+  def teardown
+    AppOpticsAPM::Config[:sanitize_sql] = false
+  end
+
   def test_sanitize_sql1
     AppOpticsAPM::Config[:sanitize_sql] = true
 
     sql = "INSERT INTO `queries` (`asdf_id`, `asdf_prices`, `created_at`, `updated_at`, `blue_pill`, `yearly_tax`, `rate`, `steam_id`, `red_pill`, `dimitri`, `origin`) VALUES (19231, 3, 'cat', 'dog', 111.0, 126.0, 116.0, 79.0, 72.0, 73.0, ?, 1, 3, 229.284, ?, ?, 100, ?, 0, 3, 1, ?, NULL, NULL, ?, 4, ?)"
     result = AppOpticsAPM::Util.sanitize_sql(sql)
     result.must_equal "INSERT INTO `queries` (`asdf_id`, `asdf_prices`, `created_at`, `updated_at`, `blue_pill`, `yearly_tax`, `rate`, `steam_id`, `red_pill`, `dimitri`, `origin`) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"
-
-    AppOpticsAPM::Config[:sanitize_sql] = false
   end
 
   def test_sanitize_sql2
@@ -20,8 +23,6 @@ def test_sanitize_sql2
     sql = "SELECT \"game_types\".* FROM \"game_types\" WHERE \"game_types\".\"game_id\" IN (1162)"
     result = AppOpticsAPM::Util.sanitize_sql(sql)
     result.must_equal "SELECT \"game_types\".* FROM \"game_types\" WHERE \"game_types\".\"game_id\" IN (?)"
-
-    AppOpticsAPM::Config[:sanitize_sql] = false
   end
 
   def test_sanitize_sql3
@@ -30,8 +31,6 @@ def test_sanitize_sql3
     sql = "SELECT \"comments\".* FROM \"comments\" WHERE \"comments\".\"commentable_id\" = 2798 AND \"comments\".\"commentable_type\" = 'Video' AND \"comments\".\"parent_id\" IS NULL ORDER BY comments.created_at DESC"
     result = AppOpticsAPM::Util.sanitize_sql(sql)
     result.must_equal "SELECT \"comments\".* FROM \"comments\" WHERE \"comments\".\"commentable_id\" = ? AND \"comments\".\"commentable_type\" = ? AND \"comments\".\"parent_id\" IS ? ORDER BY comments.created_at DESC"
-
-    AppOpticsAPM::Config[:sanitize_sql] = false
   end
 
   def test_sanitize_sql4
@@ -40,8 +39,25 @@ def test_sanitize_sql4
     sql = "SELECT `assets`.* FROM `assets` WHERE `assets`.`type` IN ('Picture') AND (updated_at >= '2015-07-08 19:22:00') AND (updated_at <= '2015-07-08 19:23:00') LIMIT 31 OFFSET 0"
     result = AppOpticsAPM::Util.sanitize_sql(sql)
     result.must_equal "SELECT `assets`.* FROM `assets` WHERE `assets`.`type` IN (?) AND (updated_at >= ?) AND (updated_at <= ?) LIMIT ? OFFSET ?"
+  end
 
-    AppOpticsAPM::Config[:sanitize_sql] = false
+  def test_sanitize_quoted_stuff1
+    AppOpticsAPM::Config[:sanitize_sql] = true
+
+    sql = "SELECT `users`.* FROM `users` WHERE (mobile IN ('234 234 234') AND email IN ('a_b_c@hotmail.co.uk'))"
+    result = AppOpticsAPM::Util.sanitize_sql(sql)
+    result.must_equal "SELECT `users`.* FROM `users` WHERE (mobile IN (?) AND email IN (?))"
+  end
+
+  def test_sanitize_quoted_stuff2
+    AppOpticsAPM::Config[:sanitize_sql] = true
+
+
+    # trying to reproduce "SELECT `users`.* FROM `users` WHERE (mobile IN (?a_b_c@hotmail.co.uk') LIMIT ?"
+    sql = "SELECT `users`.* FROM `users` WHERE (mobile IN ('\\\'1454545') AND email IN ('a_b_c@hotmail.co.uk')) LIMIT 5"
+    # sql = "SELECT `users`.* FROM `users` WHERE (mobile IN ('2342423') AND email IN ('a_b_c@hotmail.co.uk')) LIMIT 5"
+    result = AppOpticsAPM::Util.sanitize_sql(sql)
+    result.must_equal "SELECT `users`.* FROM `users` WHERE (mobile IN (?) AND email IN (?)) LIMIT ?"
   end
 
   def test_dont_sanitize