From ddcce35c28042745d14f81b1275eabdc589a71a5 Mon Sep 17 00:00:00 2001 From: blackst0ne Date: Thu, 19 Apr 2018 13:31:11 +1100 Subject: [PATCH] [Rails5] Avoid type casting in uniqueness validator This commit does two things: 1. Synchronizes `Gemfile.rails5.lock` with the latest changes in `Gemfile` 2. Adds a monkey patch for active record to avoid type casting in uniqueness validator. In Rails 5.0 there was introduced a bug [1] that causes to problems like this: ``` 1) API::Users POST /user/:id/gpg_keys/:key_id/revoke when authenticated revokes existing key Failure/Error: let(:gpg_key) { create(:gpg_key, user: user) } TypeError: can't cast Hash ``` This bug was fixed in Rails 5.1 [2]. [1]: https://github.com/rails/rails/pull/23523/commits/811a4fa8eb6ceea841e61e8ac05747ffb69595ae [2]: https://github.com/rails/rails/pull/24745/commits/aa062318c451512035c10898a1af95943b1a3803 --- Gemfile.rails5.lock | 40 ++++---- ...id_type_casting_in_uniqueness_validator.rb | 98 +++++++++++++++++++ 2 files changed, 116 insertions(+), 22 deletions(-) create mode 100644 config/initializers/active_record_avoid_type_casting_in_uniqueness_validator.rb diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index c953b9708a0..a0330cbdd02 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -69,7 +69,7 @@ GEM unf ast (2.4.0) atomic (1.1.100) - attr_encrypted (3.0.3) + attr_encrypted (3.1.0) encryptor (~> 3.0.0) attr_required (1.0.1) autoprefixer-rails (8.1.0.1) @@ -291,9 +291,9 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) gherkin-ruby (0.3.2) - gitaly-proto (0.94.0) + gitaly-proto (0.97.0) google-protobuf (~> 3.1) - grpc (~> 1.0) + grpc (~> 1.10) github-linguist (5.3.3) charlock_holmes (~> 0.7.5) escape_utils (~> 1.1.0) @@ -304,6 +304,17 @@ GEM flowdock (~> 0.7) gitlab-grit (>= 2.4.1) multi_json + gitlab-gollum-lib (4.2.7.1) + gemojione (~> 3.2) + github-markup (~> 1.6) + gollum-grit_adapter (~> 1.0) + nokogiri (>= 1.6.1, < 2.0) + rouge (~> 2.1) + sanitize (~> 2.1) + stringex (~> 2.6) + gitlab-gollum-rugged_adapter (0.4.4) + mime-types (>= 1.15) + rugged (~> 0.25) gitlab-grit (2.8.2) charlock_holmes (~> 0.6) diff-lcs (~> 1.1) @@ -321,22 +332,8 @@ GEM rubyntlm (~> 0.5) globalid (0.4.1) activesupport (>= 4.2.0) - goldiloader (2.0.1) - activerecord (>= 4.2, < 5.2) - activesupport (>= 4.2, < 5.2) gollum-grit_adapter (1.0.1) gitlab-grit (~> 2.7, >= 2.7.1) - gollum-lib (4.2.7) - gemojione (~> 3.2) - github-markup (~> 1.6) - gollum-grit_adapter (~> 1.0) - nokogiri (>= 1.6.1, < 2.0) - rouge (~> 2.1) - sanitize (~> 2.1) - stringex (~> 2.6) - gollum-rugged_adapter (0.4.4) - mime-types (>= 1.15) - rugged (~> 0.25) gon (6.1.0) actionpack (>= 3.0) json @@ -1009,7 +1006,7 @@ DEPENDENCIES asciidoctor (~> 1.5.6) asciidoctor-plantuml (= 0.0.8) asset_sync (~> 2.2.0) - attr_encrypted (~> 3.0.0) + attr_encrypted (~> 3.1.0) awesome_print (~> 1.2.0) babosa (~> 1.0.2) base32 (~> 0.3.0) @@ -1069,15 +1066,14 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.94.0) + gitaly-proto (~> 0.97.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) + gitlab-gollum-lib (~> 4.2) + gitlab-gollum-rugged_adapter (~> 0.4.4) gitlab-markup (~> 1.6.2) gitlab-styles (~> 2.3) gitlab_omniauth-ldap (~> 2.0.4) - goldiloader (~> 2.0) - gollum-lib (~> 4.2) - gollum-rugged_adapter (~> 0.4.4) gon (~> 6.1.0) google-api-client (~> 0.19.8) google-protobuf (= 3.5.1) diff --git a/config/initializers/active_record_avoid_type_casting_in_uniqueness_validator.rb b/config/initializers/active_record_avoid_type_casting_in_uniqueness_validator.rb new file mode 100644 index 00000000000..d9418caf68b --- /dev/null +++ b/config/initializers/active_record_avoid_type_casting_in_uniqueness_validator.rb @@ -0,0 +1,98 @@ +# This is a monkey patch which must be removed when migrating to Rails 5.1 from 5.0. +# +# In Rails 5.0 there was introduced a bug which casts types in the uniqueness validator. +# https://github.com/rails/rails/pull/23523/commits/811a4fa8eb6ceea841e61e8ac05747ffb69595ae +# +# That causes to bugs like this: +# +# 1) API::Users POST /user/:id/gpg_keys/:key_id/revoke when authenticated revokes existing key +# Failure/Error: let(:gpg_key) { create(:gpg_key, user: user) } +# +# TypeError: +# can't cast Hash +# # ./spec/requests/api/users_spec.rb:7:in `block (2 levels) in ' +# # ./spec/requests/api/users_spec.rb:908:in `block (4 levels) in ' +# # ------------------ +# # --- Caused by: --- +# # TypeError: +# # TypeError +# # ./spec/requests/api/users_spec.rb:7:in `block (2 levels) in ' +# +# This bug was fixed in Rails 5.1 by https://github.com/rails/rails/pull/24745/commits/aa062318c451512035c10898a1af95943b1a3803 + +if Gitlab.rails5? + ActiveSupport::Deprecation.warn("#{__FILE__} is a monkey patch which must be removed when upgrading to Rails 5.1") + + if Rails.version.start_with?("5.1") + raise "Remove this monkey patch: #{__FILE__}" + end + + # Copy-paste from https://github.com/kamipo/rails/blob/aa062318c451512035c10898a1af95943b1a3803/activerecord/lib/active_record/validations/uniqueness.rb + # including local fixes to make Rubocop happy again. + module ActiveRecord + module Validations + class UniquenessValidator < ActiveModel::EachValidator # :nodoc: + def validate_each(record, attribute, value) + finder_class = find_finder_class_for(record) + table = finder_class.arel_table + value = map_enum_attribute(finder_class, attribute, value) + + relation = build_relation(finder_class, table, attribute, value) + + if record.persisted? + if finder_class.primary_key + relation = relation.where.not(finder_class.primary_key => record.id_was || record.id) + else + raise UnknownPrimaryKey.new(finder_class, "Can not validate uniqueness for persisted record without primary key.") + end + end + + relation = scope_relation(record, table, relation) + relation = relation.merge(options[:conditions]) if options[:conditions] + + if relation.exists? + error_options = options.except(:case_sensitive, :scope, :conditions) + error_options[:value] = value + + record.errors.add(attribute, :taken, error_options) + end + rescue RangeError + end + + protected + + def build_relation(klass, table, attribute, value) #:nodoc: + if reflection = klass._reflect_on_association(attribute) + attribute = reflection.foreign_key + value = value.attributes[reflection.klass.primary_key] unless value.nil? + end + + # the attribute may be an aliased attribute + if klass.attribute_alias?(attribute) + attribute = klass.attribute_alias(attribute) + end + + attribute_name = attribute.to_s + + column = klass.columns_hash[attribute_name] + cast_type = klass.type_for_attribute(attribute_name) + + comparison = + if !options[:case_sensitive] && !value.nil? + # will use SQL LOWER function before comparison, unless it detects a case insensitive collation + klass.connection.case_insensitive_comparison(table, attribute, column, value) + else + klass.connection.case_sensitive_comparison(table, attribute, column, value) + end + + if value.nil? + klass.unscoped.where(comparison) + else + bind = Relation::QueryAttribute.new(attribute_name, value, cast_type) + klass.unscoped.where(comparison, bind) + end + end + end + end + end +end