Fix Issue board when using Ruby 2.4
What does this MR do?
As I found in the issue (https://gitlab.com/gitlab-org/gitlab-ce/issues/35769#note_36342006), and explained in the commit message
# In Ruby 2.4, Hash#compact exists and returns a Hash, while in Ruby 2.3,
# Hash#compact is implemented by Rails and returns a new
# `ActionController::Parameters` instance in this case.
# By using Hash#compact! we're safe since we're modifying the hash in place.
Using Hash#compact!
on a ActionController::Parameters
breaks things in a subtle manner. I've checked and it seems that was the only place where we used compact
on a ActionController::Parameters
hash.
Ruby 2.3 / Rails 4.2.8
[1] pry(main)> params = ActionController::Parameters.new(foo: 'bar')
=> {"foo"=>"bar"}
[2] pry(main)> params.class
=> ActionController::Parameters
[3] pry(main)> params.compact.class
=> ActionController::Parameters
[4] pry(main)> params.compact!
=> nil
[5] pry(main)> params.class
=> ActionController::Parameters
Ruby 2.4 / Rails 4.2.8
[1] pry(main)> params = ActionController::Parameters.new(foo: 'bar')
=> {"foo"=>"bar"}
[2] pry(main)> params.class
=> ActionController::Parameters
[3] pry(main)> params.compact.class
=> Hash
[4] pry(main)> params.compact!
=> nil
[5] pry(main)> params.class
=> ActionController::Parameters
Ruby 2.3 / Rails 5.0.5
>> params = ActionController::Parameters.new(foo: 'bar')
=> <ActionController::Parameters {"foo"=>"bar"} permitted: >
>> params.class
=> ActionController::Parameters
>> params.compact.class
DEPRECATION WARNING: Method compact is deprecated and will be removed in Rails 5.1, as `ActionController::Parameters` no longer inherits from hash. Using this deprecated behavior exposes potential security problems. If you continue to use this method you may be creating a security vulnerability in your app that can be exploited. Instead, consider using one of these documented methods which are not deprecated: http://api.rubyonrails.org/v5.0.5/classes/ActionController/Parameters.html (called from irb_binding at (irb):3)
=> ActiveSupport::HashWithIndifferentAccess
Ruby 2.4 / Rails 5.0.5
>> params = ActionController::Parameters.new(foo: 'bar')
=> <ActionController::Parameters {"foo"=>"bar"} permitted: >
>> params.class
=> ActionController::Parameters
>> params.compact.class
DEPRECATION WARNING: Method compact is deprecated and will be removed in Rails 5.1, as `ActionController::Parameters` no longer inherits from hash. Using this deprecated behavior exposes potential security problems. If you continue to use this method you may be creating a security vulnerability in your app that can be exploited. Instead, consider using one of these documented methods which are not deprecated: http://api.rubyonrails.org/v5.0.5/classes/ActionController/Parameters.html (called from irb_binding at (irb):3)
=> ActiveSupport::HashWithIndifferentAccess
Are there points in the code the reviewer needs to double check?
It seems that this has been solved in Rails 4.2.9: https://github.com/rails/rails/blob/v4.2.9/activesupport/lib/active_support/hash_with_indifferent_access.rb#L253-L255 (https://github.com/rails/rails/pull/27392).
So, upgrading to Rails 4.2.9 would also solve this issue without having to change our code.
In Rails 5.0.5, ActionController::Parameters#compact
and ActionController::Parameters#compact!
are called from method_missing
directly on the underlying @parameters
which is a ActiveSupport::HashWithIndifferentAccess
while in Rails 4.2.9, ActionController::Parameters
is inheriting from ActiveSupport::HashWithIndifferentAccess
.
In Rails 5.0.5, ActionController::Parameters#compact
and ActionController::Parameters#compact!
are deprecated:
DEPRECATION WARNING: Method compact is deprecated and will be removed in Rails 5.1, as `ActionController::Parameters` no longer inherits from hash. Using this deprecated behavior exposes potential security problems. If you continue to use this method you may be creating a security vulnerability in your app that can be exploited. Instead, consider using one of these documented methods which are not deprecated: http://api.rubyonrails.org/v5.0.5/classes/ActionController/Parameters.html (called from irb_binding at (irb):3)
So we'll have to solve this with another solution in the future...
Why was this MR needed?
Screenshots (if relevant)
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated -
API support added - Tests
-
Added for this feature/bug -
All builds are passing
-
- Review
-
Has been reviewed by UX -
Has been reviewed by Frontend -
Has been reviewed by Backend -
Has been reviewed by Database
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Branch has no merge conflicts with master
(if it does - rebase it please) -
Squashed related commits together
What are the relevant issue numbers?
Closes #35769 (closed)