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

Wrong array to string coercion #91

Open
vpereira opened this issue Nov 28, 2022 · 4 comments
Open

Wrong array to string coercion #91

vpereira opened this issue Nov 28, 2022 · 4 comments

Comments

@vpereira
Copy link

vpereira commented Nov 28, 2022

Basic Info

  • rails_param Version: 1.3.0
  • Ruby Version: 3.1
  • Rails Version: 7.0

Issue description

We pass in our spec the following:

req[:orders_attributes][0][:email] = %w[[email protected] [email protected]]

And in our controller we validate this param with a:

   order.param! :email, String

With rails_param (0.11.2) and rails 6 the coercion happens. With rails_param (~> 1.3.0) it doesn't

Extract from pry session while running the spec:

RAILS 6

[3] pry(#<Api::ImportController>)> order.params["email"]
=> ["[email protected]", "[email protected]"]
[4] pry(#<Api::ImportController>)> 

[3] pry(#<Api::ImportController>)> order.param! :email, String
RailsParam::Param::InvalidParameterError: '["[email protected]", "[email protected]"]' is not a valid String
from /home/foo/.bundle/ruby/3.1.0/gems/rails_param-0.11.2/lib/rails_param/param.rb:134:in `rescue in coerce'
Caused by ArgumentError: ArgumentError
from /home/foo/.bundle/ruby/3.1.0/gems/rails_param-0.11.2/lib/rails_param/param.rb:106:in `coerce'

RAILS 7 and the gem 1.3.0, the exception doesn't get raised

[1] pry(#<Api::ImportController>)> order.params["email"]
=> ["[email protected]", "[email protected]"]
[2] pry(#<Api::ImportController>)> order.param! :email, String
=> "[\"[email protected]\", \"[email protected]\"]"
[3] pry(#<Api::Las::Scc::ImportController>)> 

For sure in that case I could simply validate by format, but I have more than 30 unique fails, since this is validated in many different parameters.

Steps to reproduce

I think passing an array like %w[[email protected] [email protected]] or as nested attributes like in my case (req[:orders_attributes][0][:email] ) having he validation as foo.param! :email, String should do the trick:

@iMacTia
Copy link
Collaborator

iMacTia commented Dec 20, 2022

Thank you for reporting this @vpereira and apologies for the slow reply.
Just to confirm: does the Rails version make any difference?
Am I right to understand rails_param 0.11.x works in both Rails 6/7, while rails_param 1.x fails for both?

I won't have time to look into this anytime soon, but maybe @ifellinaholeonce can help in the meantime

@vpereira
Copy link
Author

Am I right to understand rails_param 0.11.x works in both Rails 6/7, while rails_param 1.x fails for both?

hi @iMacTia

you are right here!

@ifellinaholeonce
Copy link
Collaborator

Thanks for bringing this up! I'll be able to take a look at this but likely not until next week.

To confirm, the behaviour from 0.11.x was that a param defined as a String (ie. foo.param! :email, String) can be given an array and each item in the array will get that validation?

@vpereira
Copy link
Author

hi @ifellinaholeonce,

I dont have access to the codebase right now, but as fair as i remember, the array shouldnt be valid, regardless of what is inside. email should be a string and not an array. So the exception should be raised, and not automatically have a string being coerced to array. I don't think the validation was happening for each element of the array. If was the case, it would still be an undocumented and maybe dangerous, feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants