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

Nested params protection triggers 'undefined method `each' for "lorem ipsum":String' #1776

Closed
joaodiogocosta opened this Issue Sep 24, 2014 · 5 comments

Comments

Projects
None yet
2 participants
@joaodiogocosta

joaodiogocosta commented Sep 24, 2014

I'm getting a 'undefined method each for "lorem ipsum":String' while trying to submit a form to create a Post. The error comes from file: params_protection.rb location: filter_params! line: 98. I think this is related to nested params protection.

# controller
post :create, params: [post: [:title, :subtitle]] do
    @post = current_user.posts.build(params[:post])

    if @post.save
      redirect url(:posts, :index)
    else
      render 'new'
    end
  end
# view/form
<% form_for @post, url(:posts, :create), id: "posts-new-form" do |f| %>
  <%= f.text_field :title, placeholder: "Title", autofocus: true %>
  <%= f.text_field :subtitle, placeholder: "URL" %>

  <%= f.submit "Save", class: "button" %>
<% end %>

I was able to make it work using:

post :create, params: [:post => proc { |p| p.select { |k, v| [:title, :subtitle].include?(k.to_sym) } }] do
    @post = current_user.posts.build(params[:post])

    if @post.save
      redirect url(:posts, :index)
    else
      render 'new'
    end
  end

I tried with padrino 0.12.3, 0.12.2, 0.11.4 and the master branch.

Any thoughts on this?

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc Sep 24, 2014

Member

Can not reproduce the issue on 0.12.3 and on the edge. Please provide a minimal failing project with the locked Gemfile.

Member

ujifgc commented Sep 24, 2014

Can not reproduce the issue on 0.12.3 and on the edge. Please provide a minimal failing project with the locked Gemfile.

@ujifgc ujifgc added the revalidate label Sep 24, 2014

@joaodiogocosta

This comment has been minimized.

Show comment
Hide comment
@joaodiogocosta

joaodiogocosta Sep 27, 2014

I found the issue. The code snipped I posted shows a Post model, which I used to illustrate the issue, but in fact my real model name is Gotta. It happens that padrino doesn't know how to pluralize "gotta":

"gotta".pluralize
=> "gotta"

And the problem might be related to the line 7 on this method:

# padrino-framework/padrino-core/lib/padrino-core/application/params_protection.rb`
(...)

def filter_params!(params, allowed_params)
        params.each do |key,value|
          type = allowed_params[key]
          next if value.kind_of?(Array) && type
          case
          when type.kind_of?(Hash)
            if key == key.pluralize
              value.each do |array_index,array_value|
                value[array_index] = filter_params!(array_value, type)
              end
            else
              params[key] = filter_params!(value, type)
            end
          when type == Integer
            params[key] = value.empty? ? nil : value.to_i
          when type.kind_of?(Proc)
            params[key] = type.call(value)
          when type == true
          else
            params.delete(key)
          end
        end
      end

(...)

Entering that if clause because "gotta" == "gotta".pluralize evaluates to true.

Is "gotta" some kind of reserved word? Shouldn't the inflector add a 's' by default on words it doesn't recognize?

I was able to fix this by adding an initializer with and inflection, but I think I'll rename the model to avoid further issues.

joaodiogocosta commented Sep 27, 2014

I found the issue. The code snipped I posted shows a Post model, which I used to illustrate the issue, but in fact my real model name is Gotta. It happens that padrino doesn't know how to pluralize "gotta":

"gotta".pluralize
=> "gotta"

And the problem might be related to the line 7 on this method:

# padrino-framework/padrino-core/lib/padrino-core/application/params_protection.rb`
(...)

def filter_params!(params, allowed_params)
        params.each do |key,value|
          type = allowed_params[key]
          next if value.kind_of?(Array) && type
          case
          when type.kind_of?(Hash)
            if key == key.pluralize
              value.each do |array_index,array_value|
                value[array_index] = filter_params!(array_value, type)
              end
            else
              params[key] = filter_params!(value, type)
            end
          when type == Integer
            params[key] = value.empty? ? nil : value.to_i
          when type.kind_of?(Proc)
            params[key] = type.call(value)
          when type == true
          else
            params.delete(key)
          end
        end
      end

(...)

Entering that if clause because "gotta" == "gotta".pluralize evaluates to true.

Is "gotta" some kind of reserved word? Shouldn't the inflector add a 's' by default on words it doesn't recognize?

I was able to fix this by adding an initializer with and inflection, but I think I'll rename the model to avoid further issues.

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc Sep 28, 2014

Member
irb(main):004:0> 'gotta'.singularize
=> "gottum"
irb(main):005:0> 'gottum'.pluralize
=> "gotta"

Some kind of Latin word apparently. Padrino does not handle inflections, it's ActiveSupport's job.

Member

ujifgc commented Sep 28, 2014

irb(main):004:0> 'gotta'.singularize
=> "gottum"
irb(main):005:0> 'gottum'.pluralize
=> "gotta"

Some kind of Latin word apparently. Padrino does not handle inflections, it's ActiveSupport's job.

@ujifgc ujifgc removed the revalidate label Sep 28, 2014

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc Sep 28, 2014

Member

@joaodiogocosta please tell what exact code did you use to define your post route and what data did you post? I would like to understand the thoughts behind it and possibly improve documentation.

Member

ujifgc commented Sep 28, 2014

@joaodiogocosta please tell what exact code did you use to define your post route and what data did you post? I would like to understand the thoughts behind it and possibly improve documentation.

@joaodiogocosta

This comment has been minimized.

Show comment
Hide comment
@joaodiogocosta

joaodiogocosta Sep 28, 2014

Yup.

# Params
{ gotta: { title: "My First Gotta", subtitle: "Just created my first gotta" } }



# Route
post :create, params: [ gotta: [ :title, :subtitle ] ] do
  @gotta = current_user.gottas.build(params[:gotta])
  if @gotta.save
    redirect_to url(:gottas, :index)
  else
    render 'new'
  end
end

joaodiogocosta commented Sep 28, 2014

Yup.

# Params
{ gotta: { title: "My First Gotta", subtitle: "Just created my first gotta" } }



# Route
post :create, params: [ gotta: [ :title, :subtitle ] ] do
  @gotta = current_user.gottas.build(params[:gotta])
  if @gotta.save
    redirect_to url(:gottas, :index)
  else
    render 'new'
  end
end

@ujifgc ujifgc closed this in c33fb38 Sep 30, 2014

ujifgc added a commit that referenced this issue Sep 30, 2014

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