The key for the errors hash is a string, not a symbol in Rails 4

[zonotope]

I've been trying to integrate rails-prg into my app, and I've been debugging a nil pointer issue with the load_redirected_objects callback. I've isolated the problem to an attempt to reference the redirected instance's errors hash with a symbol instead of a string here. After stepping through the request, it looks like what's happening is that when the relevant model instance is serialized into the flash with set_redirected_object!, both the :error and :params symbol keys are converted to string. I have verified that if I change load_redirected_objects! to access the errors and params sub-hashes with string keys instead of symbol keys, that things work as expected. Would you accept a patch with this small change and, if not, is there something less invasive that I can do to get this to work properly?

[tommeier]

Hi @zonotope, sorry for the difficulties! Out of interest what Ruby version are you using? Perhaps this is the cause of the difference. I'm not sure converting it to string keys will be the answer as it is working for me (tm) right now with symbols, we could however run a .symbolize_keys on the hash to ensure identical usage across ruby versions (if that is the difference).

It could also be your Rails version, but I'm also using Rails 4 and this isn't a problem, could you .inspect the attributes and check if it is a http://api.rubyonrails.org/classes/ActiveSupport/HashWithIndifferentAccess.html object?

[zonotope]

Thanks a lot!

my rails version is: Rails 4.1.6, my ruby version is: ruby 2.1.2p95 (2014-05-08 revision 45877) [x86_64-darwin13.0]. I put in debug statements both before I call set_redirected_object! in my code as well as inside the load_redirected_objects! method defined in the library. The params that I set in my code is in fact a HashWithIndifferentAccess, however, by the time it gets into the library, it changes into a plain old Hash. I'm not sure what could cause the differences here.

[zonotope]

i also think that .symbolize_keys is a much better idea than changing to a string because that should always just work.

[tommeier]

Great! I think it might be time to add Rails version specific test runs too, will help find these issues in future, I'll try to get to this asap, could you trial .symbolize_keys and let me know if you hit any other issues?

Its a little worrying it converted to a normal hash, might need to run a .deep_symbolize_keys but i'd like to try and avoid that to ensure no performance hit.

[zonotope]

Hi again @tommeier. Sorry for the delay. I took a break from this particular project, but I'm back on it now. Calling .symbolize_keys! on the attributes hash solved my initial problem, but it revealed another one.

In load_redirected_objects!, set_params_to_redirected_instance is called with the redirected object and the params sub-hash. Before the passed in parameters are set, there's a guard method that ensures that the passed in params are both an instance of ActionController::Parameters and that they are safe here.

The problem is that this, just like the attributes hash, is just a plain old hash. The parameters are stored in the session between requests, and according to this, the server might not be able to reconstruct complex objects from the session that were stored across requests. I'm not sure if ActionController::Parameter instances are too "complex" for the server to reconstruct, but it would explain the issue that I'm seeing.

Do you have any ideas about how to get around this?

[tommeier]

@zonotope interesting, what session store are you using?

[zonotope]

We're using the cookie store, and it all makes sense now. That's why it's always just a plain Hash on read.

[tommeier]

aha! Yes, that will be it @zonotope , We're using a database session store (mostly for security reasons) and that would explain the difference in behaviour. This might be a bit trickier to resolve, I'll need to spend some time thinking over any potential security implications over passing the values via cookie based session before jumping into any changes.

[zonotope]

yeah, it is definitely riskier to store the data in cookies, but it's probably fine. in any case, i think we can safely switch over to the active record store in the meantime since our app isn't going to have to face too much traffic initially. thanks a lot.

[tommeier]

Fantastic thanks @zonotope , I'll leave this open as I definitely want to think this through and come up with a cookie friendly solution.

[tommeier]

@zonotope one possible solution would be to use :marshal as the cookie serialisation option:

Rails.application.config.action_dispatch.cookies_serializer = :marshal