From 0016280f4fde55d96738887093dc333aae0d107b Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 23 Feb 2024 10:44:40 +0100 Subject: [PATCH] Don't require an active connection for table and column quoting Extracted from: https://github.com/rails/rails/pull/50793 Right now quoting table or column names requires a leased Adapter instance, even though none of the implementations actually requires an active connection. The idea here is to move these methods to the class so that the quoting can be done without leasing a connection or even needing the connection to ever have been established. I also checked `activerecord-sqlserver-adapter` and `oracle-enhanced` gems, and neither need an active connection. --- .../attribute_methods/primary_key.rb | 2 +- .../connection_adapters/abstract/quoting.rb | 122 ++++++++++-------- .../connection_adapters/mysql/quoting.rb | 88 ++++++------- .../connection_adapters/postgresql/quoting.rb | 112 ++++++++-------- .../connection_adapters/sqlite3/quoting.rb | 88 ++++++------- .../lib/active_record/connection_handling.rb | 4 + .../lib/active_record/model_schema.rb | 2 +- activerecord/lib/active_record/relation.rb | 2 +- .../active_record/relation/calculations.rb | 4 +- .../active_record/relation/query_methods.rb | 10 +- .../lib/active_record/sanitization.rb | 4 +- .../abstract_mysql_adapter/quoting_test.rb | 19 +++ .../cases/adapters/postgresql/quoting_test.rb | 19 +++ .../cases/adapters/sqlite3/quoting_test.rb | 19 +++ .../adapters/sqlite3/sqlite3_adapter_test.rb | 4 - .../test/cases/column_definition_test.rb | 19 ++- activerecord/test/cases/quoting_test.rb | 10 +- activerecord/test/models/post.rb | 4 + 18 files changed, 307 insertions(+), 225 deletions(-) diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index aecd58696a8..db63a68db67 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -92,7 +92,7 @@ module ActiveRecord # Returns a quoted version of the primary key name, used to construct # SQL statements. def quoted_primary_key - @quoted_primary_key ||= connection.quote_column_name(primary_key) + @quoted_primary_key ||= adapter_class.quote_column_name(primary_key) end def reset_primary_key # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index 00aa4309e28..a80b56d221d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -7,6 +7,67 @@ module ActiveRecord module ConnectionAdapters # :nodoc: # = Active Record Connection Adapters \Quoting module Quoting + extend ActiveSupport::Concern + + module ClassMethods # :nodoc: + # Regexp for column names (with or without a table name prefix). + # Matches the following: + # + # "#{table_name}.#{column_name}" + # "#{column_name}" + def column_name_matcher + / + \A + ( + (?: + # table_name.column_name | function(one or no argument) + ((?:\w+\.)?\w+ | \w+\((?:|\g<2>)\)) + ) + (?:(?:\s+AS)?\s+\w+)? + ) + (?:\s*,\s*\g<1>)* + \z + /ix + end + + # Regexp for column names with order (with or without a table name prefix, + # with or without various order modifiers). Matches the following: + # + # "#{table_name}.#{column_name}" + # "#{table_name}.#{column_name} #{direction}" + # "#{table_name}.#{column_name} #{direction} NULLS FIRST" + # "#{table_name}.#{column_name} NULLS LAST" + # "#{column_name}" + # "#{column_name} #{direction}" + # "#{column_name} #{direction} NULLS FIRST" + # "#{column_name} NULLS LAST" + def column_name_with_order_matcher + / + \A + ( + (?: + # table_name.column_name | function(one or no argument) + ((?:\w+\.)?\w+ | \w+\((?:|\g<2>)\)) + ) + (?:\s+ASC|\s+DESC)? + (?:\s+NULLS\s+(?:FIRST|LAST))? + ) + (?:\s*,\s*\g<1>)* + \z + /ix + end + + # Quotes the column name. Must be implemented by subclasses + def quote_column_name(column_name) + raise NotImplementedError + end + + # Quotes the table name. Defaults to column name quoting. + def quote_table_name(table_name) + quote_column_name(table_name) + end + end + # Quotes the column value to help prevent # {SQL injection attacks}[https://en.wikipedia.org/wiki/SQL_injection]. def quote(value) @@ -71,14 +132,14 @@ module ActiveRecord s.gsub("\\", '\&\&').gsub("'", "''") # ' (for ruby-mode) end - # Quotes the column name. Defaults to no quoting. + # Quotes the column name. def quote_column_name(column_name) - column_name.to_s + self.class.quote_column_name(column_name) end - # Quotes the table name. Defaults to column name quoting. + # Quotes the table name. def quote_table_name(table_name) - quote_column_name(table_name) + self.class.quote_table_name(table_name) end # Override to return the quoted table name for assignment. Defaults to @@ -159,59 +220,6 @@ module ActiveRecord comment end - def column_name_matcher # :nodoc: - COLUMN_NAME - end - - def column_name_with_order_matcher # :nodoc: - COLUMN_NAME_WITH_ORDER - end - - # Regexp for column names (with or without a table name prefix). - # Matches the following: - # - # "#{table_name}.#{column_name}" - # "#{column_name}" - COLUMN_NAME = / - \A - ( - (?: - # table_name.column_name | function(one or no argument) - ((?:\w+\.)?\w+ | \w+\((?:|\g<2>)\)) - ) - (?:(?:\s+AS)?\s+\w+)? - ) - (?:\s*,\s*\g<1>)* - \z - /ix - - # Regexp for column names with order (with or without a table name prefix, - # with or without various order modifiers). Matches the following: - # - # "#{table_name}.#{column_name}" - # "#{table_name}.#{column_name} #{direction}" - # "#{table_name}.#{column_name} #{direction} NULLS FIRST" - # "#{table_name}.#{column_name} NULLS LAST" - # "#{column_name}" - # "#{column_name} #{direction}" - # "#{column_name} #{direction} NULLS FIRST" - # "#{column_name} NULLS LAST" - COLUMN_NAME_WITH_ORDER = / - \A - ( - (?: - # table_name.column_name | function(one or no argument) - ((?:\w+\.)?\w+ | \w+\((?:|\g<2>)\)) - ) - (?:\s+ASC|\s+DESC)? - (?:\s+NULLS\s+(?:FIRST|LAST))? - ) - (?:\s*,\s*\g<1>)* - \z - /ix - - private_constant :COLUMN_NAME, :COLUMN_NAME_WITH_ORDER - private def type_casted_binds(binds) binds.map do |value| diff --git a/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb b/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb index 3b68cfb2df0..287b9c8c872 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb @@ -6,9 +6,52 @@ module ActiveRecord module ConnectionAdapters module MySQL module Quoting # :nodoc: + extend ActiveSupport::Concern + QUOTED_COLUMN_NAMES = Concurrent::Map.new # :nodoc: QUOTED_TABLE_NAMES = Concurrent::Map.new # :nodoc: + module ClassMethods # :nodoc: + def column_name_matcher + / + \A + ( + (?: + # `table_name`.`column_name` | function(one or no argument) + ((?:\w+\.|`\w+`\.)?(?:\w+|`\w+`) | \w+\((?:|\g<2>)\)) + ) + (?:(?:\s+AS)?\s+(?:\w+|`\w+`))? + ) + (?:\s*,\s*\g<1>)* + \z + /ix + end + + def column_name_with_order_matcher + / + \A + ( + (?: + # `table_name`.`column_name` | function(one or no argument) + ((?:\w+\.|`\w+`\.)?(?:\w+|`\w+`) | \w+\((?:|\g<2>)\)) + ) + (?:\s+COLLATE\s+(?:\w+|"\w+"))? + (?:\s+ASC|\s+DESC)? + ) + (?:\s*,\s*\g<1>)* + \z + /ix + end + + def quote_column_name(name) + QUOTED_COLUMN_NAMES[name] ||= "`#{name.to_s.gsub('`', '``')}`".freeze + end + + def quote_table_name(name) + QUOTED_TABLE_NAMES[name] ||= "`#{name.to_s.gsub('`', '``').gsub(".", "`.`")}`".freeze + end + end + def cast_bound_value(value) case value when Rational @@ -26,14 +69,6 @@ module ActiveRecord end end - def quote_column_name(name) - QUOTED_COLUMN_NAMES[name] ||= "`#{super.gsub('`', '``')}`".freeze - end - - def quote_table_name(name) - QUOTED_TABLE_NAMES[name] ||= -super.gsub(".", "`.`").freeze - end - def unquoted_true 1 end @@ -81,43 +116,6 @@ module ActiveRecord super end end - - def column_name_matcher - COLUMN_NAME - end - - def column_name_with_order_matcher - COLUMN_NAME_WITH_ORDER - end - - COLUMN_NAME = / - \A - ( - (?: - # `table_name`.`column_name` | function(one or no argument) - ((?:\w+\.|`\w+`\.)?(?:\w+|`\w+`) | \w+\((?:|\g<2>)\)) - ) - (?:(?:\s+AS)?\s+(?:\w+|`\w+`))? - ) - (?:\s*,\s*\g<1>)* - \z - /ix - - COLUMN_NAME_WITH_ORDER = / - \A - ( - (?: - # `table_name`.`column_name` | function(one or no argument) - ((?:\w+\.|`\w+`\.)?(?:\w+|`\w+`) | \w+\((?:|\g<2>)\)) - ) - (?:\s+COLLATE\s+(?:\w+|"\w+"))? - (?:\s+ASC|\s+DESC)? - ) - (?:\s*,\s*\g<1>)* - \z - /ix - - private_constant :COLUMN_NAME, :COLUMN_NAME_WITH_ORDER end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index 7a3e5e24918..e2d7d416fc5 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -4,9 +4,62 @@ module ActiveRecord module ConnectionAdapters module PostgreSQL module Quoting + extend ActiveSupport::Concern + QUOTED_COLUMN_NAMES = Concurrent::Map.new # :nodoc: QUOTED_TABLE_NAMES = Concurrent::Map.new # :nodoc: + module ClassMethods # :nodoc: + def column_name_matcher + / + \A + ( + (?: + # "schema_name"."table_name"."column_name"::type_name | function(one or no argument)::type_name + ((?:\w+\.|"\w+"\.){,2}(?:\w+|"\w+")(?:::\w+)? | \w+\((?:|\g<2>)\)(?:::\w+)?) + ) + (?:(?:\s+AS)?\s+(?:\w+|"\w+"))? + ) + (?:\s*,\s*\g<1>)* + \z + /ix + end + + def column_name_with_order_matcher + / + \A + ( + (?: + # "schema_name"."table_name"."column_name"::type_name | function(one or no argument)::type_name + ((?:\w+\.|"\w+"\.){,2}(?:\w+|"\w+")(?:::\w+)? | \w+\((?:|\g<2>)\)(?:::\w+)?) + ) + (?:\s+COLLATE\s+"\w+")? + (?:\s+ASC|\s+DESC)? + (?:\s+NULLS\s+(?:FIRST|LAST))? + ) + (?:\s*,\s*\g<1>)* + \z + /ix + end + + # Quotes column names for use in SQL queries. + def quote_column_name(name) # :nodoc: + QUOTED_COLUMN_NAMES[name] ||= PG::Connection.quote_ident(name.to_s).freeze + end + + # Checks the following cases: + # + # - table_name + # - "table.name" + # - schema_name.table_name + # - schema_name."table.name" + # - "schema.name".table_name + # - "schema.name"."table.name" + def quote_table_name(name) # :nodoc: + QUOTED_TABLE_NAMES[name] ||= Utils.extract_schema_qualified_name(name.to_s).quoted.freeze + end + end + class IntegerOutOf64BitRange < StandardError def initialize(msg) super(msg) @@ -77,29 +130,14 @@ module ActiveRecord end end - # Checks the following cases: - # - # - table_name - # - "table.name" - # - schema_name.table_name - # - schema_name."table.name" - # - "schema.name".table_name - # - "schema.name"."table.name" - def quote_table_name(name) # :nodoc: - QUOTED_TABLE_NAMES[name] ||= -Utils.extract_schema_qualified_name(name.to_s).quoted.freeze - end - def quote_table_name_for_assignment(table, attr) quote_column_name(attr) end - # Quotes column names for use in SQL queries. - def quote_column_name(name) # :nodoc: - QUOTED_COLUMN_NAMES[name] ||= PG::Connection.quote_ident(super).freeze - end - # Quotes schema names for use in SQL queries. - alias_method :quote_schema_name, :quote_column_name + def quote_schema_name(schema_name) + quote_column_name(schema_name) + end # Quote date/time values for use in SQL input. def quoted_date(value) # :nodoc: @@ -153,44 +191,6 @@ module ActiveRecord type_map.lookup(column.oid, column.fmod, column.sql_type) end - def column_name_matcher - COLUMN_NAME - end - - def column_name_with_order_matcher - COLUMN_NAME_WITH_ORDER - end - - COLUMN_NAME = / - \A - ( - (?: - # "schema_name"."table_name"."column_name"::type_name | function(one or no argument)::type_name - ((?:\w+\.|"\w+"\.){,2}(?:\w+|"\w+")(?:::\w+)? | \w+\((?:|\g<2>)\)(?:::\w+)?) - ) - (?:(?:\s+AS)?\s+(?:\w+|"\w+"))? - ) - (?:\s*,\s*\g<1>)* - \z - /ix - - COLUMN_NAME_WITH_ORDER = / - \A - ( - (?: - # "schema_name"."table_name"."column_name"::type_name | function(one or no argument)::type_name - ((?:\w+\.|"\w+"\.){,2}(?:\w+|"\w+")(?:::\w+)? | \w+\((?:|\g<2>)\)(?:::\w+)?) - ) - (?:\s+COLLATE\s+"\w+")? - (?:\s+ASC|\s+DESC)? - (?:\s+NULLS\s+(?:FIRST|LAST))? - ) - (?:\s*,\s*\g<1>)* - \z - /ix - - private_constant :COLUMN_NAME, :COLUMN_NAME_WITH_ORDER - private def lookup_cast_type(sql_type) super(query_value("SELECT #{quote(sql_type)}::regtype::oid", "SCHEMA").to_i) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/quoting.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/quoting.rb index 359b86f6a87..7308b64619b 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/quoting.rb @@ -4,9 +4,52 @@ module ActiveRecord module ConnectionAdapters module SQLite3 module Quoting # :nodoc: + extend ActiveSupport::Concern + QUOTED_COLUMN_NAMES = Concurrent::Map.new # :nodoc: QUOTED_TABLE_NAMES = Concurrent::Map.new # :nodoc: + module ClassMethods # :nodoc: + def column_name_matcher + / + \A + ( + (?: + # "table_name"."column_name" | function(one or no argument) + ((?:\w+\.|"\w+"\.)?(?:\w+|"\w+") | \w+\((?:|\g<2>)\)) + ) + (?:(?:\s+AS)?\s+(?:\w+|"\w+"))? + ) + (?:\s*,\s*\g<1>)* + \z + /ix + end + + def column_name_with_order_matcher + / + \A + ( + (?: + # "table_name"."column_name" | function(one or no argument) + ((?:\w+\.|"\w+"\.)?(?:\w+|"\w+") | \w+\((?:|\g<2>)\)) + ) + (?:\s+COLLATE\s+(?:\w+|"\w+"))? + (?:\s+ASC|\s+DESC)? + ) + (?:\s*,\s*\g<1>)* + \z + /ix + end + + def quote_column_name(name) + QUOTED_COLUMN_NAMES[name] ||= %Q("#{name.to_s.gsub('"', '""')}").freeze + end + + def quote_table_name(name) + QUOTED_TABLE_NAMES[name] ||= %Q("#{name.to_s.gsub('"', '""').gsub(".", "\".\"")}").freeze + end + end + def quote_string(s) ::SQLite3::Database.quote(s) end @@ -15,14 +58,6 @@ module ActiveRecord quote_column_name(attr) end - def quote_table_name(name) - QUOTED_TABLE_NAMES[name] ||= -super.gsub(".", "\".\"").freeze - end - - def quote_column_name(name) - QUOTED_COLUMN_NAMES[name] ||= %Q("#{super.gsub('"', '""')}").freeze - end - def quoted_time(value) value = value.change(year: 2000, month: 1, day: 1) quoted_date(value).sub(/\A\d\d\d\d-\d\d-\d\d /, "2000-01-01 ") @@ -75,43 +110,6 @@ module ActiveRecord super end end - - def column_name_matcher - COLUMN_NAME - end - - def column_name_with_order_matcher - COLUMN_NAME_WITH_ORDER - end - - COLUMN_NAME = / - \A - ( - (?: - # "table_name"."column_name" | function(one or no argument) - ((?:\w+\.|"\w+"\.)?(?:\w+|"\w+") | \w+\((?:|\g<2>)\)) - ) - (?:(?:\s+AS)?\s+(?:\w+|"\w+"))? - ) - (?:\s*,\s*\g<1>)* - \z - /ix - - COLUMN_NAME_WITH_ORDER = / - \A - ( - (?: - # "table_name"."column_name" | function(one or no argument) - ((?:\w+\.|"\w+"\.)?(?:\w+|"\w+") | \w+\((?:|\g<2>)\)) - ) - (?:\s+COLLATE\s+(?:\w+|"\w+"))? - (?:\s+ASC|\s+DESC)? - ) - (?:\s*,\s*\g<1>)* - \z - /ix - - private_constant :COLUMN_NAME, :COLUMN_NAME_WITH_ORDER end end end diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb index 8fbcfdce980..02b0b78e3ac 100644 --- a/activerecord/lib/active_record/connection_handling.rb +++ b/activerecord/lib/active_record/connection_handling.rb @@ -286,6 +286,10 @@ module ActiveRecord connection_pool.db_config end + def adapter_class # :nodoc: + connection_pool.db_config.adapter_class + end + def connection_pool connection_handler.retrieve_connection_pool(connection_specification_name, role: current_role, shard: current_shard, strict: true) end diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index a0835a005e4..f1f0eff4bd1 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -279,7 +279,7 @@ module ActiveRecord # Returns a quoted version of the table name, used to construct SQL statements. def quoted_table_name - @quoted_table_name ||= connection.quote_table_name(table_name) + @quoted_table_name ||= adapter_class.quote_table_name(table_name) end # Computes the table name, (re)sets it internally, and returns it. diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index b7a02f1b6b7..717f28af26f 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -469,7 +469,7 @@ module ActiveRecord collection = eager_loading? ? apply_join_dependency : self column = connection.visitor.compile(table[timestamp_column]) - select_values = "COUNT(*) AS #{connection.quote_column_name("size")}, MAX(%s) AS timestamp" + select_values = "COUNT(*) AS #{adapter_class.quote_column_name("size")}, MAX(%s) AS timestamp" if collection.has_limit_or_offset? query = collection.select("#{column} AS collection_cache_key_timestamp") diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index bd6fea8c286..378a655e4e9 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -511,13 +511,13 @@ module ActiveRecord column = aggregate_column(column_name) column_alias = column_alias_tracker.alias_for("#{operation} #{column_name.to_s.downcase}") select_value = operation_over_aggregate_column(column, operation, distinct) - select_value.as(connection.quote_column_name(column_alias)) + select_value.as(adapter_class.quote_column_name(column_alias)) select_values = [select_value] select_values += self.select_values unless having_clause.empty? select_values.concat group_columns.map { |aliaz, field| - aliaz = connection.quote_column_name(aliaz) + aliaz = adapter_class.quote_column_name(aliaz) if field.respond_to?(:as) field.as(aliaz) else diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 3307859bd2c..e031c401996 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -634,7 +634,7 @@ module ActiveRecord # # END ASC # def in_order_of(column, values) - klass.disallow_raw_sql!([column], permit: connection.column_name_with_order_matcher) + klass.disallow_raw_sql!([column], permit: model.adapter_class.column_name_with_order_matcher) return spawn.none! if values.empty? references = column_references([column]) @@ -1848,7 +1848,7 @@ module ActiveRecord case field when Symbol arel_column(field.to_s) do |attr_name| - connection.quote_table_name(attr_name) + adapter_class.quote_table_name(attr_name) end when String arel_column(field, &:itself) @@ -1878,7 +1878,7 @@ module ActiveRecord def table_name_matches?(from) table_name = Regexp.escape(table.name) - quoted_table_name = Regexp.escape(connection.quote_table_name(table.name)) + quoted_table_name = Regexp.escape(adapter_class.quote_table_name(table.name)) /(?:\A|(?