convert non-string default params to strings in the router.

[senny]

Closes #9435.

Since Rails 4.0.0.beta1 Rails raises an exception when non-string default params
are provided in the router (eg. root to: 'main#posts', page: 1).

I see sevaral options to fix this:

1. issue a deprecation warning when non-string defaults are used.
2. convert all default params to strings.
3. get the router to work with non-string params (implemented by this patch).

I chose 3. becuase it's the only option, which is backwards compatible.
With 3.2.x the router happily accepted non-string default params.

[senny]

@rafaelfranca I'll need to move the Changelog entry into the unreleased section.

[senny]

@pixeltrix could you take a look?

[pixeltrix]

@senny my only concern is why is this error occurring - the params value should only contain path parameters which should be strings (It should be accessing the environment key action_dispatch.request.path_parameters). Have you reproduced this in a simple Rails 4 app and does your test fail without the change?

[senny]

@pixeltrix yes the test does fail indeed with the same exception as described in the referenced issue. As the title of the PR suggest my first implementation used to convert all params to strings but there were other unpleasant side-effects. I agree that the default's should always be strings the thing is, in 3.2.x it worked the way it does after this patch.

[senny]

@pixeltrix how would you like to proceed on this one?

[pixeltrix]

@senny I'm investing this at the moment to see why there are non-string params in path parameters

[senny]

They are coming directly from the route definition and are added in this method: https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/routing/mapper.rb#L150-L171

I don't think a normal request ever produces other values than strings but the defaults you can supply in the routes file can have different types and 3.2.x passed them through as is.

[pixeltrix]

Can you change this to:

*   Skip valid encoding checks for non-String parameters that come from the matched route's defaults.

[pixeltrix]

Surprised this hasn't come up before we released beta1 - the responsible commit is quite old: 3fc561a

[senny]

@pixeltrix sometimes I am really surprised how long a particular bug will reside in rails without being noticed. I updated the CHANGELOG and pushed a rebased version.

[pixeltrix]

@senny can you edit the commit message so it's less about what we could do and just explain what the commit does - thanks.

[senny]

used the commit message you suggested.

[pixeltrix]

Sorry, my bad - it looks like GitHub uses the original commit message on the Commits tab for a PR even after you force push an update.

[senny]

@pixeltrix ah no you were totally right ;) I replaced the message and made the comment afterwards. Thanks for the good review.

[pixeltrix]

@senny I see what happened - the GH dynamic updating of comments on the PR showed me your new comment but didn't update the commits tab.