This project is archived and is in readonly mode.

#857 ✓committed
Andreas Korth

Serialized attribute values "Yes" or "No" get coerced into booleans

Reported by Andreas Korth | August 19th, 2008 @ 01:18 AM | in 2.x

When setting a serialized attribute to "Yes" or "No" will set the attribute to true or false, respectively.

Test case:



class Thing < ActiveRecord::Base
  serialize :name, String
end

class ThingTest < ActiveSupport::TestCase
  def test_name_serialization
    thing = Thing.new
    thing.name = "Yes"
    assert thing.name == "Yes"
  end
end

The test case will fail with: ActiveRecord::SerializationTypeMismatch: name was supposed to be a String, but was a TrueClass

I'd love to create a patch, but after digging through the code for a while, I still have no clue where things go wrong. Sorry.

Comments and changes to this ticket

  • Chris Barnett

    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.

  • Andreas Korth

    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.

  • Peter Wagenet

    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
  • Peter Wagenet

    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

    DHH September 10th, 2008 @ 06:06 AM

    What's the use case for serializing strings?

  • Tarmo Tänav

    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.

  • Andreas Korth

    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.

  • Peter Wagenet

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

    Out of curiosity, is this getting any attention?

  • Repository

    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...

  • viktor tron (strawberry)

    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

  • viktor tron (strawberry)

    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

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>

Attachments

Referenced by

Pages