Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

activesupport's to_xml shouldn't modify options hash

#672

This patch changes the to_xml core extensions for Array and Hash so that they dup the options hash instead of modifying the passed in version. The current behavior, as demonstrated in script/console, looks like this:

>> options = {:skip_instruct => true}
=> {:skip_instruct=>true}
>> [].to_xml(options)
=> "<nil-classes type=\"array\"/>\n"
>> options
=> {:builder=><nil-classes type="array"/>
<inspect/>
, :indent=>2}
>> 

Notice the options hash has been modified by the to_xml call. With included patch, the options hash is not modified:

>> options = {:skip_instruct => true}
=> {:skip_instruct=>true}
>> [].to_xml(options)
=> "<nil-classes type=\"array\"/>\n"
>> options
=> {:skip_instruct=>true}

Reported by David Burger · July 22nd, 2008 @ 06:04 AM

State: resolved
Milestone: none
Assigned to: nobody
Importance: none

Activity

  1. Alex MacCaw
    Alex MacCaw

    +1

    I've been burned by that one before.

    July 22nd, 2008 @ 09:54 AM

  2. Clemens Kofler
    Clemens Kofler

    Good one. Not duping the options hash before using it seems to be a problem that's all across Rails.

    +1

    July 22nd, 2008 @ 12:10 PM

  3. josh
    josh
    • State changed from new to stale

    Not sure if this is still an issue, just reopen if so.

    October 28th, 2008 @ 04:30 PM

  4. Chris Kampmeier
    Chris Kampmeier

    Yup, this is still a problem, I've been bitten by it a couple times.

    +1 for the patch, except that the test for Hash#to_xml was actually testing Array#to_xml. I fixed that, and rebased the patch against master, since it no longer applied. Here's a new one.

    May 18th, 2009 @ 05:42 AM

  5. CancelProfileIsBroken
    CancelProfileIsBroken
    • Tag changed from activesupport, core_ext, edge, patch, to_xml to activesupport, bugmash, core_ext, edge, patch, to_xml

    August 6th, 2009 @ 02:43 PM

  6. Dana Jones
    Dana Jones
    • Assigned user set to Pratik

    +1 - Problem exists on both 2-3-stable and 3-0-pre. Neither of the preceding patches would apply cleanly, so I created new patches for both versions.

    August 9th, 2009 @ 08:51 PM

  7. Dana Jones
  8. CancelProfileIsBroken
    CancelProfileIsBroken
    • State changed from stale to open

    August 9th, 2009 @ 08:52 PM

  9. Rizwan Reza
    Rizwan Reza

    verified

    +1 Both patches work perfectly. All tests pass.

    August 9th, 2009 @ 09:06 PM

  10. Repository
    Repository
    • State changed from open to resolved

    (from [c9d4bcf16337241cc270eddb402b17cc094c609e]) Fix that Hash#to_xml and Array#to_xml shouldn't modify their options hashes [#672 state:resolved] [David Burger, Dana Jones]

    Signed-off-by: Pratik Naik pratiknaik@gmail.com
    http://github.com/rails/rails/commit/c9d4bcf16337241cc270eddb402b17...

    August 9th, 2009 @ 09:47 PM

  11. Repository
    Repository

    (from [1382f4de1f9b0e443e7884bd4da53c20f0754568]) Fix that Hash#to_xml and Array#to_xml shouldn't modify their options hashes [#672 state:resolved]

    Signed-off-by: Pratik Naik pratiknaik@gmail.com
    http://github.com/rails/rails/commit/1382f4de1f9b0e443e7884bd4da53c...

    August 9th, 2009 @ 09:47 PM

  12. CancelProfileIsBroken
    CancelProfileIsBroken
    • Assigned user cleared.
    • Tag changed from activesupport, bugmash, core_ext, edge, patch, to_xml to activesupport, core_ext, edge, patch, to_xml
    • Milestone cleared.

    August 9th, 2009 @ 10:05 PM