This project is archived and is in readonly mode.
OrderedHash mishandles keys
Reported by Brandon Keepers | December 31st, 2008 @ 03:46 PM | in 2.x
The latest implementation of OrderedHash does not behave as
expected when it comes to managing keys in several situations. For
example, #merge! doesn't work:
>> h = ActiveSupport::OrderedHash.new
>> h.merge! :a => 1, :b => 2
=> {:a=>1, :b=>2}
>> h.keys
=> []
>> h.inspect
=> "{:a=>1, :b=>2}"
Also, when calling non-destructive methods like
#reject it mangles the keys:
>> h = ActiveSupport::OrderedHash.new
>> h[:a] = 1
>> h[:b] = 2
>> h2 = h.reject {|k,v| k == :a }
=> {:b=>2}
>> h.inspect
=> "{:a=>1, :b=>2}"
>> h.keys
=> [:b]
Comments and changes to this ticket
-
Brandon Keepers December 31st, 2008 @ 03:49 PM
Attached is a patch that fixes these two issues, and probably a few more.
-
Frederick Cheung December 31st, 2008 @ 04:01 PM
- Tag changed from “activesupport, hash” to “activesupport, hash, patch”
Is the dup in initialize_copy not redundant given that keys already does a dup? Other than that looks sane to me.
-
Brandon Keepers December 31st, 2008 @ 05:10 PM
Yeah, it is redundant, but I thought it more clearly communicated the intent. I'm ok with removing it, I just didn't want people to be confused about it's just setting the instance variable to the value of keys.
-
Frederick Cheung December 31st, 2008 @ 05:22 PM
I'd remove it. If anything (for me at least) it adds confusion (' I thought the return value of keys was safe to mess around with but it's dup-ing it here, should I always dup it myself ? ').
There might be other bugs like this. When I rewrote OrderedHash a few weeks ago I was mostly worried about backwards compatibility with the existing implementation (when it was an array with knobs on) so I didn't worry to much about methods that didn't exist on the old OrderedHash.
-
-
Repository January 16th, 2009 @ 05:40 PM
- State changed from “new” to “resolved”
(from [452cd74d81ecfa38d0c7d3a4dc83d2b731de9e73]) Dup keys in OrderedHash to prevent them from being modified [#1676 state:resolved]
Signed-off-by: Frederick Cheung frederick.cheung@gmail.com http://github.com/rails/rails/co...
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Brandon Keepers
Attachments
Tags
Referenced by
-
1676
OrderedHash mishandles keys
(from [452cd74d81ecfa38d0c7d3a4dc83d2b731de9e73]) Dup key...