Skip to content
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

Try harder when deciding whether to add a new array element #1089

Merged
merged 1 commit into from
Jun 29, 2016

Conversation

matthewd
Copy link
Contributor

Only move to a new entry if the end key is taken; checking only the top-level key is insufficient.

This implementation is pretty unfortunate, but is the option that minimizes changes to normalize_params. I'm tempted to try for a more invasive (and correspondingly less tacked-on) alternative, though.

Fixes #1086, rails/rails#23997

cc @rthbound @bquorning @bmedenwald

Only move to a new entry if the end key is taken; checking only the
top-level key is insufficient.
@bmedenwald
Copy link

For what it's worth, this fix does successfully address all the use cases I reported.

@@ -135,6 +134,18 @@ def params_hash_type?(obj)
obj.kind_of?(@params_class)
end

def params_hash_has_key?(hash, key)
return false if key =~ /\[\]/
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm probably missing something, but it would seem since you are splitting on [], that the guard clause would ensure that [] exists in the query?

return false if key !~ /\[\]/

Then again, I probably just missed the nuance here.

Yay make rack better!

@tenderlove tenderlove merged commit 33a7cd5 into rack:master Jun 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants