Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[api] Merge! is deprecated #2354

Merged
merged 2 commits into from Nov 23, 2016
Merged

[api] Merge! is deprecated #2354

merged 2 commits into from Nov 23, 2016

Conversation

Ana06
Copy link
Member

@Ana06 Ana06 commented Nov 18, 2016

Method merge! is deprecated and will be removed in Rails 5.1, as 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.0.1/classes/ActionController/Parameters.html

@Ana06
Copy link
Member Author

Ana06 commented Nov 18, 2016

@hennevogel
Copy link
Member

hennevogel commented Nov 18, 2016

Looks like you broke requests...

Method merge! is deprecated and will be removed in Rails 5.1, as  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.0.1/classes/ActionController/Parameters.html
@Ana06
Copy link
Member Author

Ana06 commented Nov 21, 2016

I've fixed it, but I still don't know why it was failing before. With this code:

roles = params[:roles].split(',') if params[:roles]
types = params[:types].split(',') if params[:types]
states = params[:states].split(',') if params[:states]
review_states = params[:reviewstates].split(',') if params[:reviewstates]
ids = params[:ids].split(',').map { |i| i.to_i } if params[:ids]
params = params.merge({states: states, types: types, review_states: review_states, roles: roles, ids: ids})

I get this in the log when running the tests:

[e4d32cd3-66fb-4999-82cf-4ebec35403e6] [4842:-200846929.83] Completed 500 Internal Server Error in 0ms (ActiveRecord: 0.0ms | Backend: 0.0ms | XML: 0.0ms)
[e4d32cd3-66fb-4999-82cf-4ebec35403e6] [4842:-200846929.83]   
[e4d32cd3-66fb-4999-82cf-4ebec35403e6] [4842:-200846929.83] NoMethodError (undefined method `merge' for nil:NilClass):
[e4d32cd3-66fb-4999-82cf-4ebec35403e6] [4842:-200846929.83]   
[e4d32cd3-66fb-4999-82cf-4ebec35403e6] [4842:-200846929.83] app/controllers/request_controller.rb:37:in `render_request_collection'
[e4d32cd3-66fb-4999-82cf-4ebec35403e6] [4842:-200846929.83] app/controllers/request_controller.rb:16:in `index'
[e4d32cd3-66fb-4999-82cf-4ebec35403e6] [4842:-200846929.83] config/initializers/wrap_parameters.rb:38:in `call'
[e4d32cd3-66fb-4999-82cf-4ebec35403e6] [4842:-200846929.83] test/test_helper.rb:184:in `process_with_kwargs'
[e4d32cd3-66fb-4999-82cf-4ebec35403e6] [4842:-200846929.83] test/functional/maintenance_test.rb:1295:in `test_create_maintenance_project_and_release_packages'

Although params is not nil. Why does it say that is nil? any clue?

@hennevogel hennevogel added the Frontend Things related to the OBS RoR app label Nov 22, 2016
@hennevogel
Copy link
Member

LGTM

@hennevogel hennevogel merged commit 7cadf70 into openSUSE:master Nov 23, 2016
@Ana06
Copy link
Member Author

Ana06 commented Nov 24, 2016

In case that someone is curious.

I've fixed it, but I still don't know why it was failing before. [...] Although params is not nil. Why does it say that is nil? any clue?

This is how ruby works: if you have a method called foo and you defined a local variable called foo assigning the value of foo with something the foo in the right side of the expression will be the local variable in the left side.

$ irb
>> def foo
>>   {}
>>   end
=> :foo
>> foo = foo.merge(a: 1)
NoMethodError: undefined method `merge' for nil:NilClass
    from (irb):4
    from /Users/rafaelfranca/.rbenv/versions/2.3.1/bin/irb:11:in `<main>'

And params is a method, so it was wrong and this is the way to solve it (although I prefer the solution that has already been merged):

self.params = self.params.merge({states: states, types: types, review_states: review_states, roles: roles, ids: ids})

References:

@bgeuken
Copy link
Member

bgeuken commented Nov 24, 2016

Great that you took the effort to figure out the root cause. Well done

@Ana06 Ana06 deleted the merge branch October 6, 2018 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend Things related to the OBS RoR app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants