Handle path_params gracefully when a user sends ?path_params=string

[martinemde]

In some cases it's possible for path_params to be passed into url_for by an end-user of a rails app. Kaminari currently triggers this.

It's also currently inconvenient to write a test to ensure this doesn't happen. Passing path_params: "string" in a functional test crashes the test runner. (see this example in a rails bug report template that crashes.)

A string value will cause a 500 error in rails internals. A hash ?path_params[inject]=string can be used to inject a path_param into the url generation. However, I don't believe there is any vulnerability since path_params are lower priority than actual params and get ignored when there are no matches. Also, given that the very similar #39616 did not raise alarm bells, I'm not considering this a vulnerability. Still, it is probably good to scrub this key. A vulnerability once existed for a similar problem: GHSA-r5jw-62xg-j433

Motivation / Background

I was trying to fix a 500 caused by a security scan of rubygems.org. An unknown researcher sent a huge list of params to many pages, snipped here:

This caused at least two 500 errors on rubygems.org, a minor annoyance but enough to potentially page someone on-call.

I tracked the actual issue down to kaminari for which I opened kaminari/kaminari#1123.

While I think that this should be fixed in kaminari, I also think rails should fix this because it is somewhat difficult to write a test to ensure that this key is handled well. (see bug report template linked at top)

This PR also saves one hash allocation and one merge for every URL generate that doesn't have a path_params: key (most of them?), but the option = option.dup might negate most of the benefit (it could be reconfigured to save that dup, most likely).

Detail

This Pull Request changes ActionDispatch::Journey::Formatter#generate to only extract and merge path_params when it is a hash.

This PR does not address in any way filtering path_params. I believe kaminari should do this. However, this addresses the crash when path_params is not a Hash and makes most url generations slightly more efficient.

Additional information

This is very similar to another fix by my colleague @simi submitted to rails, #39616, which I think also deserves attention for the same reason, if it is still applicable to current rails code.

I'm also patching this on rubygems.org in the meantime so we don't get needless 500s.

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
• Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
• Tests are added or updated if you fix a bug or add a feature.
• CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.