#2723 to_xml generates invalid xml with namespaced ActiveRecord - Ruby on Rails

This project is archived and is in readonly mode.

#2723 ✓committed

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>

xml_modules.diff 2.7 KB

Comments and changes to this ticket

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”
You flagged this item as spam.
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 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.
You flagged this item as spam.
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.
You flagged this item as spam.
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.
You flagged this item as spam.
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.
- 2723-invalid_namespaced_xml.patch 3.8 KB
You flagged this item as spam.
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 August 9th, 2009 @ 08:59 PM
verified

+1 The patch applies cleanly to 2-3-stable.
Rizwan Reza August 9th, 2009 @ 08:59 PM
I think Josh is right. If the functionality is there in master, we should backport that.
You flagged this item as spam.
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".
- 2723-invalid_namespaced_xml-backport.patch 3.1 KB
You flagged this item as spam.
Dan Croak August 10th, 2009 @ 03:30 AM
+1 verified Josh's 2723-invalid_namespaced_xml-backport.patch applies cleanly and runs green.
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 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”
You flagged this item as spam.
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.
You flagged this item as spam.
Mark Foster January 31st, 2010 @ 09:34 AM
I created a ticket with a simple patch for this issue at:

https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets...
Ryan Bigg October 9th, 2010 @ 09:44 PM
- Tag cleared.
- Importance changed from “” to “Low”
Automatic cleanup of spam.
You flagged this item as spam.
bingbing March 26th, 2011 @ 02:08 AM
louis vuitton watches