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

Fix broken Hashie::Hash#to_hash #133

Merged
merged 1 commit into from Mar 31, 2014

Conversation

mhuggins
Copy link
Contributor

Per my comment on the original pull request for this feature:

I'm running into an issue with this code. I'm trying to take a basic hash with string keys and convert it to a basic hash with symbol keys. Take the following example.

hash = {"id": 1, "name": "Bob"}
hash = Hashie::Hash[hash].to_hash(symbolize_keys: true)

This throws an exception in Hashie's to_hash method using the code provided here. The reason is that on line 15 the value of k is being changed from "id" or "name" to :id or :name respectively. Then on line 17, the call to self[k] returns nil since hash[:id] is not defined (only hash["id"] is defined), which then errors when trying to evaluate nil.each.

I think a proper fix would be to assign the key to a NEW variable instead. e.g.:

assignment_key = options[:symbolize_keys] ? k.to_sym : k.to_s
out[assignment_key] ||= []
self[k].each do |array_object|
  out[assignment_key] << (Hash === array_object ? array_object.to_hash : array_object)
end

Additionally, this fixes a similar issue even when :symbolize_keys is false (or not provided) to the #to_hash function. If a key is not already a string, then a similar error will be raised. There was not a test to cover this, so I added one.

@mhuggins
Copy link
Contributor Author

If the two tests I added to this PR are run on master right now, both will fail. Try for yourself. :)

@dblock dblock merged commit 4302d2d into hashie:master Mar 31, 2014
@dblock
Copy link
Member

dblock commented Mar 31, 2014

This has been merged, sorry for the delay. Thanks for contributing!

@mhuggins
Copy link
Contributor Author

No problem, thanks so much! 😄

jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Feb 6, 2015
${RUBY_PKGPREFIX}-hashie2.

## 2.1.2 (5/12/2014)

* [#169](hashie/hashie#169): Hash#to_hash will also convert nested objects that implement `to_hash` - [@gregory](https://github.com/gregory).

## 2.1.1 (4/12/2014)

* [#144](hashie/hashie#144): Fixed regression invoking `to_hash` with no parameters - [@mbleigh](https://github.com/mbleigh).

## 2.1.0 (4/6/2014)

* [#134](hashie/hashie#134): Add deep_fetch extension for nested access - [@tylerdooling](https://github.com/tylerdooling).
* Removed support for Ruby 1.8.7 - [@dblock](https://github.com/dblock).
* Ruby style now enforced with Rubocop - [@dblock](https://github.com/dblock).
* [#138](hashie/hashie#138): Added Hashie::Rash, a hash whose keys can be regular expressions or ranges - [@epitron](https://github.com/epitron).
* [#131](hashie/hashie#131): Added IgnoreUndeclared, an extension to silently ignore undeclared properties at intialization - [@righi](https://github.com/righi).
* [#136](hashie/hashie#136): Removed Hashie::Extensions::Structure - [@markiz](https://github.com/markiz).
* [#107](hashie/hashie#107): Fixed excessive value conversions, poor performance of deep merge in Hashie::Mash - [@davemitchell](https://github.com/dblock), [@dblock](https://github.com/dblock).
* [#69](hashie/hashie#69): Fixed assigning multiple properties in Hashie::Trash - [@einzige](https://github.com/einzige).
* [#100](hashie/hashie#100): IndifferentAccess#store will respect indifference - [@jrochkind](https://github.com/jrochkind).
* [#103](hashie/hashie#103): Fixed support for Hashie::Dash properties that end in bang - [@thedavemarshall](https://github.com/thedavemarshall).
* [89](hashie/hashie#89): Do not respond to every method with suffix in Hashie::Mash, fixes Rails strong_parameters - [@Maxim-Filimonov](https://github.com/Maxim-Filimonov).
* [#110](hashie/hashie#110): Correctly use Hash#default from Mash#method_missing - [@ryansouza](https://github.com/ryansouza).
* [#120](hashie/hashie#120): Pass options to recursive to_hash calls - [@pwillett](https://github.com/pwillett).
* [#113](hashie/hashie#113): Fixed Hash#merge with Hashie::Dash - [@spencer1248](https://github.com/spencer1248).
* [#99](hashie/hashie#99): Hash#deep_merge raises errors when it encounters integers - [@defsprite](https://github.com/defsprite).
* [#133](hashie/hashie#133): Fixed Hash##to_hash with symbolize_keys - [@mhuggins](https://github.com/mhuggins).
* [#130](hashie/hashie#130): IndifferentAccess now works without MergeInitializer - [@npj](https://github.com/npj).
* [#111](hashie/hashie#111): Trash#translations correctly maps original to translated names - [@artm](https://github.com/artm).
* [#129](hashie/hashie#129): Added Trash#permitted_input_keys and inverse_translations - [@artm](https://github.com/artm).
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.

None yet

2 participants