Avoid quote_string in favor of bindings where possible

app/models/actions/scopes/default.rb

@@ -59,16 +59,18 @@ module Actions::Scopes
       def map_actions(permission, actions:, global:, module_name:, grant_to_admin:, public:)
         actions.map do |namespace, actions|
           actions.map do |action|
-            values = [
-              quote_string("#{action_v3_name(namespace)}/#{action}"),
-              quote_string(permission),
-              global,
-              module_name ? quote_string(module_name) : "NULL",
-              grant_to_admin,
-              public
-            ].join(", ")
-
-            "(#{values})"
+            ActiveRecord::Base.send(
+              :sanitize_sql_array,
+              [
+                "(?, ?, ?, ?, ?, ?)",
+                "#{action_v3_name(namespace)}/#{action}",
+                permission,
+                global,
+                module_name,
+                grant_to_admin,
+                public
+              ]
+            )
           end
         end
       end
@@ -76,10 +78,6 @@ module Actions::Scopes
       def action_v3_name(name)
         API::Utilities::PropertyNameConverter.from_ar_name(name.to_s.singularize).pluralize.underscore
       end
-
-      def quote_string(string)
-        ActiveRecord::Base.connection.quote(string.to_s)
-      end
     end
   end
 end

app/models/custom_field/order_statements.rb

@@ -113,6 +113,11 @@ module CustomField::OrderStatements
   #   ) cf_order_NNN ON cf_order_NNN.customized_id = …
   #
   def join_for_order_sql(value:, add_select: nil, join: nil, multi_value: false)
+    customized_type_condition = ActiveRecord::Base.send(
+      :sanitize_sql_array,
+      ["cv.customized_type = ?", self.class.customized_class.base_class.name]
+    )
+
     <<~SQL.squish
       LEFT OUTER JOIN (
         SELECT
@@ -122,7 +127,7 @@ module CustomField::OrderStatements
             #{", #{add_select}" if add_select}
           FROM #{CustomValue.quoted_table_name} cv
           #{join}
-          WHERE cv.customized_type = #{CustomValue.connection.quote(self.class.customized_class.base_class.name)}
+          WHERE #{customized_type_condition}
             AND cv.custom_field_id = #{id}
             AND cv.value IS NOT NULL
             AND cv.value != ''

app/models/queries/operators/boolean_equals.rb

@@ -40,8 +40,7 @@ module Queries::Operators
         sql = "#{db_table}.#{db_field} IS NULL OR "
       end
 
-      sql += "#{db_table}.#{db_field} IN (" +
-             values.map { |val| "'#{connection.quote_string(val)}'" }.join(",") + ")"
+      sql += ActiveRecord::Base.send(:sanitize_sql_array, ["#{db_table}.#{db_field} IN (?)", values])
 
       sql
     end

app/models/queries/operators/boolean_equals_strict.rb

@@ -34,7 +34,7 @@ module Queries::Operators
     set_symbol "="
 
     def self.sql_for_field(values, db_table, db_field)
-      "#{db_table}.#{db_field} IN (#{values.map { |val| "'#{connection.quote_string(val)}'" }.join(',')})"
+      ActiveRecord::Base.send(:sanitize_sql_array, ["#{db_table}.#{db_field} IN (?)", values])
     end
   end
 end

app/models/queries/operators/custom_fields/equals_all.rb

@@ -41,10 +41,18 @@ module Queries::Operators
 
         if values.present?
           sql = values.map do |val|
-            "EXISTS (SELECT 1 FROM #{cv_table} WHERE customized_type = '#{connection.quote_string(customized_type)}' " \
-              "AND custom_field_id = #{custom_field_id} " \
-              "AND customized_id = #{customized_id_join_field} " \
-              "AND value ='#{connection.quote_string(val)}')"
+            ActiveRecord::Base.send(
+              :sanitize_sql_array,
+              [
+                "EXISTS (SELECT 1 FROM #{cv_table} WHERE customized_type = ? " \
+                "AND custom_field_id = ? " \
+                "AND customized_id = #{customized_id_join_field} " \
+                "AND value = ?)",
+                customized_type,
+                custom_field_id,
+                val
+              ]
+            )
           end
 
           sql.join(" AND ")

app/models/queries/operators/custom_fields/not_equals_all.rb

@@ -38,10 +38,18 @@ module Queries::Operators
 
         if values.present?
           sql = values.map do |val|
-            "NOT EXISTS (SELECT 1 FROM #{cv_table} WHERE customized_type = '#{connection.quote_string(customized_type)}' " \
-              "AND custom_field_id = #{custom_field_id} " \
-              "AND customized_id = #{customized_id_join_field} " \
-              "AND value ='#{connection.quote_string(val)}')"
+            ActiveRecord::Base.send(
+              :sanitize_sql_array,
+              [
+                "NOT EXISTS (SELECT 1 FROM #{cv_table} WHERE customized_type = ? " \
+                "AND custom_field_id = ? " \
+                "AND customized_id = #{customized_id_join_field} " \
+                "AND value = ?)",
+                customized_type,
+                custom_field_id,
+                val
+              ]
+            )
           end
 
           sql.join(" AND ")

app/models/queries/operators/equals.rb

@@ -44,8 +44,7 @@ module Queries::Operators
           sql = "#{db_table}.#{db_field} IS NULL OR "
         end
 
-        sql += "#{db_table}.#{db_field} IN (" +
-               values.map { |val| "'#{connection.quote_string(val)}'" }.join(",") + ")"
+        sql += ActiveRecord::Base.send(:sanitize_sql_array, ["#{db_table}.#{db_field} IN (?)", values])
       else
         # empty set of allowed values produces no result
         sql = "0=1"

app/models/queries/operators/not_equals.rb

@@ -38,8 +38,8 @@ module Queries::Operators
       values = values.map(&:to_s)
 
       if values.present?
-        "(#{db_table}.#{db_field} IS NULL OR #{db_table}.#{db_field} NOT IN (" +
-          values.map { |val| "'#{connection.quote_string(val)}'" }.join(",") + "))"
+        not_in = ActiveRecord::Base.send(:sanitize_sql_array, ["#{db_table}.#{db_field} NOT IN (?)", values])
+        "(#{db_table}.#{db_field} IS NULL OR #{not_in})"
       else
         # empty set of forbidden values allows all results
         "1=1"

app/models/queries/work_packages/filter/shared_with_user_filter.rb

@@ -93,17 +93,32 @@ class Queries::WorkPackages::Filter::SharedWithUserFilter <
     members_table = Member.table_name
 
     where_clauses = values_replaced.map do |user_id|
-      <<~SQL.squish
-        EXISTS (SELECT 1
-                FROM #{members_table}
-                WHERE #{members_table}.entity_id = #{work_packages_table}.id
-                AND #{members_table}.user_id = #{ActiveRecord::Base.connection.quote_string(user_id)})
-      SQL
+      normalized_user_id = normalize_user_id(user_id)
+
+      if normalized_user_id.nil?
+        "1=0"
+      else
+        ActiveRecord::Base.send(
+          :sanitize_sql_array,
+          [<<~SQL.squish, normalized_user_id]
+            EXISTS (SELECT 1
+                    FROM #{members_table}
+                    WHERE #{members_table}.entity_id = #{work_packages_table}.id
+                    AND #{members_table}.user_id = ?)
+          SQL
+        )
+      end
     end
 
     where_clauses.join(" AND ")
   end
 
+  def normalize_user_id(user_id)
+    Integer(user_id, 10)
+  rescue ArgumentError, TypeError
+    nil
+  end
+
   def querying_for_self?
     values_replaced.size == 1 && values_replaced.first == User.current.id.to_s
   end

app/models/sessions/sql_bypass.rb

@@ -82,27 +82,43 @@ module Sessions
 
     def insert!
       @new_record = false
-      connection.update <<~SQL, "Create session"
-         INSERT INTO sessions (session_id, data, user_id, updated_at)
-         VALUES (
-           #{connection.quote(session_id)},
-           #{connection.quote(self.class.serialize(data))},
-           #{connection.quote(user_id)},
-           (now() at time zone 'utc')
-        )
-      SQL
+      sql = ActiveRecord::Base.send(
+        :sanitize_sql_array,
+        [
+          <<~SQL.squish,
+            INSERT INTO sessions (session_id, data, user_id, updated_at)
+            VALUES (?, ?, ?, (now() at time zone 'utc'))
+          SQL
+          session_id,
+          self.class.serialize(data),
+          user_id
+        ]
+      )
+
+      connection.update sql, "Create session"
     end
 
     def update!
-      connection.update <<~SQL, "Update session"
-        UPDATE sessions
-        SET
-          data=#{connection.quote(self.class.serialize(data))},
-          session_id=#{connection.quote(session_id)},
-          user_id=#{connection.quote(user_id)},
-          updated_at=(now() at time zone 'utc')
-        WHERE session_id=#{connection.quote(@retrieved_by)}
-      SQL
+      sql = ActiveRecord::Base.send(
+        :sanitize_sql_array,
+        [
+          <<~SQL.squish,
+            UPDATE sessions
+            SET
+              data = ?,
+              session_id = ?,
+              user_id = ?,
+              updated_at = (now() at time zone 'utc')
+            WHERE session_id = ?
+          SQL
+          self.class.serialize(data),
+          session_id,
+          user_id,
+          @retrieved_by
+        ]
+      )
+
+      connection.update sql, "Update session"
     end
   end
 end

lib/api/v3/projects/project_sql_representer.rb

@@ -47,7 +47,7 @@ module API
           end
 
           def ancestors_sql(walker_result)
-            <<-SQL.squish
+            <<~SQL.squish
               SELECT id, CASE WHEN count(link) = 0 THEN '[]' ELSE json_agg(link) END ancestors
               FROM
                 (
@@ -73,8 +73,13 @@ module API
           end
 
           def ancestor_projection
+            undisclosed_ancestor_title = ActiveRecord::Base.send(
+              :sanitize_sql_array,
+              ["?", I18n.t(:"api_v3.undisclosed.ancestor")]
+            )
+
             if User.current.admin?
-              <<-SQL.squish
+              <<~SQL.squish
                 CASE
                   WHEN ancestors.id IS NOT NULL
                     THEN #{workspace_type_link_case('ancestors')}
@@ -82,13 +87,13 @@ module API
                 END
               SQL
             else
-              <<-SQL.squish
+              <<~SQL.squish
                 CASE
                   WHEN ancestors.id IS NOT NULL AND ancestors.id IN (SELECT id FROM visible_projects)
                     THEN #{workspace_type_link_case('ancestors')}
                   WHEN ancestors.id IS NOT NULL AND ancestors.id NOT IN (SELECT id FROM visible_projects)
                     THEN json_build_object('href', '#{API::V3::URN_UNDISCLOSED}',
-                                           'title', #{ActiveRecord::Base.connection.quote(I18n.t(:"api_v3.undisclosed.ancestor"))})
+                                           'title', #{undisclosed_ancestor_title})
                   ELSE NULL
                 END
               SQL

lib/open_project/nested_set/rebuild_patch.rb

@@ -47,7 +47,7 @@ module OpenProject::NestedSet::RebuildPatch
   module ClassMethods
     # Rebuilds the left & rights if unset or invalid.  Also very useful for converting from acts_as_tree.
     # Very similar to original nested_set implementation but uses update_all so that callbacks are not triggered
-    def rebuild_silently!(roots = nil)
+    def rebuild_silently!(roots = nil) # rubocop:disable Metrics/AbcSize,Metrics/PerceivedComplexity
       # Don't rebuild a valid tree.
       return true if valid?
 
@@ -55,7 +55,10 @@ module OpenProject::NestedSet::RebuildPatch
       if acts_as_nested_set_options[:scope]
         scope = lambda { |node|
           scope_column_names.inject("") do |str, column_name|
-            str << "AND #{connection.quote_column_name(column_name)} = #{connection.quote(node.send(column_name.to_sym))} "
+            str << ActiveRecord::Base.send(
+              :sanitize_sql_array,
+              ["AND #{connection.quote_column_name(column_name)} = ? ", node.send(column_name.to_sym)]
+            )
           end
         }
       end

modules/meeting/app/models/queries/meetings/filters/attended_user_filter.rb

@@ -45,16 +45,20 @@ class Queries::Meetings::Filters::AttendedUserFilter < Queries::Meetings::Filter
     when "="
       [operator_strategy.sql_for_field(values, MeetingParticipant.table_name, "user_id"), condition].join(" AND ")
     when "!"
-      return "1=1" if values.empty?
+      user_id = normalized_user_id(values.first)
+      return "1=1" if user_id.nil?
 
-      <<~SQL.squish
-        NOT EXISTS (
-          SELECT 1 FROM #{MeetingParticipant.table_name}
-          WHERE #{MeetingParticipant.table_name}.meeting_id = meetings.id
-          AND #{MeetingParticipant.table_name}.user_id = '#{MeetingParticipant.connection.quote_string(values.first)}'
-          AND #{condition}
-        )
-      SQL
+      ActiveRecord::Base.send(
+        :sanitize_sql_array,
+        [<<~SQL.squish, user_id]
+          NOT EXISTS (
+            SELECT 1 FROM #{MeetingParticipant.table_name}
+            WHERE #{MeetingParticipant.table_name}.meeting_id = meetings.id
+            AND #{MeetingParticipant.table_name}.user_id = ?
+            AND #{condition}
+          )
+        SQL
+      )
     when "*"
       ["#{MeetingParticipant.table_name}.user_id IS NOT NULL", condition].join(" AND ")
     when "!*"
@@ -79,4 +83,12 @@ class Queries::Meetings::Filters::AttendedUserFilter < Queries::Meetings::Filter
   def self.key
     :attended_user_id
   end
+
+  private
+
+  def normalized_user_id(value)
+    Integer(value, 10)
+  rescue ArgumentError, TypeError
+    nil
+  end
 end

modules/meeting/app/models/queries/meetings/filters/invited_user_filter.rb

@@ -45,16 +45,20 @@ class Queries::Meetings::Filters::InvitedUserFilter < Queries::Meetings::Filters
     when "="
       [operator_strategy.sql_for_field(values, MeetingParticipant.table_name, "user_id"), condition].join(" AND ")
     when "!"
-      return "1=1" if values.empty?
+      user_id = normalized_user_id(values.first)
+      return "1=1" if user_id.nil?
 
-      <<~SQL.squish
-        NOT EXISTS (
-          SELECT 1 FROM #{MeetingParticipant.table_name}
-          WHERE #{MeetingParticipant.table_name}.meeting_id = meetings.id
-          AND #{MeetingParticipant.table_name}.user_id = '#{MeetingParticipant.connection.quote_string(values.first)}'
-          AND #{condition}
-        )
-      SQL
+      ActiveRecord::Base.send(
+        :sanitize_sql_array,
+        [<<~SQL.squish, user_id]
+          NOT EXISTS (
+            SELECT 1 FROM #{MeetingParticipant.table_name}
+            WHERE #{MeetingParticipant.table_name}.meeting_id = meetings.id
+            AND #{MeetingParticipant.table_name}.user_id = ?
+            AND #{condition}
+          )
+        SQL
+      )
     when "*"
       ["#{MeetingParticipant.table_name}.user_id IS NOT NULL", condition].join(" AND ")
     when "!*"
@@ -79,4 +83,12 @@ class Queries::Meetings::Filters::InvitedUserFilter < Queries::Meetings::Filters
   def self.key
     :invited_user_id
   end
+
+  private
+
+  def normalized_user_id(value)
+    Integer(value, 10)
+  rescue ArgumentError, TypeError
+    nil
+  end
 end

modules/reporting/lib/report/operator.rb

@@ -33,6 +33,8 @@ class Report::Operator
 
   #############################################################################################
   # Wrapped so we can place this at the top of the file.
+  # rubocop:disable Lint/NestedMethodDefinition, Metrics/AbcSize, Metrics/PerceivedComplexity
+  # The operator DSL intentionally defines #modify on each operator's singleton.
   def self.define_operators # :nodoc:
     # Defaults
     defaults do
@@ -108,7 +110,7 @@ class Report::Operator
     new "!~", arity: 1, label: :label_not_contains do
       def modify(query, field, *values)
         value = values.first || ""
-        query.where "LOWER(#{field}) NOT LIKE '%#{quote_string(value.to_s.downcase)}%'"
+        query.where ["LOWER(#{field}) NOT LIKE ?", "%#{value.to_s.downcase}%"]
         query
       end
     end
@@ -129,7 +131,7 @@ class Report::Operator
     new "~", arity: 1, label: :label_contains do
       def modify(query, field, *values)
         value = values.first || ""
-        query.where "LOWER(#{field}) LIKE '%#{quote_string(value.to_s.downcase)}%'"
+        query.where ["LOWER(#{field}) LIKE ?", "%#{value.to_s.downcase}%"]
         query
       end
     end
@@ -246,6 +248,7 @@ class Report::Operator
       end
     end
   end
+  # rubocop:enable Lint/NestedMethodDefinition, Metrics/AbcSize, Metrics/PerceivedComplexity
   #############################################################################################
 
   module CoreExt

modules/reporting/lib/report/query_utils.rb

@@ -35,17 +35,6 @@ module Report::QueryUtils
 
   include Costs::NumberHelper
 
-  ##
-  # Graceful string quoting.
-  #
-  # @param [Object] str String to quote
-  # @return [Object] Quoted version
-  def quote_string(str)
-    return str unless str.respond_to? :to_str
-
-    engine.reporting_connection.quote_string(str)
-  end
-
   def current_language
     ::I18n.locale
   end
@@ -53,7 +42,7 @@ module Report::QueryUtils
   ##
   # Creates a SQL fragment representing a collection/array.
   #
-  # @see quote_string
+  # @see quote_sql_string_value
   # @param [#flatten] *values Ruby collection
   # @return [String] SQL collection
   def collection(*values)
@@ -67,7 +56,7 @@ module Report::QueryUtils
           split_with_safe_return(str)
         end
 
-    "(#{v.flatten.map { |x| "'#{quote_string(x)}'" }.join(', ')})"
+    "(#{v.flatten.map { |x| quote_sql_string_value(x) }.join(', ')})"
   end
 
   def split_with_safe_return(str)
@@ -80,11 +69,11 @@ module Report::QueryUtils
   ##
   # Graceful, internationalized quoted string.
   #
-  # @see quote_string
+  # @see quote_sql_string_value
   # @param [Object] str String to quote/translate
   # @return [Object] Quoted, translated version
   def quoted_label(ident)
-    "'#{quote_string ::I18n.t(ident)}'"
+    quote_sql_string_value(::I18n.t(ident))
   end
 
   def quoted_date(date)
@@ -202,10 +191,14 @@ module Report::QueryUtils
   end
 
   def typed(type, value, escape = true)
-    safe_value = escape ? "'#{quote_string value}'" : value
+    safe_value = escape ? quote_sql_string_value(value) : value
     "#{safe_value}::#{type}"
   end
 
+  def quote_sql_string_value(value)
+    engine.reporting_connection.quote(value.to_s)
+  end
+
   def iso_year_week(field_name)
     "(EXTRACT(isoyear from #{field_name})*100 + \n\t\t" \
       "EXTRACT(week from #{field_name} - \n\t\t" \

spec/support/queries/filters/shared_filter_examples.rb

@@ -165,7 +165,7 @@ RSpec.shared_examples_for "list_optional query filter" do
       let(:operator) { "=" }
 
       it "is the same as handwriting the query" do
-        quoted_values = db_values.map { |val| "'#{ActiveRecord::Base.connection.quote_string(val)}'" }.join(",")
+        quoted_values = db_values.map { |val| ActiveRecord::Base.connection.quote(val) }.join(",")
 
         expected = expected_base_scope
                    .where("#{expected_table_name}.#{attribute} IN (#{quoted_values})")
@@ -178,7 +178,7 @@ RSpec.shared_examples_for "list_optional query filter" do
       let(:operator) { "!" }
 
       it "is the same as handwriting the query" do
-        quoted_values = db_values.map { |val| "'#{ActiveRecord::Base.connection.quote_string(val)}'" }.join(",")
+        quoted_values = db_values.map { |val| ActiveRecord::Base.connection.quote(val) }.join(",")
 
         sql = "(#{expected_table_name}.#{attribute} IS NULL
                OR #{expected_table_name}.#{attribute} NOT IN (#{quoted_values}))".squish