url_for doesn't like HashWithIndifferentAccess

+1

I'm currently using symbolize_keys to skip this problem.

Protip: you're going to need some tests to show exactly what it fixes.

But other than that, it does indeed work and fix a few other bugs that people have been experiencing - like the following: #4419 current_page? in a partial template does not work.

Add a few test cases and then it'll get accepted.

I'll take more of a look at this tomorrow, and can guide you a little if you want it. Sorry for being condescending earlier.

No prob. If there's any tests that need writing I can give it a shot... I just figured that this patch, although working, probably wasn't the best way of doing things as it seems to duplicate code by calling doing to_hash.symbolize_keys in two different places. I was just kind of hoping that someone more familiar with the code would know of a better fix, but if this patch is acceptable then all the better. I can write a few tests if it will help, although I don't really know how extensive the coverage should be if this patch affects other tickets.

Lenary, did you take a look at this ticket yet? What did you think of it?

It was not clear to me if this problem is only in rails3 branch or in 2-3-stable branch too. It turns out that 2-3-stable branch works only. This is an issue only with rails3 branch. Adding this comment so that it to clear to others who look at this ticket.

Someone should put right tag. There are so many of them related to rails3 that I am not sure which one I should apply.

This patch still doesn't feel right IMHO. using hashwithindifferent access should mean that we don't get these issues and don't have to call to_hash.symbolize_keys on the hash to get this to work. I'm going to have a much closer look later today

Can we not apply this patch until then. This to me kind-of feels like were barking up the wrong tree, but I need time to look at it properly.

Jay, thanks for all the legwork. I'll take a proper look over what's happening and propose a few solutions in this thread later today.

I think this is a better way to solve it.
symbolize_keys on hash with indifferent access for me should return a hash with symbolized keys.

Deeper on the framework things doesn't expect a HWIA so we need to convert on the helper and that's what symbolize_keys should do

Yeah, this appears to be the behaviour in 2.3. In 2.3, calling symbolize_keys or stringify_keys returns an actual Hash with the appropriate keys, whereas in 3.0 they return a HWIA:

# in 2.3:
>> HashWithIndifferentAccess.new({ :one => :two }).stringify_keys.class
=> Hash
>> HashWithIndifferentAccess.new({ :one => :two }).symbolize_keys.class
=> Hash

# in 3:
irb(main):005:0> HashWithIndifferentAccess.new({ :one => :two }).stringify_keys.class
=> ActiveSupport::HashWithIndifferentAccess
irb(main):006:0> HashWithIndifferentAccess.new({ :one => :two }).symbolize_keys.class
=> ActiveSupport::HashWithIndifferentAccess

That being the case and if symbolize_keys is to return a Hash with symbolized keys, then I suppose stringify_keys should also return a Hash with stringified keys, yeah?

I've added a test for the symbolize_keys method and re upload the patch.
Now looks good.

For me HashWithIndifferentAccess.new({ :one => :two }).stringify_keys.class should return ActiveSupport::HashWithIndifferentAccess
but we need to do the same on both versions i think.

I am just about to take a good look at this now. Sorry i haven't got around to it earlier.

I am no clearer on what is going on with this. I took a look at some raw output from url_for, and got some of the way with that, but then got to where you had all got to.

I have now tried santiago's patch, and it hasn't really worked for me (still getting controller and action params in querystring when in controller). is the latest one ( https://rails.lighthouseapp.com/projects/8994/tickets/4391/a/491493... ) the entire patch, or do i have to apply more than that?

(from [5c9c30ac65d25bd5a83d773739c2cd8fdd6da4d3]) url_for now works with HashWithIndifferentAccess ht jay [#4391 url_for doesn't like HashWithIndifferentAccess state:committed]

Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/5c9c30ac65d25bd5a83d773739c2cd...

The applied patch in http://github.com/rails/rails/commit/5c9c30ac65d25bd5a83d773739c2cd... is partially there, but url_for still fails with stringified keys in a controller.

I think that ActionDispatch::Routing::Generator#normalize_options! and #normalize_controller_action_id! are be the culprits. They both assume symbolized keys when they do their options merging, and end up creating options Hashes that sort of duplicate their entries because of it. In the cases where HWIAs are used this merging obviously goes a lot smoother, but in the case of regular Hashes with stringified keys things start to go awry.

For instance, in the controller given following:

url_for('controller' => 'c', 'action' => 'a')

This will pass a Hash with stringified keys to ActionDispatch::Routing::RouteSet.url_for. When normalize_options! gets a hold of it, it won't do much of anything because it specifically looks for symbolized keys and we aren't dealing with a HWIA at all.

In my set up, I'm calling url_for from MainController#index. normalize_controller_action_id! tries to combine the original options Hash with @recall, which is now set to {:controller => 'main', :action => 'index'}, thus using symbolized keys. The use_recall_for method uses Symbols to try and fill in any missing parameters, but it ends up duplicating the parameters since we're not using a HWIA. The options Hash now looks like the following:

{:controller=>"main", "action"=>"a", :action=>"index", "controller"=>"c"}

This later causes all sorts of problems when other methods like handle_nil_action! try to delete entries from the options Hash because while they may delete :action they'll be leaving behind 'action'. Eventually these duplicate keys seem to cause problems later on down the line in rack-mount and the like.

Part of the reason url_for is working with both stringified keys in views is because the keys are being symbolized long before they ever make it to normalize_options! and the like, whereas in ActionDispatch they are still being untouched, and thus you could slip HWIAs or Hashes with stringified keys in there. My original assumption that only HWIAs were causing the problem then is a little off -- it's actually more of a case of stringified keys.

Anyways, that's what I've managed to figure out. So, that being the case, another quick patch seems to be needed -- the controller version of url_for needs to make sure that it's passing along a symbolized Hash.

I've attached a patch with a couple of tests and it seems to be working now for me. Note that in this case I don't believe we'd need to call to_hash as we're actually calling symbolize_keys on url_options after the merging, and url_options is already a Hash.

(from [275e839b8dfa3cf2bdedf1b31302dec20ac96a46]) Ensure that url_for uses symbolized keys in the controller. [#4391 url_for doesn't like HashWithIndifferentAccess]

Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/275e839b8dfa3cf2bdedf1b31302de...

We have fixed this issue in another way now commits here http://github.com/rails/rails/compare/275e839...ebf9820