generated URL helpers should accept AC::Parameters as an argument.

[jcoyne]

In 5.0.0.beta1

opts = { foo: 'bar' }
=> {:foo=>"bar"}
Rails.application.routes.url_helpers.root_path(opts)
=> "/?foo=bar"

parameters = ActionController::Parameters.new(foo: 'bar')
=> {"foo"=>"bar"}
parameters.permit!
=> {"foo"=>"bar"}
Rails.application.routes.url_helpers.root_path(parameters)
=> "/"
Rails.application.routes.url_helpers.root_path(parameters.to_h)
=> "/?foo=bar"

I don't think we should expect the same behavior from the last two invocations of the route helper.

[jcoyne]

I now wonder where the non-parameter arguments to route helpers (i.e. only_path: true) should go? A second argument? Like this?

Rails.application.routes.url_helpers.root_url(parameters, only_path: true)

[jcoyne]

I don't think this is something that necessarily has to be fixed. I just seemed a bit odd. I'll close. Reopen if you think it's important.

[rafaelfranca]

In fact it should raise an exception like we do in others case. It seems to be a missing case of #20797

[prathamesh-sonpatki]

@rafaelfranca Do you mean without calling parameters.permit!, if we call Rails.application.routes.url_helpers.root_path(parameters) it should raise error?

[rafaelfranca]

Yes. We expect users to filter the parameters before passing to url helpers.

[maletor]

Yes. We expect users to filter the parameters before passing to url helpers.

@rafaelfranca, really? This doesn't seem like a sane default. Why should the user's application be responsible for maintaining the list of special options that change the URL.

There are people upgrading rails 4 to 5 that have root_url(params) that will change that to root_url(params.permit!) or root_url(params.to_unsafe_h).

Every app should convert that to

def safe_params
  params.except(:host, :port, :protocol, :subdomain, :etc).to_unsafe_h
end

Is it unreasonable for safe_params to be part of the rails API?

[maletor]

Also, ActionController::Parameters to_unsafe_h will always return a string for Hash keys and url_for requires symbols to override the dangerous stuff like :host so in the end the whole thing is kinda moot, don't you think.