bjjb / ebayr

A small library to help using the eBay Trading API with Ruby
MIT License
60 stars 49 forks source link

Prevent switching of ActiveSupport::XmlMini.backend #30

Open DmytroVasin opened 4 years ago

DmytroVasin commented 4 years ago

Title: Ebayr should not change ActiveSupport::XmlMini.backend

This gem unexpectedly switches default ActiveSupport XMLMini backend. By default XmlMini.backend is equal to 'ActiveSupport::XmlMini_REXML' or 'REXML' (In Rails 5.2 and Rails 4.2)

When we call your gem - backend was unexpectedly switched to 'Nokogiri'

After some research, I found out that you require 'Nokogiri' to achieve correct parsing

PR that adds bug: https://github.com/bjjb/ebayr/pull/20/files

I have added a small spiky-fix that rollback to the old backend.

The more correct solution would look like that (use Nokogiri only for parsing purposes).

module Ebayr #:nodoc:
  class Response < Record
    ...
    hash = self.class.from_xml(Nokogiri::XML(@body)) if @body
    ...
  end
end

But this solution will not work - because of next code relies on the fact that in a Hash node key will be the first and attributes key will be the last

So I assume that Ebayr::Record should be fixed to not rely on a position in a hash. But I created the only workaround to solve a bug.

Example:

xml = "<Description><p class=\"p1\"><span class=\"s1\"><br/></span></p></Description>"
obj = Nokogiri::XML(xml).to_s

ActiveSupport::XmlMini.backend = 'REXML'
Hash.from_xml(obj)
=> {"Description"=>{"p"=>{"class"=>"p1", "span"=>{"class"=>"s1", "br"=>nil}}}}
                         |------------|  |--------------------------------|
                            Attributes                 NODEs

ActiveSupport::XmlMini.backend = 'Nokogiri'
Hash.from_xml(obj)
=> {"Description"=>{"p"=>{"span"=>{"br"=>nil, "class"=>"s1"}, "class"=>"p1"}}}
                         |---------------------------------|  |------------|
                                  NODEs                         Attributes

That happens because of the Rails parser implementation:
ActiveSupport::XmlMini_Nokogiri.parse(xml)
=> {"Description"=>{"p"=>[{"span"=>{"br"=>{}, "class"=>"s1"}, "class"=>"p1"}]}}

ActiveSupport::XmlMini_REXML.parse(xml)
=> {"Description"=>{"p"=>[{"class"=>"p1", "span"=>{"class"=>"s1", "br"=>{}}}]}}

Hope that will help.