Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  Chris Barnett August 19th, 2008 @ 05:09 AM

  • Tag changed from “2.1, activerecord, bug, serialize” to “2.1, activerecord, bug, edge, serialize”

  I can confirm this test fails for both 2.1 and edge.

  The reason it fails is that attribute serialization was not written to support serialization of Strings.

  activerecord/lib/active_record/base.rb: (edge line 2769)

  
        def object_from_yaml(string)
          return string unless string.is_a?(String)
          YAML::load(string) rescue string
        end
  

  Despite the name, this method is passed whatever you assign to a serialized attribute, in it's unserialized form. So in the test above, object_from_yaml("Yes") is called, which returns true because "Yes" is a String, and YAML::load translates "Yes" to true.

  My guess would be that to fix this, somewhere further up the method call chain, there needs to be a decision to not try to unserialize values that haven't been serialized yet.

  I can try to write a patch, but I'm a rails hacking n00b, and I'm not sure if it's really, really worth supporting the use case of serializing a String.

  On the other hand, it might be worth fixing because the problem also affects serialization of String subclasses.

• You flagged this item as spam.
  

  Andreas Korth August 19th, 2008 @ 09:54 AM

  Thanks, Chris.

  Now that I know where things go wrong, I can probably create a patch.

  It might not make a whole lot of sense to serialize a String, but in my case the attribute value can be either a String, or an Array, or a Hash.

  This is definitely a Rails bug, since YAML::load(YAML::dump("Yes")) == "Yes".

  I'll work around this for now, hoping that someone from core will pick it up. If not, I'll schedule it as a task for our next iteration.

  Thanks again for figuring this out.

• You flagged this item as spam.
  

  Peter Wagenet August 23rd, 2008 @ 08:02 PM

  I think I have a pretty simple fix for this. I'll post it as soon as I get a couple things sorted out.

• 

  Peter Wagenet August 23rd, 2008 @ 08:52 PM

  • no changes were found...

  • serialized_strings.diff 1.5 KB

• You flagged this item as spam.
  

  Peter Wagenet August 23rd, 2008 @ 08:52 PM

  • Tag changed from “2.1, activerecord, bug, edge, serialize” to “2.1, activerecord, bug, edge, patch, serialize, tested”

  Here we go.

• 

  DHH September 10th, 2008 @ 06:06 AM

  What's the use case for serializing strings?

• 

  Tarmo Tänav September 10th, 2008 @ 06:53 AM

  For me the case is to have an answer attribute that can hold any type of value for dynamically built forms. So depending on the field type it may contain, a string, a datetime, an array of selected options, a ranking order... And this means that string too must be serialized for there to not be any unintended unserializings of certain string values.

• You flagged this item as spam.
  

  Andreas Korth September 10th, 2008 @ 08:18 AM

  My use case is the same as Tarmo's. Serializing a string alone doesn't make sense, but whenever the attribute value can be of variable type, the bug might hit you.

• You flagged this item as spam.
  

  Peter Wagenet October 26th, 2008 @ 01:48 AM

  Out of curiosity, is this getting any attention?

• 

  Repository October 27th, 2008 @ 04:17 PM

  • State changed from “new” to “committed”

  (from [c94ba8150a726da4a894cd8325ee682a3286ec9f]) Fixed that serialized strings should never be type-casted (i.e. turning "Yes" to a boolean)(Andreas Korth) [#857 state:committed] http://github.com/rails/rails/co...

• You flagged this item as spam.
  

  viktor tron (strawberry) October 22nd, 2009 @ 12:30 PM

  We should reopen this issue.
  It took me hours to figure out why serialization fails on my object.
  It was because YAML refuses to deserialize a Method,
  but the object_from_yaml function rescues this and falls back to the unserialized string value
  if you constrained the serialization for a type, then the object will fail with Type Mismatch.

  suppose attr on object s is serialized as a Hash
  this is what you get.
  

  s.attr ActiveRecord::SerializationTypeMismatch: attr was supposed to be a Hash, but was a String
  s.instance_eval { object_from_yaml(@attributes['attr']) }.class => String s.instance_eval { YAML::load(@attributes['attr']) }.class TypeError: allocator undefined for Method
  

      from /usr/lib/ruby/1.8/yaml.rb:133:in `transfer'
  

  Very annoying that on the s.attr call the real problem is masked because of the rescue in object_from_yaml.

  While in this case it is just annoying, imagine the case mentioned by some of you when you serialize because of type polymorphism.
  In that case the string will be a legitimate type (No type mismatch error), and we will not fail, but instead of an object we get the serialized string of an object.
  This is a security risk since it could display confidential info stored in the serialized objects attributes which you never intended to display for instance.

  The real problem is of course a bug in YAML which should refuse to dump objects which it cannot load
  but that is another issue....

  I suggest removing the rescue string

• You flagged this item as spam.
  

  viktor tron (strawberry) October 22nd, 2009 @ 12:41 PM

  sorry for awkward formatting, I resubmit.

  We should reopen this issue.
  It took me hours to figure out why serialization fails on my object.
  It was because YAML refuses to deserialize a Method,
  but the object_from_yaml function rescues this and falls back to the unserialized string value
  if you constrained the serialization for a type, then the object will fail with Type Mismatch.

  suppose attr on object s is serialized as a Hash
  this is what you get.

      >> s.attr
       ActiveRecord::SerializationTypeMismatch: attr was supposed to be a Hash, but was a String
      >> s.instance_eval { object_from_yaml(@attributes['attr']) }.class 
      => String 
      >> s.instance_eval { YAML::load(@attributes['attr']) }.class 
        TypeError: allocator undefined for Method
       from /usr/lib/ruby/1.8/yaml.rb:133:in `transfer'
  

  Very annoying that on the s.attr call the real problem is masked because of the rescue in object_from_yaml.

  While in this case it is just annoying, imagine the case mentioned by some of you when you serialize because of type polymorphism.
  In that case the string will be a legitimate type (No type mismatch error), and we will not fail, but instead of an object we get the serialized string of an object.
  This is a security risk since it could display confidential info stored in the serialized objects attributes which you never intended to display for instance.

  The real problem is of course a bug in YAML which should refuse to dump objects which it cannot load
  but that is another issue....

  I suggest removing the rescue string