Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Stringify all params values #82

Closed
homakov opened this Issue · 9 comments

5 participants

@homakov

Rails Params Parsing is magic. You never know what it can lead to. Nested hash in params[:id], Array full of nils etc.
1. Array stuffed with nils.
User.find_by_token(params[:token]) can find a user with empty token if we provide params[:token]=[nil] or [1,nil]. It was possible half a year ago, and then 'reinvented' with JSON/XML params parsers.
2. Nested hashes - same problem would happen if we could get symbols in keys in {"select"=>"code"}.
3. Did you know the trick with bruteforcing?: instead of passing ?token=abc you can pass ?token[]=abc2&token[]=abc2... Or for example username[]=testingfirst&username[]=testingsecond.. in find_by_username_and_password(params[:username], sha(params[:password))

Thus we need a reliable tool to get exact structure of params we asked for. Some people suggest to stringify it in ActiveRecord and find_by_ should accept only strings - i think it's just killing useful tool for no reason.
Maybe strong_params should control all incoming params(before_filter), not only for mass assignment purposes? e.g.

class PeopleController < ActionController::Base
  params.require(:person).permit(:name, :age)

  def search
    Person.find_by_age(params[:person][:age]) #if we pass ?person[age][]=1&person[age][]=2&person[age][]=3 it will be casted to "[1,2,3]"

If developer needs nested structures he will specify it explicitly in controller. Not so agile, but user input is the last thing that should be agile. will fix rails/rails#8831 and possible future vulnerabilities in params, once and for all.

P.S I might be wrong thinking developer expects static structure - please point out if there are use cases with agile and random structure

@dhh
Owner
@goshakkk

I am just curious, how awkward it will be to do the following with this stuff:

Suppose I allow users to allow create web portals and add quizzes to them. I am going to allow portal creation with quizzes with questions with options all in one form. Using your proposed solution, how would I tell that portal can have multiple quizzes, and quizzes have multiple questions, which, in turn, what multiple options? Oh, and depending on type of questions there might be other extra-nested resources. (Yes, I had to do similar app in the past for real.)

Would it be something like this?

params.require(:portal).permit(:title, :slug, :description,
  params.nested(:quizzes).permit(:title, :info, :max_score,
    params.nested(:questions).permit(:title,
      params.nested(:options).permit(:value, :score)))))
@homakov

Particularly "default permit(:age) will only return a string" is easy to achieve, just add .to_sym at the end of https://github.com/rails/strong_parameters/blob/master/lib/action_controller/parameters.rb#L47
What i wonder about is built-in(pseudo code)

before_filter :to_strong_params

def to_strong_params
  £casting :id to string, person[name] and person[age] to strings
  params = params.require(:person).permit(:name, :age)
end
@homakov

@goshakkk permit accepts nested arguments now. This structure is long - so definition is long too. Dont expect magic tool, many rules lead to many lines

@dhh
Owner
@svs

a DSL for parameter parsing does not give the programmer enough space to properly specify what the params should look like. Better to create an object with the params and do validations on it in the comfort of its own class. i.e.

class QuizesController < ActionController::Base
  def create
    @params = QuizesCreateParams.new(params).sanitize!
     @quiz = Quiz.new(@params)
    .....

end

More here: http://svs.io/post/38090231306/param-objects-in-rails

@fxn
Owner
fxn commented

Can we close this one after the introduction of permitted scalars?

@homakov homakov closed this
@fxn
Owner
fxn commented

Excellent, thanks Egor.

@homakov

Waiting for super strong params, thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.