diff options
author | Richard Monette <richard.monette@shopify.com> | 2016-12-12 16:51:39 -0500 |
---|---|---|
committer | Richard Monette <richard.monette@shopify.com> | 2016-12-15 16:38:35 -0500 |
commit | e220fda3e5a497f4b971cf1bb59b2020059634bf (patch) | |
tree | 9ece2b2f7e1d1e4dd82caa26576ec9b530d15c3f | |
parent | 4b576f482152411097ecda0c4527bfb01a1d2965 (diff) | |
download | rails-e220fda3e5a497f4b971cf1bb59b2020059634bf.tar.gz rails-e220fda3e5a497f4b971cf1bb59b2020059634bf.tar.bz2 rails-e220fda3e5a497f4b971cf1bb59b2020059634bf.zip |
fix QueryCache nil dup
make sql statements frozen
dup if arel is not our string
expect runtime error
dont wrap runtime error in invalid
log errors will now be treated as runtime errors
update changelog
5 files changed, 34 insertions, 5 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4b74527144..eac54e7566 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,13 @@ +* Notifications see frozen SQL string. + + Fixes #23774 + + *Richard Monette* + +* RuntimeErrors are no longer translated to ActiveRecord::StatementInvalid. + + *Richard Monette* + * Change the schema cache format to use YAML instead of Marshal. *Kir Shatrov* diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index faccd1d641..947796eea0 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -10,9 +10,9 @@ module ActiveRecord def to_sql(arel, binds = []) if arel.respond_to?(:ast) collected = visitor.accept(arel.ast, collector) - collected.compile(binds, self) + collected.compile(binds, self).freeze else - arel + arel.dup.freeze end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 237367c8b3..284529b46e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -598,7 +598,12 @@ module ActiveRecord def translate_exception(exception, message) # override in derived class - ActiveRecord::StatementInvalid.new(message) + case exception + when RuntimeError + exception + else + ActiveRecord::StatementInvalid.new(message) + end end def without_prepared_statement?(binds) diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 8de69869a4..3fce0a1df1 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -285,13 +285,13 @@ module ActiveRecord unless current_adapter?(:PostgreSQLAdapter) def test_log_invalid_encoding - error = assert_raise ActiveRecord::StatementInvalid do + error = assert_raises RuntimeError do @connection.send :log, "SELECT 'ы' FROM DUAL" do raise "ы".force_encoding(Encoding::ASCII_8BIT) end end - assert_not_nil error.cause + assert_not_nil error.message end end diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 90054ce83d..4a49bfe9b1 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -202,6 +202,20 @@ class QueryCacheTest < ActiveRecord::TestCase ActiveSupport::Notifications.unsubscribe subscriber end + def test_query_cache_does_not_allow_sql_key_mutation + subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |_, _, _, _, payload| + payload[:sql].downcase! + end + + assert_raises RuntimeError do + ActiveRecord::Base.cache do + assert_queries(1) { Task.find(1); Task.find(1) } + end + end + ensure + ActiveSupport::Notifications.unsubscribe subscriber + end + def test_cache_is_flat Task.cache do assert_queries(1) { Topic.find(1); Topic.find(1); } |