This project is archived and is in readonly mode.

#2723 ✓committed
blythe

to_xml generates invalid xml with namespaced ActiveRecord

Reported by blythe | May 26th, 2009 @ 11:48 PM | in 2.3.4

When using an ActiveRecord object with a namespace, the generated xml is not valid.

MyApplication::Business::Project would generate
<my-application/business/project> which unescaped isn't valid xml.

When using the :include option, the association name is used but the type is set to the full project name.

In the patch I demodulized the names (which removes the type field), but I am unsure if the type should also be set to the full ruby class name.

Without patch:
<?xml version="1.0" encoding="UTF-8"?>
<my-application/business/project>
1 Active Record

<developer type="MyApplication::Business::Developer">
  <access-level type="NilClass">1</access-level>
  <created-at type="datetime" nil="true"></created-at>
  <developer-id type="NilClass">1</developer-id>
  <id type="integer">1</id>
  <joined-on type="NilClass">2004-10-10</joined-on>
  <name>David</name>
  <project-id type="NilClass">1</project-id>
  <salary type="integer">80000</salary>
  <updated-at type="datetime" nil="true"></updated-at>
</developer>

With patch:

<?xml version="1.0" encoding="UTF-8"?>
<project>
  <id type="integer">1</id>
  <name>Active Record</name>
  <developers type="array">
    <developer>
      <access-level type="NilClass">1</access-level>
      <created-at type="datetime" nil="true"></created-at>
      <developer-id type="NilClass">1</developer-id>
      <id type="integer">1</id>
      <joined-on type="NilClass">2004-10-10</joined-on>
      <name>David</name>
      <project-id type="NilClass">1</project-id>
      <salary type="integer">80000</salary>
      <updated-at type="datetime" nil="true"></updated-at>
    </developer>
    <developer>

Comments and changes to this ticket

  • CancelProfileIsBroken

    CancelProfileIsBroken August 6th, 2009 @ 02:52 PM

    • Tag changed from activerecord, module, namespace, serialize, to_xml, xml to activerecord, bugmash, module, namespace, serialize, to_xml, xml
  • Abhishek Parolkar

    Abhishek Parolkar August 9th, 2009 @ 06:15 PM

    passes test , with warning

    Warning: /test/cases/../../lib/active_record/serializers/xml_serializer.rb:323: warning: Object#type is deprecated; use Object#class

  • Rizwan Reza

    Rizwan Reza August 9th, 2009 @ 07:23 PM

    verified

    +1 This patch only works in 2-3-stable. All tests pass with the warning pasted by Abhishek.

  • Elad Meidar

    Elad Meidar August 9th, 2009 @ 07:25 PM

    +1 verified, +1 patch applies and tests pass on 2-3-stable with the same warning as Abhishek Parolkar described.

    using #class will just return the Column instance (ActiveRecord::ConnectionAdapters::Column), there should be a better way to access that "type" variable.

  • Edd Morgan

    Edd Morgan August 9th, 2009 @ 07:42 PM

    Confirmed passing tests on 2-3-stable; I'm also seeing the deprecation warning. Changing the offending call from Object#type to Object#class causes tests to fail. The two calls don't seem to be returning the same thing.

  • Josh Nichols

    Josh Nichols August 9th, 2009 @ 08:00 PM

    • Tag changed from activerecord, bugmash, module, namespace, serialize, to_xml, xml to activerecord, bugmash, module, namespace, patch, serialize, to_xml, verified, xml

    +1 for having valid xml generated, and verified the patch makes things happy.

    Attached a patch which gets rid of the Object#type deprecation warnings. This includes bythe's original commit in addition to the warning fix.

    The problem was type was being called on NilClass.

  • Josh Nichols

    Josh Nichols August 9th, 2009 @ 08:18 PM

    So, it looks like this is fixed to some extent on master. Instead of generating my-application/business/project as the root node, it makes my-application-business-project.

    If this is the case, should we backport that behavior to 2-3-stable?

  • Rizwan Reza

    Rizwan Reza August 9th, 2009 @ 08:59 PM

    verified

    +1 The patch applies cleanly to 2-3-stable.

  • Rizwan Reza

    Rizwan Reza August 9th, 2009 @ 08:59 PM

    I think Josh is right. If the functionality is there in master, we should backport that.

  • Josh Nichols

    Josh Nichols August 10th, 2009 @ 03:19 AM

    Attached a patch which backports behavior from master. Note, this produces different xml than the original post described.

    In particular, the root node is dasherized, ie my-application-project-business-project, and the association nodes have type attributes, ie type="MyApplication::Project::Developer".

  • Dan Croak

    Dan Croak August 10th, 2009 @ 03:30 AM

    +1 verified Josh's 2723-invalid_namespaced_xml-backport.patch applies cleanly and runs green.

  • Repository

    Repository August 10th, 2009 @ 04:14 AM

    • State changed from “new” to “committed”

    (from [15fd67e9d8155181695add77c8a7b11d92564391]) Backported XML serialization behavior from master for dealing with root nodes that have modules.

    ie,
    - the root node is dasherized, such that MyApplication::Business::Project becomes <my-application-project-business-project> - association children nodes have type attributes, such that MyApplication::Business::Developer becomes

    [#2723 state:committed]

    Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
    http://github.com/rails/rails/commit/15fd67e9d8155181695add77c8a7b1...

  • Jeremy Kemper

    Jeremy Kemper August 10th, 2009 @ 04:20 AM

    • Tag changed from activerecord, bugmash, module, namespace, patch, serialize, to_xml, verified, xml to activerecord, module, namespace, patch, serialize, to_xml, verified, xml
    • Milestone changed from 2.x to 2.3.4
  • Mark Foster

    Mark Foster January 30th, 2010 @ 05:36 PM

    Gentlemen. Have found that all this work to fix this breaks down when the ActiveRecord is in an array since the active_support/core_ext/array/conversions.rb
    to_xml method sets options[root] to the class name so that when root is called in the above patch, it finds a value already set and that value includes the invalid characters when the active record has namespaces.

    I could propose a patch, but I am not proficient in the open source protocols. One way is to add activerecord checking into the conversions.rb file and use model_name instead of class in setting the root, but I am not sure that is the correct approach. The other is in the xml_serializer.rb check if the options[root] value is the default created in conversions.rb, but that is potentially brittle. If someone could comment on the appropriate approach, I will propose a patch. Thank you.

  • Mark Foster
  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 09:44 PM

    • Tag cleared.
    • Importance changed from “” to “Low”

    Automatic cleanup of spam.

  • bingbing

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>

Referenced by

Pages